[mlir][sme] Use signed comparison in ArmSMEToSCF #156834
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change switches from `ult` to `slt` in the comparison generated by
`TileLoadOpWithMaskAndPadNonZeroConversion`.
From the updated test:
```
%[[ROW_IS_ACTIVE:.*]] = arith.cmpi slt, %[[TILE_SLICE_INDEX]], %[[NUM_ROWS]] : index
```
Here:
- `%[[TILE_SLICE_INDEX]]` is always non-negative.
- `%[[NUM_ROWS]]` represents the number of remaining rows.
`%[[NUM_ROWS]]` is computed as:
```mlir
%c-4 = arith.constant -4 : index
%c-4_vscale = arith.muli %c-4, %vscale_11 : index
%num_rows_remaining = arith.addi %num_rows_init, %c-4_vscale : index
```
(inserted by the "arm-sme-vector-legalization" pass, see
`VectorLegalization.cpp`).
Because of the subtraction, `%num_rows_remaining` can be negative.
Therefore, the comparison must be signed (`slt`) rather than unsigned (`ult`).
Fixes: iree-org/iree#21714
banach-space
requested review from
dcaballe and
nicolasvasilache
as code owners
|
@llvm/pr-subscribers-mlir Author: Andrzej Warzyński (banach-space) ChangesThis change switches from From the updated test: Here:
%c-4 = arith.constant -4 : index
%c-4_vscale = arith.muli %c-4, %vscale_11 : index
%num_rows_remaining = arith.addi %num_rows_init, %c-4_vscale : index(inserted by the "arm-sme-vector-legalization" pass, see Because of the subtraction, Fixes: iree-org/iree#21714 Full diff: https://github.com/llvm/llvm-project/pull/156834.diff 2 Files Affected:
diff --git a/mlir/lib/Conversion/ArmSMEToSCF/ArmSMEToSCF.cpp b/mlir/lib/Conversion/ArmSMEToSCF/ArmSMEToSCF.cpp
index c69ede988b3dc..bad53c0a4a97a 100644
--- a/mlir/lib/Conversion/ArmSMEToSCF/ArmSMEToSCF.cpp
+++ b/mlir/lib/Conversion/ArmSMEToSCF/ArmSMEToSCF.cpp
@@ -309,7 +309,7 @@ struct TileLoadOpWithMaskAndPadNonZeroConversion
// Combine masks.
auto rowIsActive = arith::CmpIOp::create(
- rewriter, loc, arith::CmpIPredicate::ult, tileSliceIndex, numRows);
+ rewriter, loc, arith::CmpIPredicate::slt, tileSliceIndex, numRows);
auto rowIsActiveI32 = arith::ExtSIOp::create(
rewriter, loc, rewriter.getI32Type(), rowIsActive);
auto mask =
diff --git a/mlir/test/Conversion/ArmSMEToSCF/arm-sme-to-scf.mlir b/mlir/test/Conversion/ArmSMEToSCF/arm-sme-to-scf.mlir
index 6f2766ddc6e6e..e7bbedc586012 100644
--- a/mlir/test/Conversion/ArmSMEToSCF/arm-sme-to-scf.mlir
+++ b/mlir/test/Conversion/ArmSMEToSCF/arm-sme-to-scf.mlir
@@ -81,7 +81,7 @@ func.func @arm_sme_tile_load_hor_with_mask_and_pad_zero(%src : memref<?x?xi32>)
// CHECK-DAG: %[[VSCALE:.*]] = vector.vscale
// CHECK-NEXT: %[[NUM_TILE_SLICES:.*]] = arith.muli %[[C4]], %[[VSCALE]] : index
// CHECK-NEXT: scf.for %[[TILE_SLICE_INDEX:.*]] = %[[C0]] to %[[NUM_TILE_SLICES]] step %[[C1]] iter_args(%[[CURRENT_TILE:.*]] = %[[TILE]]) -> (vector<[4]x[4]xi32>) {
-// CHECK-NEXT: %[[ROW_IS_ACTIVE:.*]] = arith.cmpi ult, %[[TILE_SLICE_INDEX]], %[[NUM_ROWS]] : index
+// CHECK-NEXT: %[[ROW_IS_ACTIVE:.*]] = arith.cmpi slt, %[[TILE_SLICE_INDEX]], %[[NUM_ROWS]] : index
// CHECK-NEXT: %[[ROW_IS_ACTIVE_SEXT_I32:.*]] = arith.extsi %[[ROW_IS_ACTIVE]] : i1 to i32
// CHECK-NEXT: %[[MASK:.*]] = arith.andi %[[ROW_IS_ACTIVE_SEXT_I32]], %[[NUM_COLS_I32]] : i32
// CHECK-NEXT: %[[MASK_INDEX:.*]] = arith.index_cast %[[MASK]] : i32 to index
|
MacDue
approved these changes
Sep 4, 2025
egebeysel
approved these changes
Sep 4, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/55/builds/16720 Here is the relevant piece of the build log for the reference |
banach-space
added a commit
to banach-space/iree
that referenced
this pull request
Sep 5, 2025
This reverts commit 992860b. See iree-org#21714 for a discussion on the underlying issue. MLIR fix: llvm/llvm-project#156834 NOTE: This change can only be merged once the MLIR fix has been integrated. Signed-off-by: Andrzej Warzynski <andrzej.warzynski@arm.com>
banach-space
added a commit
to iree-org/iree
that referenced
this pull request
Sep 10, 2025
This reverts commit 992860b. See #21714 for a discussion on the underlying issue. MLIR fix: llvm/llvm-project#156834 Signed-off-by: Andrzej Warzynski <andrzej.warzynski@arm.com>
hhkit
pushed a commit
to opencompl/iree
that referenced
this pull request
Sep 11, 2025
) This reverts commit 992860b. See iree-org#21714 for a discussion on the underlying issue. MLIR fix: llvm/llvm-project#156834 Signed-off-by: Andrzej Warzynski <andrzej.warzynski@arm.com> Signed-off-by: Ivan Ho <ivan.ho@cl.cam.ac.uk>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change switches from
ulttosltin the comparison generated byTileLoadOpWithMaskAndPadNonZeroConversion.From the updated test:
Here:
%[[TILE_SLICE_INDEX]]is always non-negative.%[[NUM_ROWS]]represents the number of remaining rows.%[[NUM_ROWS]]is computed as:(inserted by the "arm-sme-vector-legalization" pass, see
VectorLegalization.cpp).Because of the subtraction,
%num_rows_remainingcan be negative.Therefore, the comparison must be signed (
slt) rather than unsigned (ult).Fixes: iree-org/iree#21714