[Mlir-commits] [mlir] ab86b8c - [mlir][linalg][transform] Fix printing of TileToForall in edge case.

[Ingo Müller]

Author: Ingo Müller
Date: 2023-07-12T13:30:15Z
New Revision: ab86b8cef4b04c6d078e9533353d16ac1843fc40

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

LOG: [mlir][linalg][transform] Fix printing of TileToForall in edge case.

The `static_(num_threads|tile_sizes)` attributes of this op are
`DefaultValuedOptionalAttr`s, so they can be constructed *without* such
an attribute. In other words, the following is a valid op (note the
absense of the `static_num_threads` attribute):

"builtin.module"() ({
  "transform.sequence"() <{failure_propagation_mode = 1 : i32, operand_segment_sizes = array<i32: 0, 0>}> ({
  ^bb0(%arg0: !pdl.operation, %arg1: !transform.op<"linalg.matmul">, %arg2: !transform.op<"linalg.elemwise_binary">):
    %0 = "transform.structured.match"(%arg0) <{ops = ["test.dummy"]}> : (!pdl.operation) -> !pdl.operation
    %1:2 = "transform.structured.tile_to_forall_op"(%arg1, %0) <{operand_segment_sizes = array<i32: 1, 0, 0, 0, 1>}> : (!transform.op<"linalg.matmul">, !pdl.operation) -> (!transform.op<"scf.forall">, !transform.op<"linalg.matmul">)
    "transform.yield"() : () -> ()
  }) : () -> ()
}) : () -> ()

However, the custom printing directive converted those to an `ArrayRef`,
which crashes if done on an empty `ArrayAttr`. This patch changes the
signature such that no automatic conversion takes place and extends the
test to test for existinnce of the attribute.

Reviewed By: nicolasvasilache

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

Added: 
    

Modified: 
    mlir/include/mlir/Dialect/Transform/Utils/Utils.h
    mlir/lib/Dialect/Transform/Utils/Utils.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Dialect/Transform/Utils/Utils.h b/mlir/include/mlir/Dialect/Transform/Utils/Utils.h
index 97b193bc723d43..868054e5e2aef4 100644
--- a/mlir/include/mlir/Dialect/Transform/Utils/Utils.h
+++ b/mlir/include/mlir/Dialect/Transform/Utils/Utils.h
@@ -36,7 +36,7 @@ class TransformState;
 void printPackedOrDynamicIndexList(OpAsmPrinter &printer, Operation *op,
                                    Value packed, Type packedType,
                                    OperandRange values, TypeRange valueTypes,
-                                   ArrayRef<int64_t> integers);
+                                   DenseI64ArrayAttr integers);
 
 /// Parser hook for custom directive in assemblyFormat.
 ///

diff  --git a/mlir/lib/Dialect/Transform/Utils/Utils.cpp b/mlir/lib/Dialect/Transform/Utils/Utils.cpp
index d516a56feed478..08068d285b4c24 100644
--- a/mlir/lib/Dialect/Transform/Utils/Utils.cpp
+++ b/mlir/lib/Dialect/Transform/Utils/Utils.cpp
@@ -16,9 +16,10 @@ using namespace mlir::transform;
 
 void mlir::transform::printPackedOrDynamicIndexList(
     OpAsmPrinter &printer, Operation *op, Value packed, Type packedType,
-    OperandRange values, TypeRange valueTypes, ArrayRef<int64_t> integers) {
+    OperandRange values, TypeRange valueTypes, DenseI64ArrayAttr integers) {
   if (packed) {
-    assert(values.empty() && integers.empty() && "expected no values/integers");
+    assert(values.empty() && (!integers || integers.empty()) &&
+           "expected no values/integers");
     printer << "*(" << packed << " : " << packedType << ")";
     return;
   }