[mlir][Linalg] Post submit addressed comments missed in f0cdc5bcd3f25192f12bfaff072ce02497b59c3c

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

Author: nicolasvasilache

mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.td

@@ -767,9 +767,17 @@ def VectorizeOp : Op<Transform_Dialect, "structured.vectorize",
     Note that this transformation is invalidating the handles to any payload IR
     operation that is contained inside the vectorization target.
 
-    `disable_multi_reduction_to_contract_patterns` and 
-    `disable_transfer_permutation_map_lowering_patterns` limits the power of 
-    vectorization. They are currently intended for testing purposes.
+    This transformation supports the following attributes:
+      - `vectorize_padding`: a UnitAttr to activate the vectorization of 
+      `tensor.pad` ops. Different pipelines may prefer to lower such ops to 
+      loops.
+      - `disable_multi_reduction_to_contract_patterns`: a UnitAttr to deactivate
+      the rewrite of `vector.multi_reduction` to `vector.contract`. This is 
+      intended to be used in tests only.
+      - `disable_transfer_permutation_map_lowering_patterns`: a UnitAttr to
+      deactivate the rewrite of `vector.transfer` with permutation maps into
+      explicit `vector.transpose` operations. This is intended to be used in
+      tests only but may be promotoed to a first class attribute in the future.
 
     #### Return modes:
 
@@ -780,13 +788,12 @@ def VectorizeOp : Op<Transform_Dialect, "structured.vectorize",
   }];
 
   let arguments = (ins PDL_Operation:$target,
-                   DefaultValuedAttr<BoolAttr, "false">:$vectorize_padding,                   
-                   DefaultValuedAttr<BoolAttr, "false">:$disable_multi_reduction_to_contract_patterns,                   
-                   DefaultValuedAttr<BoolAttr, "false">:$disable_transfer_permutation_map_lowering_patterns);
+                   UnitAttr:$vectorize_padding,                   
+                   UnitAttr:$disable_multi_reduction_to_contract_patterns,                   
+                   UnitAttr:$disable_transfer_permutation_map_lowering_patterns);
   let results = (outs PDL_Operation:$transformed);
 
   let assemblyFormat = "$target attr-dict";
-
   let extraClassDeclaration = [{
     ::mlir::DiagnosedSilenceableFailure applyToOne(
         ::mlir::Operation *target, 

mlir/python/mlir/dialects/_structured_transform_ops_ext.py

@@ -287,7 +287,7 @@ def __init__(self,
                ip=None):
     pdl_operation_type = pdl.OperationType.get()
     if isinstance(vectorize_padding, bool):
-      vectorize_padding = BoolAttr.get(vectorize_padding)
+      vectorize_padding = UnitAttr.get()
     super().__init__(
         pdl_operation_type,
         _get_op_result_or_value(target),

mlir/test/Dialect/Linalg/transform-op-vectorize.mlir

@@ -130,7 +130,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.matmul"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1 {vectorize_padding = true}
+    %2 = transform.structured.vectorize %1 {vectorize_padding}
   }
 }
 

mlir/test/Dialect/Linalg/vectorization.mlir

@@ -18,7 +18,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.dot"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns = true }
+    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns }
   }
 }
 
@@ -40,7 +40,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.matvec"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns = true }
+    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns }
   }
 }
 
@@ -61,7 +61,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.matmul"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns = true }
+    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns }
   }
 }
 
@@ -83,7 +83,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.batch_matmul"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns = true }
+    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns }
   }
 }
 
@@ -126,7 +126,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.generic"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns = true, disable_transfer_permutation_map_lowering_patterns = true }
+    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns, disable_transfer_permutation_map_lowering_patterns }
   }
 }
 
@@ -169,7 +169,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.generic"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns = true, disable_transfer_permutation_map_lowering_patterns = true }
+    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns, disable_transfer_permutation_map_lowering_patterns }
   }
 }
 
@@ -199,7 +199,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.generic"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns = true, disable_transfer_permutation_map_lowering_patterns = true }
+    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns, disable_transfer_permutation_map_lowering_patterns }
   }
 }
 
@@ -242,7 +242,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.generic"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns = true, disable_transfer_permutation_map_lowering_patterns = true }
+    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns, disable_transfer_permutation_map_lowering_patterns }
   }
 }
 
@@ -265,7 +265,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.matmul"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns = true }
+    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns }
   }
 }
 
@@ -553,7 +553,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.generic"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  {disable_transfer_permutation_map_lowering_patterns = true }
+    %2 = transform.structured.vectorize %1  { disable_transfer_permutation_map_lowering_patterns }
   }
 }
 
@@ -647,7 +647,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.generic"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  { disable_transfer_permutation_map_lowering_patterns = true }
+    %2 = transform.structured.vectorize %1 { disable_transfer_permutation_map_lowering_patterns }
   }
 }
 
@@ -694,7 +694,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.generic"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  {disable_transfer_permutation_map_lowering_patterns = true }
+    %2 = transform.structured.vectorize %1  { disable_transfer_permutation_map_lowering_patterns }
   }
 }
 
@@ -740,7 +740,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.generic"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  {disable_transfer_permutation_map_lowering_patterns = true }
+    %2 = transform.structured.vectorize %1  { disable_transfer_permutation_map_lowering_patterns }
   }
 }
 
@@ -775,7 +775,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.matmul"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns = true, disable_transfer_permutation_map_lowering_patterns = true }
+    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns, disable_transfer_permutation_map_lowering_patterns }
   }
 }
 
@@ -807,7 +807,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["tensor.pad"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1 { vectorize_padding = true }
+    %2 = transform.structured.vectorize %1 { vectorize_padding }
   }
 }
 
@@ -839,7 +839,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["tensor.pad"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  { vectorize_padding = true }
+    %2 = transform.structured.vectorize %1  { vectorize_padding }
   }
 }
 
@@ -879,7 +879,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["tensor.pad"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  { vectorize_padding = true }
+    %2 = transform.structured.vectorize %1  { vectorize_padding }
   }
 }
 
@@ -913,7 +913,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["tensor.pad"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1 { vectorize_padding = true }
+    %2 = transform.structured.vectorize %1 { vectorize_padding }
   }
 }
 
@@ -949,7 +949,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %3 = transform.structured.match ops{["tensor.pad"]} in %arg1
     %4 = get_closest_isolated_parent %3
-    %5 = transform.structured.vectorize %4  { vectorize_padding = true }
+    %5 = transform.structured.vectorize %4  { vectorize_padding }
   }
 }
 
@@ -989,7 +989,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %3 = transform.structured.match ops{["tensor.pad"]} in %arg1
     %4 = get_closest_isolated_parent %3
-    %5 = transform.structured.vectorize %4 { vectorize_padding = true }
+    %5 = transform.structured.vectorize %4 { vectorize_padding }
   }
 }
 
@@ -1026,7 +1026,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %3 = transform.structured.match ops{["tensor.pad"]} in %arg1
     %4 = get_closest_isolated_parent %3
-    %5 = transform.structured.vectorize %4  { vectorize_padding = true }
+    %5 = transform.structured.vectorize %4  { vectorize_padding }
   }
 }
 
@@ -1097,7 +1097,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %3 = transform.structured.match ops{["tensor.pad"]} in %arg1
     %4 = get_closest_isolated_parent %3
-    %5 = transform.structured.vectorize %4  { vectorize_padding = true }
+    %5 = transform.structured.vectorize %4  { vectorize_padding }
   }
 }
 
@@ -1183,7 +1183,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %3 = transform.structured.match ops{["linalg.generic"]} in %arg1
     %4 = get_closest_isolated_parent %3
-    %5 = transform.structured.vectorize %4  { disable_multi_reduction_to_contract_patterns = true, disable_transfer_permutation_map_lowering_patterns = true }
+    %5 = transform.structured.vectorize %4  { disable_multi_reduction_to_contract_patterns, disable_transfer_permutation_map_lowering_patterns }
   }
 }
 
@@ -1216,7 +1216,7 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %3 = transform.structured.match ops{["linalg.generic"]} in %arg1
     %4 = get_closest_isolated_parent %3
-    %5 = transform.structured.vectorize %4 { vectorize_padding = true }
+    %5 = transform.structured.vectorize %4 { vectorize_padding }
   }
 }
 
@@ -1586,6 +1586,6 @@ transform.with_pdl_patterns {
   ^bb1(%arg1: !pdl.operation):
     %0 = transform.structured.match ops{["linalg.generic"]} in %arg1
     %1 = get_closest_isolated_parent %0
-    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns = true, disable_transfer_permutation_map_lowering_patterns = true }
+    %2 = transform.structured.vectorize %1  { disable_multi_reduction_to_contract_patterns, disable_transfer_permutation_map_lowering_patterns }
   }
-}
+}

mlir/test/python/dialects/transform_structured_ext.py

@@ -178,4 +178,4 @@ def testVectorize():
   # CHECK-LABEL: TEST: testVectorize
   # CHECK: transform.sequence
   # CHECK: = transform.structured.vectorize
-  # CHECK: vectorize_padding = true
+  # CHECK: {vectorize_padding}