Skip to content

Fix SIGABRT on zero-extent slice copy (SliceCopyStartGreaterThanDimSize) - #4758

Open
alexxony wants to merge 4 commits into
llvm:mainfrom
alexxony:fix-slicecopy-zero-extent-crash
Open

alexxony wants to merge 4 commits into
llvm:mainfrom
alexxony:fix-slicecopy-zero-extent-crash

Conversation

@alexxony

@alexxony alexxony commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes the SIGABRT crash in SliceCopyStartGreaterThanDimSize_Module_basic
(linalg-on-tensors e2e test). The crash happens because x[100:10:1] = y
with y = torch.rand(0, 4, 4) (an empty slice source) hits two independent
bugs in the lowering/runtime path. Both are fixed here; each is a small,
localized commit.

Commit 1: TorchToTMTensor broadcast fold (numpy semantics bug)

getBroadcastShape (lib/Conversion/TorchToTMTensor/TorchToTMTensor.cpp)
folds index-tensor broadcast sizes with max(size, acc), where acc
starts at 1. This maps a size-0 dimension (from an empty index/slice) to
1 instead of preserving 0 — the numpy broadcasting rule is
(size == 1) ? acc : size, which only differs from max at 0. This
corrupted the scatter loop bound for index_put lowering derived from
RecomposeSliceCopy_.

Commit 2: RefBackend zero-extent memref stride canonicalization

Separately, torch.rand(0, 4, 4).numpy() reports strides (0, 0, 0).
When RefBackend marshals this into a memref descriptor as-is, the
runtime-verification pass's memref.cast check (identity/contiguous
layout, last-dim stride == 1) rejects it and aborts — even though the
array has zero addressable elements, so the stride is semantically
irrelevant.

This is the same fix as #4680, which was self-closed by the author without
review on 2026-07-31. Looking at that author's other PRs closed the same
day, the closures were driven by a TOSA-specific realization (zero-extent
tensors aren't TOSA-conformant) — #4680 targets RefBackend
(linalg-on-tensors), which has no such constraint, so it appears to have
been swept up in that batch close rather than rejected on its own merits.
I independently hit the same bug, arrived at the same fix, and am
reviving it here with a regression test.

Commit 3: keep the ONNX xfail set consistent

ONNX_CRASHING_SET inherits LINALG_CRASHING_SET, so removing the four
zero-extent tests from the latter makes the onnx config (which CI always
runs) attempt them. Running it showed:

  • TraceModule_empty and TraceUnsignedIntModule_empty now pass, so they
    are removed from ONNX_XFAIL_SET (they would otherwise be unexpected
    passes).
  • SliceCopyStartGreaterThanDimSize_Module_basic fails under onnx in
    torch.onnx.export (inconsistent constraints for the empty slice), so it
    is added to ONNX_XFAIL_SET.
  • ReduceAllDimEmpty_basic passes under onnx and needs no change.

Testing

  • New lit test: projects/pt1/python/test/refbackend/zero_extent_memref.py
  • check-torch_mlir-conversion-torchtotmtensor: 1/1
  • check-torch_mlir-conversion-torchtolinalg: 22/22 (no regressions)
  • SliceCopyStartGreaterThanDimSize_Module_basic via
    projects.pt1.e2e_testing.main -c linalg: now PASSES (was SIGABRT)
  • Full, unfiltered runs of the four configs CI uses (onnx, fx_importer,
    fx_importer_stablehlo, fx_importer_tosa) on this branch: no crashes.
    The only non-passing results are 22 unexpected passes under onnx and 3
    failures under fx_importer_tosa (ArgminIntModule_basic,
    ArgminIntModule_multiple_mins, ReduceMinAlongDimSignedInt_basic), and
    the same ones occur on the base commit, so they are not caused by this PR.
    The four tests above end up PASS/XFAIL as intended in each config (torch
    2.15.0.dev20260917).

Scope note

SliceCopyStartGreaterThanDimSize_Module_basic also appears in xfail
lists for other backends (e.g. TOSA/STABLEHLO sections) — those are
not touched here. The stablehlo and tosa sets do not inherit
LINALG_CRASHING_SET, and with this change the tests stay expected-fail
there. This PR removes the entries from the linalg-on-tensors
LINALG_CRASHING_SET, where they were directly verified, plus the ONNX
adjustment above.

Assisted-by: Claude (Anthropic)

alexxony and others added 2 commits September 13, 2026 18:24
getBroadcastShape used max(size, acc) to fold index broadcast sizes,
which maps a size-0 dimension to 1 since the accumulator starts at 1.
This diverges from numpy broadcasting semantics and corrupts loop
bounds for index_put/scatter lowering when an index tensor has a
zero-extent dimension (e.g. from an empty slice).

Replace with the correct fold: (size == 1) ? acc : size.

Verified: forcing valid strides on a zero-extent y in
SliceCopyStartGreaterThanDimSize_Module_basic now succeeds (was OOB
load abort before this fix). check-torch_mlir-conversion-torchtotmtensor
and check-torch_mlir-conversion-torchtolinalg pass 23/23.
PyTorch→NumPy conversion produces non-canonical strides for
zero-element tensors. These fail memref.cast runtime verification
against statically-shaped contiguous memrefs at the RefBackend ABI
boundary.

Add _canonicalize_zero_element_strides() to rewrite zero-element
array strides to canonical contiguous form before constructing the
memref descriptor in invoke(). Non-empty arrays are unchanged.

Supersedes llvm#4680 (self-closed without review,
2026-07-31) — same fix, revived here after independent diagnosis.

Combined with the prior TorchToTMTensor broadcast fix, this resolves
SliceCopyStartGreaterThanDimSize_Module_basic (previously SIGABRT on
memref.cast stride mismatch for a zero-extent slice source).

Verified (Colab, torchmlir-pt3):
- New lit test test/refbackend/zero_extent_memref.py: PASS
- check-torch_mlir-conversion-torchtotmtensor: 1/1
- check-torch_mlir-conversion-torchtolinalg: 22/22
- SliceCopyStartGreaterThanDimSize_Module_basic harness run: PASS (rc=0)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CLerxrBvP29pgpxDJ9D4zJ
@alexxony alexxony closed this Sep 13, 2026
@alexxony alexxony reopened this Sep 13, 2026
@alexxony

Copy link
Copy Markdown
Contributor Author

Friendly note: CI shows action_required (needs a maintainer to approve the workflow run for first-time contributors) rather than having actually run. Happy to address any review feedback once CI is green.

alexxony and others added 2 commits October 10, 2026 00:06
…ests

ONNX_CRASHING_SET inherits LINALG_CRASHING_SET, so removing the four
zero-extent tests from it makes the onnx config (run by CI on every push)
attempt them for the first time:

- TraceModule_empty and TraceUnsignedIntModule_empty now pass under onnx,
  so drop them from ONNX_XFAIL_SET (they were XPASS).
- SliceCopyStartGreaterThanDimSize_Module_basic fails under onnx with a
  torch.onnx export error (inconsistent constraints for the empty slice),
  so add it to ONNX_XFAIL_SET.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Join the broadcastSizes assignment onto one line; this is what the
pre-commit clang-format hook produces for the getBroadcastShape fix.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant