[Mlir-commits] [mlir] f1bdb76 - [mlir][MemRefToLLVM] fix incorrect `nuw` on `GEP/mul` when lowering `memref.load/store` with negative strides (#204309)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Thu Jun 18 06:38:37 PDT 2026
Author: Federico Bruzzone
Date: 2026-06-18T13:38:32Z
New Revision: f1bdb76cbf817c584cdd2853a49d9a992558c954
URL: https://github.com/llvm/llvm-project/commit/f1bdb76cbf817c584cdd2853a49d9a992558c954
DIFF: https://github.com/llvm/llvm-project/commit/f1bdb76cbf817c584cdd2853a49d9a992558c954.diff
LOG: [mlir][MemRefToLLVM] fix incorrect `nuw` on `GEP/mul` when lowering `memref.load/store` with negative strides (#204309)
`MemRefToLLVM` was unconditionally emitting `getelementptr inbounds|nuw`
(and consequently `mul overflow<nsw,nuw>` on every intermediate index
computation inside `getStridedElementPtr`) for all `memref.load` and
`memref.store` lowerings.
This is _unsound_ when any stride is negative or dynamic.
`getStridedElementPtr` propagates `GEPNoWrapFlags::nuw` to
`IntegerOverflowFlags::nuw` on every intermediate `llvm.mul` and
`llvm.add` it emits. With a negative stride (e.g. `-1`, which is
`2^64-1` unsigned), an access like index=5 produces `mul nuw 5,
(2^64-1)`, which unsigned-overflows and yields poison per LangRef —
regardless of whether the final offset happens to be non-negative.
This issue came up in the discussion in PR #202118. Thanks to
@banach-space for the detailed discussion.
This PR hopefully concludes the path to fix the regression related to
`affine-super-vectorize` (see
#201180 and #202766 for details).
---------
Signed-off-by: Federico Bruzzone <federico.bruzzone.i at gmail.com>
Co-authored-by: Tobias Gysi <tobias.gysi at nextsilicon.com>
Added:
Modified:
mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
mlir/test/Conversion/MemRefToLLVM/convert-dynamic-memref-ops.mlir
mlir/test/Conversion/MemRefToLLVM/expand-then-convert-to-llvm.mlir
Removed:
################################################################################
diff --git a/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td b/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
index 9dba4d790d631..d316fc6c2eef8 100644
--- a/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
+++ b/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
@@ -1276,10 +1276,14 @@ def LoadOp : MemRef_Op<"load",
The number of indices must match the rank of the memref. The indices must
be in-bounds: `0 <= idx < dim_size`.
- Lowerings of `memref.load` may emit attributes, e.g. `inbouds` + `nuw`
- when converting to LLVM's `llvm.getelementptr`, that would cause undefined
- behavior if indices are out of bounds or if computing the offset in the
- memref would cause signed overflow of the `index` type.
+ Lowerings of `memref.load` may emit no-wrap flags on
+ `llvm.getelementptr` when converting to LLVM. The `inbounds` flag is
+ always emitted (valid since indices are guaranteed in-bounds) and causes
+ undefined behavior if that precondition is violated. The `nuw` flag is
+ emitted only when all strides of the memref are statically non-negative;
+ with negative strides, `nuw` would propagate to intermediate `mul`
+ operations and cause unsigned overflow (poison) even for in-bounds
+ indices.
The single result of `memref.load` is a value with the same type as the
element type of the memref.
@@ -2057,10 +2061,14 @@ def MemRef_StoreOp : MemRef_Op<"store",
The number of indices must match the rank of the memref. The indices must
be in-bounds: `0 <= idx < dim_size`.
- Lowerings of `memref.store` may emit attributes, e.g. `inbouds` + `nuw`
- when converting to LLVM's `llvm.getelementptr`, that would cause undefined
- behavior if indices are out of bounds or if computing the offset in the
- memref would cause signed overflow of the `index` type.
+ Lowerings of `memref.store` may emit no-wrap flags on
+ `llvm.getelementptr` when converting to LLVM. The `inbounds` flag is
+ always emitted (valid since indices are guaranteed in-bounds) and causes
+ undefined behavior if that precondition is violated. The `nuw` flag is
+ emitted only when all strides of the memref are statically non-negative;
+ with negative strides, `nuw` would propagate to intermediate `mul`
+ operations and cause unsigned overflow (poison) even for in-bounds
+ indices.
A set `nontemporal` attribute indicates that this store is not expected to
be reused in the cache. For details, refer to the
diff --git a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
index 61069fc4d660d..72046c1b3d3cc 100644
--- a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
+++ b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
@@ -38,8 +38,20 @@ namespace mlir {
using namespace mlir;
-static constexpr LLVM::GEPNoWrapFlags kNoWrapFlags =
- LLVM::GEPNoWrapFlags::inbounds | LLVM::GEPNoWrapFlags::nuw;
+/// Returns GEP no-wrap flags for a memref load/store.
+/// inbounds is always valid when indices are in-bounds per the memref spec.
+/// nuw requires every index*stride term to not unsigned-wrap, which holds iff
+/// all strides are statically non-negative. Negative strides would make the
+/// intermediate mul nuw overflow (e.g., idx * (-1 as u64) wraps for idx > 0).
+static LLVM::GEPNoWrapFlags getLoadStoreNoWrapFlags(MemRefType type) {
+ auto [strides, offset] = type.getStridesAndOffset();
+ LLVM::GEPNoWrapFlags flags = LLVM::GEPNoWrapFlags::inbounds;
+ if (llvm::all_of(strides, [](int64_t s) {
+ return !ShapedType::isDynamic(s) && s >= 0;
+ }))
+ flags = flags | LLVM::GEPNoWrapFlags::nuw;
+ return flags;
+}
namespace {
@@ -964,9 +976,9 @@ struct LoadOpLowering : public LoadStoreOpLowering<memref::LoadOp> {
// Per memref.load spec, the indices must be in-bounds:
// 0 <= idx < dim_size, and additionally all offsets are non-negative,
// hence inbounds and nuw are used when lowering to llvm.getelementptr.
- Value dataPtr = getStridedElementPtr(rewriter, loadOp.getLoc(), type,
- adaptor.getMemref(),
- adaptor.getIndices(), kNoWrapFlags);
+ Value dataPtr = getStridedElementPtr(
+ rewriter, loadOp.getLoc(), type, adaptor.getMemref(),
+ adaptor.getIndices(), getLoadStoreNoWrapFlags(type));
rewriter.replaceOpWithNewOp<LLVM::LoadOp>(
loadOp, typeConverter->convertType(type.getElementType()), dataPtr,
loadOp.getAlignment().value_or(0), false, loadOp.getNontemporal());
@@ -987,9 +999,9 @@ struct StoreOpLowering : public LoadStoreOpLowering<memref::StoreOp> {
// Per memref.store spec, the indices must be in-bounds:
// 0 <= idx < dim_size, and additionally all offsets are non-negative,
// hence inbounds and nuw are used when lowering to llvm.getelementptr.
- Value dataPtr =
- getStridedElementPtr(rewriter, op.getLoc(), type, adaptor.getMemref(),
- adaptor.getIndices(), kNoWrapFlags);
+ Value dataPtr = getStridedElementPtr(
+ rewriter, op.getLoc(), type, adaptor.getMemref(), adaptor.getIndices(),
+ getLoadStoreNoWrapFlags(type));
rewriter.replaceOpWithNewOp<LLVM::StoreOp>(op, adaptor.getValue(), dataPtr,
op.getAlignment().value_or(0),
false, op.getNontemporal());
diff --git a/mlir/test/Conversion/MemRefToLLVM/convert-dynamic-memref-ops.mlir b/mlir/test/Conversion/MemRefToLLVM/convert-dynamic-memref-ops.mlir
index 543fdf5c26f5e..02baff36ba155 100644
--- a/mlir/test/Conversion/MemRefToLLVM/convert-dynamic-memref-ops.mlir
+++ b/mlir/test/Conversion/MemRefToLLVM/convert-dynamic-memref-ops.mlir
@@ -175,9 +175,9 @@ func.func @mixed_load(%mixed : memref<42x?xf32>, %i : index, %j : index) {
// CHECK-DAG: %[[J:.*]] = builtin.unrealized_conversion_cast %[[Jarg]]
// CHECK: %[[ptr:.*]] = llvm.extractvalue %[[ld:.*]][1] : !llvm.struct<(ptr, ptr, i64, array<2 x i64>, array<2 x i64>)>
// CHECK-NEXT: %[[st0:.*]] = llvm.extractvalue %[[ld]][4, 0] : !llvm.struct<(ptr, ptr, i64, array<2 x i64>, array<2 x i64>)>
-// CHECK-NEXT: %[[offI:.*]] = llvm.mul %[[I]], %[[st0]] overflow<nsw, nuw> : i64
-// CHECK-NEXT: %[[off1:.*]] = llvm.add %[[offI]], %[[J]] overflow<nsw, nuw> : i64
-// CHECK-NEXT: %[[addr:.*]] = llvm.getelementptr inbounds|nuw %[[ptr]][%[[off1]]] : (!llvm.ptr, i64) -> !llvm.ptr, f32
+// CHECK-NEXT: %[[offI:.*]] = llvm.mul %[[I]], %[[st0]] overflow<nsw> : i64
+// CHECK-NEXT: %[[off1:.*]] = llvm.add %[[offI]], %[[J]] overflow<nsw> : i64
+// CHECK-NEXT: %[[addr:.*]] = llvm.getelementptr inbounds %[[ptr]][%[[off1]]] : (!llvm.ptr, i64) -> !llvm.ptr, f32
// CHECK-NEXT: llvm.load %[[addr]] : !llvm.ptr -> f32
%0 = memref.load %mixed[%i, %j] : memref<42x?xf32>
return
@@ -192,9 +192,9 @@ func.func @dynamic_load(%dynamic : memref<?x?xf32>, %i : index, %j : index) {
// CHECK-DAG: %[[J:.*]] = builtin.unrealized_conversion_cast %[[Jarg]]
// CHECK: %[[ptr:.*]] = llvm.extractvalue %[[ld:.*]][1] : !llvm.struct<(ptr, ptr, i64, array<2 x i64>, array<2 x i64>)>
// CHECK-NEXT: %[[st0:.*]] = llvm.extractvalue %[[ld]][4, 0] : !llvm.struct<(ptr, ptr, i64, array<2 x i64>, array<2 x i64>)>
-// CHECK-NEXT: %[[offI:.*]] = llvm.mul %[[I]], %[[st0]] overflow<nsw, nuw> : i64
-// CHECK-NEXT: %[[off1:.*]] = llvm.add %[[offI]], %[[J]] overflow<nsw, nuw> : i64
-// CHECK-NEXT: %[[addr:.*]] = llvm.getelementptr inbounds|nuw %[[ptr]][%[[off1]]] : (!llvm.ptr, i64) -> !llvm.ptr, f32
+// CHECK-NEXT: %[[offI:.*]] = llvm.mul %[[I]], %[[st0]] overflow<nsw> : i64
+// CHECK-NEXT: %[[off1:.*]] = llvm.add %[[offI]], %[[J]] overflow<nsw> : i64
+// CHECK-NEXT: %[[addr:.*]] = llvm.getelementptr inbounds %[[ptr]][%[[off1]]] : (!llvm.ptr, i64) -> !llvm.ptr, f32
// CHECK-NEXT: llvm.load %[[addr]] : !llvm.ptr -> f32
%0 = memref.load %dynamic[%i, %j] : memref<?x?xf32>
return
@@ -230,9 +230,9 @@ func.func @dynamic_store(%dynamic : memref<?x?xf32>, %i : index, %j : index, %va
// CHECK-DAG: %[[J:.*]] = builtin.unrealized_conversion_cast %[[Jarg]]
// CHECK: %[[ptr:.*]] = llvm.extractvalue %[[ld:.*]][1] : !llvm.struct<(ptr, ptr, i64, array<2 x i64>, array<2 x i64>)>
// CHECK-NEXT: %[[st0:.*]] = llvm.extractvalue %[[ld]][4, 0] : !llvm.struct<(ptr, ptr, i64, array<2 x i64>, array<2 x i64>)>
-// CHECK-NEXT: %[[offI:.*]] = llvm.mul %[[I]], %[[st0]] overflow<nsw, nuw> : i64
-// CHECK-NEXT: %[[off1:.*]] = llvm.add %[[offI]], %[[J]] overflow<nsw, nuw> : i64
-// CHECK-NEXT: %[[addr:.*]] = llvm.getelementptr inbounds|nuw %[[ptr]][%[[off1]]] : (!llvm.ptr, i64) -> !llvm.ptr, f32
+// CHECK-NEXT: %[[offI:.*]] = llvm.mul %[[I]], %[[st0]] overflow<nsw> : i64
+// CHECK-NEXT: %[[off1:.*]] = llvm.add %[[offI]], %[[J]] overflow<nsw> : i64
+// CHECK-NEXT: %[[addr:.*]] = llvm.getelementptr inbounds %[[ptr]][%[[off1]]] : (!llvm.ptr, i64) -> !llvm.ptr, f32
// CHECK-NEXT: llvm.store %{{.*}}, %[[addr]] : f32, !llvm.ptr
memref.store %val, %dynamic[%i, %j] : memref<?x?xf32>
return
@@ -247,9 +247,9 @@ func.func @mixed_store(%mixed : memref<42x?xf32>, %i : index, %j : index, %val :
// CHECK-DAG: %[[J:.*]] = builtin.unrealized_conversion_cast %[[Jarg]]
// CHECK: %[[ptr:.*]] = llvm.extractvalue %[[ld:.*]][1] : !llvm.struct<(ptr, ptr, i64, array<2 x i64>, array<2 x i64>)>
// CHECK-NEXT: %[[st0:.*]] = llvm.extractvalue %[[ld]][4, 0] : !llvm.struct<(ptr, ptr, i64, array<2 x i64>, array<2 x i64>)>
-// CHECK-NEXT: %[[offI:.*]] = llvm.mul %[[I]], %[[st0]] overflow<nsw, nuw> : i64
-// CHECK-NEXT: %[[off1:.*]] = llvm.add %[[offI]], %[[J]] overflow<nsw, nuw> : i64
-// CHECK-NEXT: %[[addr:.*]] = llvm.getelementptr inbounds|nuw %[[ptr]][%[[off1]]] : (!llvm.ptr, i64) -> !llvm.ptr, f32
+// CHECK-NEXT: %[[offI:.*]] = llvm.mul %[[I]], %[[st0]] overflow<nsw> : i64
+// CHECK-NEXT: %[[off1:.*]] = llvm.add %[[offI]], %[[J]] overflow<nsw> : i64
+// CHECK-NEXT: %[[addr:.*]] = llvm.getelementptr inbounds %[[ptr]][%[[off1]]] : (!llvm.ptr, i64) -> !llvm.ptr, f32
// CHECK-NEXT: llvm.store %{{.*}}, %[[addr]] : f32, !llvm.ptr
memref.store %val, %mixed[%i, %j] : memref<42x?xf32>
return
@@ -645,3 +645,21 @@ func.func @ranked_unranked() {
memref.cast %0 : memref<1 x memref<* x f32>> to memref<* x memref<* x f32>>
return
}
+
+// -----
+
+// Verify that loading from a negative-stride memref does not emit nuw on the
+// GEP or the intermediate mul: mul nuw with a negative (unsigned-large) stride
+// would produce poison per LangRef.
+// CHECK-LABEL: func @memref_load_neg_stride(
+func.func @memref_load_neg_stride(%base: memref<2000xf32>) -> f32 {
+ // CHECK: %[[OFF:.*]] = llvm.mul %{{.*}}, %{{.*}} overflow<nsw> : i64
+ // CHECK-NEXT: %[[PTR:.*]] = llvm.getelementptr inbounds %{{.*}}[%[[OFF]]] : (!llvm.ptr, i64) -> !llvm.ptr, f32
+ // CHECK-NEXT: llvm.load %[[PTR]] : !llvm.ptr -> f32
+ %flip = memref.reinterpret_cast %base to
+ offset: [1000], sizes: [100], strides: [-1]
+ : memref<2000xf32> to memref<100xf32, strided<[-1], offset: 1000>>
+ %c5 = arith.constant 5 : index
+ %v = memref.load %flip[%c5] : memref<100xf32, strided<[-1], offset: 1000>>
+ return %v : f32
+}
diff --git a/mlir/test/Conversion/MemRefToLLVM/expand-then-convert-to-llvm.mlir b/mlir/test/Conversion/MemRefToLLVM/expand-then-convert-to-llvm.mlir
index c2c93525b6509..eaa4c35b3c2bf 100644
--- a/mlir/test/Conversion/MemRefToLLVM/expand-then-convert-to-llvm.mlir
+++ b/mlir/test/Conversion/MemRefToLLVM/expand-then-convert-to-llvm.mlir
@@ -686,7 +686,7 @@ func.func @collapse_static_shape_with_non_identity_layout(%arg: memref<1x1x8x8xf
// CHECK: %[[OFFSET:.*]] = llvm.extractvalue %[[DESC]][2] : !llvm.struct<(ptr, ptr, i64, array<2 x i64>, array<2 x i64>)>
// CHECK: %[[BUFF_ADDR:.*]] = llvm.getelementptr %[[ALIGNED_PTR]][%[[OFFSET]]] : (!llvm.ptr, i64) -> !llvm.ptr, f32
// CHECK: llvm.intr.assume %{{.*}} ["align"(%[[BUFF_ADDR]], %{{.*}} : !llvm.ptr, i64)] : i1
-// CHECK: %[[LD_ADDR:.*]] = llvm.getelementptr inbounds|nuw %[[BUFF_ADDR]][%{{.*}}] : (!llvm.ptr, i64) -> !llvm.ptr, f32
+// CHECK: %[[LD_ADDR:.*]] = llvm.getelementptr inbounds %[[BUFF_ADDR]][%{{.*}}] : (!llvm.ptr, i64) -> !llvm.ptr, f32
// CHECK: %[[VAL:.*]] = llvm.load %[[LD_ADDR]] : !llvm.ptr -> f32
// CHECK: return %[[VAL]] : f32
func.func @load_and_assume(
More information about the Mlir-commits
mailing list