[PATCH] D156889: [mlir][cf] Add ControlFlow to SCF lifting pass

Markus Böck via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 8 04:41:31 PDT 2023

zero9178 added inline comments.

Comment at: mlir/lib/Transforms/Utils/CFGToSCF.cpp:1004
+  //                  +---+    |      +---+     |
+  //                  |ret+<---+---->++   ++<---+
+  //                  +---+          |     |
gysit wrote:
> ultra nit:  Any chance there is a way to illustrate a bit better that the return is part of region1? Otherwise the graphics are super nice!
I tried to move the `ret` block further up and moving the `Region 1` label to have about equal distance to both in the hopes of it looking clearer

Comment at: mlir/test/Conversion/ControlFlowToSCF/test.mlir:291
+// CHECK-NEXT:   %[[IF_RES:.*]]:3 = scf.if %[[CALL_PAIR]]#0
+// CHECK-NEXT:     scf.yield %[[POISON]], %[[C1]], %[[C0]]
+// CHECK-NEXT:   else
gysit wrote:
> I would have expected that we return %[[ARG1]] instead of %[[POISON]] here since this should be the result of the control flow loop if I understand correctly?
Good catch! This was actually a bug :) The `transformToReduceLoop` method incorrectly handled loop header block arguments, accidently causing uses of the loop header block arguments outside the loop to be replaced by its values at the end of the loop body. This should be fixed now.



More information about the llvm-commits mailing list