[PATCH] D73348: Add lowering for loop.parallel to cfg.

[Stephan Herhut via Phabricator via llvm-commits]

herhut added inline comments.


================
Comment at: mlir/lib/Conversion/LoopToStandard/ConvertLoopToStandard.cpp:276
+  for (auto &op : parallelOp.body().front().without_terminator()) {
+    rewriter.clone(op, mapping);
+  }
----------------
ftynse wrote:
> Have you considered moving the region instead of cloning (`inlineRegionBefore`) and then calling `replaceAllUsesWith` to update induction variables?
I did not want to rewrite the block arguments. In essence, I would need the equivalent of a signature conversion and that seemed involved. The issue here is that the block of the innermost loop only declares one induction variable. 


================
Comment at: mlir/test/Conversion/convert-to-cfg.mlir:174
+
+func @std_parallel_loop(%arg0 : index, %arg1 : index, %arg2 : index,
+                        %arg3 : index, %arg4 : index) {
----------------
ftynse wrote:
> `std` looks like leftover from the temporary state when loops where in the standard dialect, let's drop it here (and later clean up the tests above)
I will clean it up and also move this test to LoopsToCfg as that test feels a bit misplaced.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73348/new/

https://reviews.llvm.org/D73348