[MLIR] Avoid vector.extract_strided_slice when not needed

[lialan]

In staticallyExtractSubvector, When the extracting slice is the same as source vector, do not need to emit vector.extract_strided_slice.

This fixes the lit test case @vector_store_i4 in
mlir\test\Dialect\Vector\vector-emulate-narrow-type.mlir, where converting from vector<8xi4> to vector<4xi8> does not need slice extraction.

The issue was introduced in #113411 and #115070, CI failure link: https://buildkite.com/llvm-project/github-pull-requests/builds/118845

This PR does not include a lit test case because it is a fix and the above mentioned @vector_store_i4 test actually tests the mechanism.

[llvmbot]

@llvm/pr-subscribers-mlir-vector

@llvm/pr-subscribers-mlir

Author: lialan (lialan)

Changes

In staticallyExtractSubvector, When the extracting slice is the same as source vector, do not need to emit vector.extract_strided_slice.

This fixes the lit test case @<!-- -->vector_store_i4 in
mlir\test\Dialect\Vector\vector-emulate-narrow-type.mlir, where converting from vector<8xi4> to vector<4xi8> does not need slice extraction.

---

Full diff: https://github.com/llvm/llvm-project/pull/115941.diff

1 Files Affected:

• (modified) mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp (+7)

diff --git a/mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp b/mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp
index 7578aadee23a6e..6fd6ccc04e8045 100644
--- a/mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp
+++ b/mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp
@@ -150,6 +150,13 @@ static Value staticallyExtractSubvector(OpBuilder &rewriter, Location loc,
   assert((vectorType.getRank() == 1 && extractType.getRank() == 1) &&
          "expected 1-D source and destination types");
   (void)vectorType;
+  assert(frontOffset + subvecSize <= vectorType.getNumElements() &&
+         "subvector out of bounds");
+
+  // do not need extraction if the subvector size is the same as the source
+  if (vectorType.getNumElements() == subvecSize)
+    return source;
+
   auto offsets = rewriter.getI64ArrayAttr({frontOffset});
   auto sizes = rewriter.getI64ArrayAttr({subvecSize});
   auto strides = rewriter.getI64ArrayAttr({1});

[MaheshRavishankar]

Is there a lit test with this? I thought that this was meant to fix a lit test?

[hanhanW]

@lialan told me that it is fixing the post CI failures. E.g., https://buildkite.com/llvm-project/github-pull-requests/builds/118845

Please add the link to the PR description which makes is clearer.

[lialan]

@hanhanW Thanks Hanhan, updated the PR description to include more information about this.

[lialan]

Is there a lit test with this? I thought that this was meant to fix a lit test?

Tried to build a lit test for it, but it actually totally overlaps with the existing failing test. So I specifically mentioned such case int the PR description.

[joker-eph]

Do we understand why this fails there but not for the post-merge bots? The test is passing here at head for example: https://lab.llvm.org/buildbot/#/builders/138/builds/6300/steps/6/logs/stdio

I'd really like a test I could reproduce locally for this!

[hanhanW]

Do we understand why this fails there but not for the post-merge bots? The test is passing here at head for example: https://lab.llvm.org/buildbot/#/builders/138/builds/6300/steps/6/logs/stdio

Ah, there is a race when I clicked the merge button and you left a comment.. Any preference how to proceed here?

[kuhar]

nit: Use proper sentences in code comments: https://llvm.org/docs/CodingStandards.html#commenting