[mlir] fix outdated assert in affine symbol verification

[ftynse]

Summary

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.

[bondhugula]

LGTM.

mlir/lib/Dialect/Affine/IR/AffineOps.cpp

341

Sounds fine. Thanks for fixing this.

345–346

I just noticed something more to fix/generalize here. Instead of listing out ViewOp, SubViewOp, we need to have ViewLikeOpInterface and AllocLikeOpInterface here.

[bondhugula]

mlir/lib/Dialect/Affine/IR/AffineOps.cpp

345–346

I can fix this part in another revision.

[ftynse]

mlir/lib/Dialect/Affine/IR/AffineOps.cpp

345–346

It would be nice, thanks!

mlir/lib/Dialect/Affine/IR/AffineOps.cpp

static bool isDimOpValidSymbol(ShapedDimOpInterface dimOp, Region *region) {
   // Conservatively handle remaining BlockArguments as non-valid symbols.
   // E.g. scf.for iterArgs.
   if (dimOp.getShapedValue().template isa<BlockArgument>())
     return false;
 
   // 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>(
           [&](auto op) { return isMemRefSizeValidSymbol(op, i, region); })
       .Default([](Operation *) { return false; });
 }
 
 // A value can be used as a symbol (at all its use sites) iff it meets one of
 // the following conditions:
 // *) It is a constant.
 // *) Its defining op or block arg appearance is immediately enclosed by an op

mlir/test/Dialect/Affine/invalid.mlir

 
 // -----
 
 func.func @delinearize(%idx: index, %basis0: index, %basis1 :index) {
   // expected-error@+1 {{'affine.delinearize_index' op basis should not be empty}}
   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
+}