[MLIR][ArithToLLVM] Fix index_cast on memref types generating invalid LLVM IR#189227
Conversation
… LLVM IR `arith.index_cast` and `arith.index_castui` accept memref operands (via `IndexCastTypeConstraint`), but `IndexCastOpLowering::matchAndRewrite` did not handle this case. When the operand was a memref, the conversion framework substituted the converted LLVM struct type, and the lowering incorrectly attempted to emit `llvm.sext`/`llvm.zext`/`llvm.trunc` on a struct value, producing invalid LLVM IR. Since LLVM uses opaque pointers, all memrefs with integer or index element types lower to the same `\!llvm.struct<(ptr, ptr, i64, ...)>` type, making `arith.index_cast` on memrefs a no-op at the LLVM level. Add a check that treats the memref case as an identity conversion (same as the same-bit-width path). Fixes llvm#92377 Assisted-by: Claude Code
|
@llvm/pr-subscribers-mlir Author: Mehdi Amini (joker-eph) Changes
Since LLVM uses opaque pointers, all memrefs with integer or index element types lower to the same Fixes #92377 Assisted-by: Claude Code Full diff: https://github.com/llvm/llvm-project/pull/189227.diff 2 Files Affected:
diff --git a/mlir/lib/Conversion/ArithToLLVM/ArithToLLVM.cpp b/mlir/lib/Conversion/ArithToLLVM/ArithToLLVM.cpp
index 11aae4de26c1e..f9ea8dba105a4 100644
--- a/mlir/lib/Conversion/ArithToLLVM/ArithToLLVM.cpp
+++ b/mlir/lib/Conversion/ArithToLLVM/ArithToLLVM.cpp
@@ -368,6 +368,14 @@ LogicalResult IndexCastOpLowering<OpTy, ExtCastTy>::matchAndRewrite(
return success();
}
+ // Memref index_cast is a no-op at the LLVM level since LLVM uses opaque
+ // pointers and memrefs of different integer/index element types all convert
+ // to the same LLVM struct type.
+ if (isa<MemRefType>(op.getIn().getType())) {
+ rewriter.replaceOp(op, adaptor.getIn());
+ return success();
+ }
+
bool isNonNeg = false;
if constexpr (std::is_same_v<ExtCastTy, LLVM::ZExtOp>)
isNonNeg = op.getNonNeg();
diff --git a/mlir/test/Conversion/ArithToLLVM/arith-to-llvm.mlir b/mlir/test/Conversion/ArithToLLVM/arith-to-llvm.mlir
index 6a6016c4f5b16..75601e215744c 100644
--- a/mlir/test/Conversion/ArithToLLVM/arith-to-llvm.mlir
+++ b/mlir/test/Conversion/ArithToLLVM/arith-to-llvm.mlir
@@ -160,6 +160,30 @@ func.func @index_castui_nneg_not_set(%arg0: i1) {
// -----
+// Memref index_cast is a no-op at the LLVM level since LLVM uses opaque
+// pointers and all memrefs with integer or index element types convert to the
+// same struct type. Verify that no sext/zext/trunc is generated.
+
+// CHECK-LABEL: @memref_index_cast
+// CHECK-NOT: llvm.sext
+// CHECK-NOT: llvm.trunc
+func.func @memref_index_cast(%arg0: memref<3xi32>) -> memref<3xindex> {
+ %0 = arith.index_cast %arg0 : memref<3xi32> to memref<3xindex>
+ return %0 : memref<3xindex>
+}
+
+// -----
+
+// CHECK-LABEL: @memref_index_castui
+// CHECK-NOT: llvm.zext
+// CHECK-NOT: llvm.trunc
+func.func @memref_index_castui(%arg0: memref<3xi32>) -> memref<3xindex> {
+ %0 = arith.index_castui %arg0 : memref<3xi32> to memref<3xindex>
+ return %0 : memref<3xindex>
+}
+
+// -----
+
// Checking conversion of signed integer types to floating point.
// CHECK-LABEL: @sitofp
func.func @sitofp(%arg0 : i32, %arg1 : i64) {
|
|
Though I'm rather surprised that we can even |
|
Do we have some documentation on strict aliasing expectations on Memref? (by this you mean "type based aliasing" I assume). I looked at https://mlir.llvm.org/docs/Dialects/MemRef/ but there is no mention of alias (almost). |
|
I don't think there are such expectations, hence just expressing surprise rather than blocking the patch |
|
Yeah I was asking because maybe we should document it (one way or another) |
… LLVM IR (llvm#189227) `arith.index_cast` and `arith.index_castui` accept memref operands (via `IndexCastTypeConstraint`), but `IndexCastOpLowering::matchAndRewrite` did not handle this case. When the operand was a memref, the conversion framework substituted the converted LLVM struct type, and the lowering incorrectly attempted to emit `llvm.sext`/`llvm.zext`/`llvm.trunc` on a struct value, producing invalid LLVM IR. Since LLVM uses opaque pointers, all memrefs with integer or index element types lower to the same `\!llvm.struct<(ptr, ptr, i64, ...)>` type, making `arith.index_cast` on memrefs a no-op at the LLVM level. Add a check that treats the memref case as an identity conversion (same as the same-bit-width path). Fixes llvm#92377 Assisted-by: Claude Code
… LLVM IR (llvm#189227) `arith.index_cast` and `arith.index_castui` accept memref operands (via `IndexCastTypeConstraint`), but `IndexCastOpLowering::matchAndRewrite` did not handle this case. When the operand was a memref, the conversion framework substituted the converted LLVM struct type, and the lowering incorrectly attempted to emit `llvm.sext`/`llvm.zext`/`llvm.trunc` on a struct value, producing invalid LLVM IR. Since LLVM uses opaque pointers, all memrefs with integer or index element types lower to the same `\!llvm.struct<(ptr, ptr, i64, ...)>` type, making `arith.index_cast` on memrefs a no-op at the LLVM level. Add a check that treats the memref case as an identity conversion (same as the same-bit-width path). Fixes llvm#92377 Assisted-by: Claude Code
arith.index_castandarith.index_castuiaccept memref operands (viaIndexCastTypeConstraint), butIndexCastOpLowering::matchAndRewritedid not handle this case. When the operand was a memref, the conversion framework substituted the converted LLVM struct type, and the lowering incorrectly attempted to emitllvm.sext/llvm.zext/llvm.truncon a struct value, producing invalid LLVM IR.Since LLVM uses opaque pointers, all memrefs with integer or index element types lower to the same
\!llvm.struct<(ptr, ptr, i64, ...)>type, makingarith.index_caston memrefs a no-op at the LLVM level. Add a check that treats the memref case as an identity conversion (same as the same-bit-width path).Fixes #92377
Assisted-by: Claude Code