Skip to content

Conversation

@d0k
Copy link
Member

@d0k d0k commented Nov 24, 2025

OffsetSizeAndStrideOpInterface does not specify whether it's operating on the input or output shape and in fact different ops implement this in different ways, which is also why SubviewOp is special cased here.

This "marked as dynamic but not really dynamic" folding is better handled by shape inference, so just remove the bad fold.

OffsetSizeAndStrideOpInterface does not specify whether it's operating
on the input or output shape and in fact different ops implement this
in different ways, which is also why SubviewOp is special cased here.

This "marked as dynamic but not really dynamic" folding is better
handled by shape inference, so just remove the bad fold.
@llvmbot
Copy link
Member

llvmbot commented Nov 24, 2025

@llvm/pr-subscribers-mlir-memref

@llvm/pr-subscribers-mlir

Author: Benjamin Kramer (d0k)

Changes

OffsetSizeAndStrideOpInterface does not specify whether it's operating on the input or output shape and in fact different ops implement this in different ways, which is also why SubviewOp is special cased here.

This "marked as dynamic but not really dynamic" folding is better handled by shape inference, so just remove the bad fold.


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

2 Files Affected:

  • (modified) mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp (-7)
  • (modified) mlir/test/Dialect/MemRef/canonicalize.mlir (-13)
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 1c21a2f270da6..1035d7cb46e6e 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -1074,13 +1074,6 @@ OpFoldResult DimOp::fold(FoldAdaptor adaptor) {
     return subview.getDynamicSize(sourceIndex);
   }
 
-  if (auto sizeInterface =
-          dyn_cast_or_null<OffsetSizeAndStrideOpInterface>(definingOp)) {
-    assert(sizeInterface.isDynamicSize(unsignedIndex) &&
-           "Expected dynamic subview size");
-    return sizeInterface.getDynamicSize(unsignedIndex);
-  }
-
   // dim(memrefcast) -> dim
   if (succeeded(foldMemRefCast(*this)))
     return getResult();
diff --git a/mlir/test/Dialect/MemRef/canonicalize.mlir b/mlir/test/Dialect/MemRef/canonicalize.mlir
index 313090272ef90..e02717a2f5689 100644
--- a/mlir/test/Dialect/MemRef/canonicalize.mlir
+++ b/mlir/test/Dialect/MemRef/canonicalize.mlir
@@ -208,19 +208,6 @@ func.func @subview_negative_stride2(%arg0 : memref<7xf32>) -> memref<?xf32, stri
 
 // -----
 
-// CHECK-LABEL: func @dim_of_sized_view
-//  CHECK-SAME:   %{{[a-z0-9A-Z_]+}}: memref<?xi8>
-//  CHECK-SAME:   %[[SIZE:.[a-z0-9A-Z_]+]]: index
-//       CHECK:   return %[[SIZE]] : index
-func.func @dim_of_sized_view(%arg : memref<?xi8>, %size: index) -> index {
-  %c0 = arith.constant 0 : index
-  %0 = memref.reinterpret_cast %arg to offset: [0], sizes: [%size], strides: [1] : memref<?xi8> to memref<?xi8>
-  %1 = memref.dim %0, %c0 : memref<?xi8>
-  return %1 : index
-}
-
-// -----
-
 // CHECK-LABEL: func @no_fold_subview_negative_size
 //  CHECK:        %[[SUBVIEW:.+]] = memref.subview
 //  CHECK:        return %[[SUBVIEW]]

@d0k d0k merged commit 4650f85 into llvm:main Nov 24, 2025
13 checks passed
aadeshps-mcw pushed a commit to aadeshps-mcw/llvm-project that referenced this pull request Nov 26, 2025
…vm#169327)

OffsetSizeAndStrideOpInterface does not specify whether it's operating
on the input or output shape and in fact different ops implement this in
different ways, which is also why SubviewOp is special cased here.

This "marked as dynamic but not really dynamic" folding is better
handled by shape inference, so just remove the bad fold.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants