Repository navigation
slice_scatter: leave the input unchanged for an empty slice - #145
john-rocky wants to merge 5 commits into
Conversation
An assignment to an empty slice, such as mask[:, 8:] = 1.0 on a dim of size 8, exports to a slice_scatter whose src has a zero-size dim. PyTorch returns the input unchanged. The lowering emitted a slice_update with the empty update, and to_coreai() then died with a segmentation fault.
|
nit: The PR looks good, could you also check the fix for validity on dynamic shapes ? |
test_slice_assignment_dynamic exports the test_slice_assignment model with every dim but the sliced one dynamic, then runs the program at three sizes. test_slice_assignment_from_dynamic_end assigns to x[:, n:] with n = x.shape[1] dynamic. In both, src keeps a static zero-size dim, so replace_slice_scatter returns the input. Without that check, test_slice_assignment_from_dynamic_end crashes in to_coreai() (exit 139), and test_slice_assignment_dynamic aborts with --compute-unit-kind=gpu. Tested: - pytest tests/ops/test_ops.py -k slice_assignment with --compute-unit-kind=gpu and with --compute-unit-kind=cpu: 9 passed each. - pytest tests/ -n 4: 6566 passed, 126 skipped, 18 xfailed, 1 xpassed. - ruff check and ruff format --check: clean.
|
Thanks for the review. The fix is valid with dynamic shapes, and c081281 adds two tests for it. The fix leaves the input unchanged only when
When the slice length is dynamic, as in One thing outside this PR: in that case, with With a static start on a dynamic dim, such as |
Summary
An assignment to an empty slice, such as
mask[:, 8:, :, :] = 1.0on a dim of size 8, exports to aslice_scatterwhosesrchas a zero-size dim. PyTorch returns the input unchanged. The lowering emitted acoreai.slice_updatewith that empty update, andto_coreai()then died with a segmentation fault (exit 139).replace_slice_scatternow returns the input whensrchas a static zero-size dim.Reading an empty slice (the SIGTRAP in the issue notes) takes another path and is not changed here.
Testing
TestSliceScatter::test_slice_assignmentassigns to[:, 4:]and[:, 8:]of a constant and of an input-derived tensor and compares the output with PyTorch. Before the fix, the8:cases exit 139. After it, they pass.Fixes #142