[Mlir-commits] [mlir] 91b9c0c - [mlir] fix outdated assert in affine symbol verification

[Alex Zinenko]

Author: Alex Zinenko
Date: 2023-01-23T15:18:00Z
New Revision: 91b9c0cd6b9f9bae0753bf7e41188a55a5ee1dfb

URL: https://github.com/llvm/llvm-project/commit/91b9c0cd6b9f9bae0753bf7e41188a55a5ee1dfb
DIFF: https://github.com/llvm/llvm-project/commit/91b9c0cd6b9f9bae0753bf7e41188a55a5ee1dfb.diff

LOG: [mlir] fix outdated assert in affine symbol verification

The verification of affine value classification for symbols was
expecting, incorrectly, that the dimension operand of `memref.dim` was
being produced by a constant-like operation. This is legacy of the
dimension being an attribute originally, and was never updated after it
was switched to be an operation. Treat such cases conservatively and
classify the value as non-symbol.

A more advanced version could attempt to check that the value would be a
valid symbol for all possible values the dimension attribute could take,
but this does not seem immediately useful.

Fixes #59993.

Reviewed By: bondhugula

Differential Revision: https://reviews.llvm.org/D142204

Added: 
    

Modified: 
    mlir/lib/Dialect/Affine/IR/AffineOps.cpp
    mlir/test/Dialect/Affine/invalid.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index db76278b7322d..4481c147a8f8c 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -336,8 +336,11 @@ static bool isDimOpValidSymbol(ShapedDimOpInterface dimOp, Region *region) {
   // The dim op is also okay if its operand memref is a view/subview whose
   // corresponding size is a valid symbol.
   std::optional<int64_t> index = getConstantIntValue(dimOp.getDimension());
-  assert(index.has_value() &&
-         "expect only `dim` operations with a constant index");
+
+  // Be conservative if we can't understand the dimension.
+  if (!index.has_value())
+    return false;
+
   int64_t i = index.value();
   return TypeSwitch<Operation *, bool>(dimOp.getShapedValue().getDefiningOp())
       .Case<memref::ViewOp, memref::SubViewOp, memref::AllocOp>(

diff  --git a/mlir/test/Dialect/Affine/invalid.mlir b/mlir/test/Dialect/Affine/invalid.mlir
index 866aa4062f3a6..964e281b62537 100644
--- a/mlir/test/Dialect/Affine/invalid.mlir
+++ b/mlir/test/Dialect/Affine/invalid.mlir
@@ -501,3 +501,17 @@ func.func @delinearize(%idx: index, %basis0: index, %basis1 :index) {
   affine.delinearize_index %idx into () : index
   return
 }
+
+// -----
+
+func.func @dynamic_dimension_index() {
+  "unknown.region"() ({
+    %idx = "unknown.test"() : () -> (index)
+    %memref = "unknown.test"() : () -> memref<?x?xf32>
+    %dim = memref.dim %memref, %idx : memref<?x?xf32>
+    // expected-error @below {{op index must be a dimension or symbol identifier}}
+    affine.load %memref[%dim, %dim] : memref<?x?xf32>
+    "unknown.terminator"() : () -> ()
+  }) : () -> ()
+  return
+}