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

[Alex Zinenko via Phabricator via llvm-commits]

ftynse created this revision.
ftynse added reviewers: nicolasvasilache, herhut, pifon2a.
Herald added subscribers: llvm-commits, jurahul, Kayjukh, frgossen, grosul1, Joonsoo, stephenneuendorffer, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, antiagainst, shauheen, jpienaar, rriddle, mehdi_amini.
Herald added a project: LLVM.
ftynse added a child revision: D80138: [mlir] ensureRegionTerminator: take OpBuilder.

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.

Depends On D80136 <https://reviews.llvm.org/D80136>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80137

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


Index: mlir/test/Conversion/convert-to-cfg.mlir
===================================================================
--- mlir/test/Conversion/convert-to-cfg.mlir
+++ mlir/test/Conversion/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 @@
   }
   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
+}
Index: mlir/lib/Conversion/SCFToStandard/SCFToStandard.cpp
===================================================================
--- mlir/lib/Conversion/SCFToStandard/SCFToStandard.cpp
+++ mlir/lib/Conversion/SCFToStandard/SCFToStandard.cpp
@@ -407,8 +407,11 @@
 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();
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80137.264653.patch
Type: text/x-patch
Size: 1853 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200518/6ee7d86a/attachment.bin>