[PATCH] D80656: [mlir][SCF] Add utility to clone an scf.ForOp while appending new yield values.

[Uday Bondhugula via Phabricator via llvm-commits]

bondhugula added inline comments.


================
Comment at: mlir/lib/Transforms/Utils/LoopUtils.cpp:2541-2543
+  for (auto it : llvm::zip(loop.region().front().getArguments(),
+                           newLoop.region().front().getArguments().take_front(
+                               loop.region().front().getNumArguments())))
----------------
ftynse wrote:
> herhut wrote:
> > Would `bvm.map(loop.region().front().getArguments(), newLoop.region().front().getArguments().take_front(loop.region().front().getNumArguments()))` work?
> Could we make it a bit shorter by, e.g., caching the result of loop.region().front() in a variable loopBody?
Since there's only one region and one block, `region().front() -> getBody() / body()`. I'm surprised there isn't an accessor. For eg. spirv::LoopOp has a `body()`.


================
Comment at: mlir/lib/Transforms/Utils/LoopUtils.cpp:2560
+
+  // Perform `loop` results replacement if requested.
+  if (replaceLoopResults) {
----------------
Nit: Replace `loop`'s results if ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80656