[Mlir-commits] [mlir] a7d88a9 - [mlir] SCFToStandard: support any ops in and around the control flow ops

[Alex Zinenko]

Author: Alex Zinenko
Date: 2020-05-20T16:12:05+02:00
New Revision: a7d88a90386d6c4b456a68710ea48069b5d3a0bb

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

LOG: [mlir] SCFToStandard: support any ops in and around the control flow ops

Originally, the SCFToStandard conversion only declared Ops from the Standard
dialect as legal after conversion. This is undesirable as it would fail the
conversion if the SCF ops contained ops from any other dialect. Furthermore,
this would be problematic for progressive lowering of `scf.parallel` to
`scf.for` after `ensureRegionTerminator` is made aware of the pattern rewriting
infrastructure because it creates temporary `scf.yield` operations declared
illegal. Change the legalization target to declare any op other than `scf.for`,
`scf.if` and `scf.parallel` legal.

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

Added: 
    

Modified: 
    mlir/lib/Conversion/SCFToStandard/SCFToStandard.cpp
    mlir/test/Conversion/SCFToStandard/convert-to-cfg.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Conversion/SCFToStandard/SCFToStandard.cpp b/mlir/lib/Conversion/SCFToStandard/SCFToStandard.cpp
index 96fdaf3551f1..34ee48758e9e 100644
--- a/mlir/lib/Conversion/SCFToStandard/SCFToStandard.cpp
+++ b/mlir/lib/Conversion/SCFToStandard/SCFToStandard.cpp
@@ -407,8 +407,11 @@ void mlir::populateLoopToStdConversionPatterns(
 void SCFToStandardPass::runOnOperation() {
   OwningRewritePatternList patterns;
   populateLoopToStdConversionPatterns(patterns, &getContext());
+  // Configure conversion to lower out scf.for, scf.if and scf.parallel.
+  // Anything else is fine.
   ConversionTarget target(getContext());
-  target.addLegalDialect<StandardOpsDialect>();
+  target.addIllegalOp<scf::ForOp, scf::IfOp, scf::ParallelOp>();
+  target.markUnknownOpDynamicallyLegal([](Operation *) { return true; });
   if (failed(applyPartialConversion(getOperation(), target, patterns)))
     signalPassFailure();
 }

diff  --git a/mlir/test/Conversion/SCFToStandard/convert-to-cfg.mlir b/mlir/test/Conversion/SCFToStandard/convert-to-cfg.mlir
index d2dedf91bd51..e4fc2a9f202c 100644
--- a/mlir/test/Conversion/SCFToStandard/convert-to-cfg.mlir
+++ b/mlir/test/Conversion/SCFToStandard/convert-to-cfg.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt -convert-scf-to-std %s | FileCheck %s
+// RUN: mlir-opt -allow-unregistered-dialect -convert-scf-to-std %s | FileCheck %s
 
 // CHECK-LABEL: func @simple_std_for_loop(%{{.*}}: index, %{{.*}}: index, %{{.*}}: index) {
 //  CHECK-NEXT:  br ^bb1(%{{.*}} : index)
@@ -398,3 +398,17 @@ func @parallel_reduce_loop(%arg0 : index, %arg1 : index, %arg2 : index,
   }
   return %0#0, %0#1 : f32, i64
 }
+
+// Check that the conversion is not overly conservative wrt unknown ops, i.e.
+// that the presence of unknown ops does not prevent the conversion from being
+// applied.
+// CHECK-LABEL: @unknown_op_inside_loop
+func @unknown_op_inside_loop(%arg0: index, %arg1: index, %arg2: index) {
+  // CHECK-NOT: scf.for
+  scf.for %i = %arg0 to %arg1 step %arg2 {
+    // CHECK: unknown.op
+    "unknown.op"() : () -> ()
+    scf.yield
+  }
+  return
+}