Skip to content

[LinalgExt] Fix gather->extract_slice to invert the dimension_map - #24871

Open
EylonKrause wants to merge 1 commit into
iree-org:mainfrom
EylonKrause:fix/gather-to-extract-inverse-dimmap
Open

[LinalgExt] Fix gather->extract_slice to invert the dimension_map#24871
EylonKrause wants to merge 1 commit into
iree-org:mainfrom
EylonKrause:fix/gather-to-extract-inverse-dimmap

Conversation

@EylonKrause

Copy link
Copy Markdown
Contributor

Problem

ConvertGatherToExtract (canonicalization of iree_linalg_ext.gather) rewrites the gather into a tensor.extract_slice, building the slice offsets from the loaded index values:

// offsets[i] = indexValue[i]
applyPermutationToVector(offsets, gatherOp.getDimensionMap());

applyPermutationToVector(v, p) computes result[i] = v[p[i]], so this produces sourceOffset[i] = indexValue[dimensionMap[i]].

But that is the inverse of the op's actual semantics. dimension_map maps each index component i to the source dimension it indexes, and GatherOp::generateScalarImplementation implements it as:

auto dim = dimMap[i];
starts[dim] = ret;         // starts[dimensionMap[i]] = indexValue[i]
... memref.load(getSource(), starts)

i.e. sourceOffset[dimensionMap[i]] = indexValue[i]. Building the offset vector from the in-order index values is therefore the inverse permutation of dimension_map.

The forward and inverse permutations coincide only when dimension_map is an involution (identity, or a single transposition). Every existing gather_to_extract_slice* test uses [0], [0, 1], or [1, 0] — all involutions — so the bug is invisible to the current suite. For a non-involutive map such as [1, 2, 0], the rewrite reads the wrong source coordinates; and when the indexed dimensions have different extents, an index value that is in-range for its intended dimension but larger than the dimension it is wrongly placed on yields an out-of-bounds extract_slice.

Example

dimension_map = [1, 2, 0], index values [a, b, c]:

source offsets (dim order)
generateScalarImplementation (correct) [c, a, b]
current fold [b, c, a]
this PR [c, a, b]

Fix

Invert the permutation before applying it, matching generateScalarImplementation. invertPermutationVector is already used elsewhere in this file.

Testing

I verified the offset arithmetic against generateScalarImplementation's starts[dimMap[i]] = indexValue[i] with a standalone check over several maps: for every involutive map the current code and the fix both match the ground truth, and for the non-involutive maps [1,2,0], [2,0,1], [3,0,1,2] the current code diverges while the fix matches. Added a FileCheck regression test (gather_to_extract_slice_perm3, dimension_map = [1, 2, 0]) asserting the inverse-permuted offsets; it fails on the current code and passes with the fix. I wasn't able to run the lit suite locally (IREE doesn't fully build on my box) — happy to adjust the CHECK if CI flags any ordering.

Disclosure: this contribution was authored with an AI coding assistant (Claude) and reviewed and validated before submission.

ConvertGatherToExtract lowers iree_linalg_ext.gather to a tensor.extract_slice,
building the slice offsets from the loaded index values. It applied the op's
dimension_map directly to the in-order index values:

    applyPermutationToVector(offsets, gatherOp.getDimensionMap());

But dimension_map maps each index component i to the source dimension it
indexes: GatherOp::generateScalarImplementation sets
starts[dimensionMap[i]] = indexValue[i]. Constructing the source offsets from
the in-order index values is therefore the *inverse* permutation. Applying the
map directly is correct only when it is an involution (identity or a single
transposition) -- which is all the existing tests cover. For a non-involutive
dimension_map (e.g. [1, 2, 0]) the fold reads the wrong source coordinates, and
when the indexed dimensions have different extents it can produce an
out-of-bounds extract_slice.

Invert the permutation to match generateScalarImplementation, and add a
regression test with dimension_map = [1, 2, 0].

Signed-off-by: Eylon Krause <eylon1909@gmail.com>
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