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

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache marked 15 inline comments as done.
nicolasvasilache added inline comments.


================
Comment at: mlir/lib/Transforms/Utils/LoopUtils.cpp:2534
+  yieldOperands.append(newYieldedValues.begin(), newYieldedValues.end());
+  auto newYield = b.create<scf::YieldOp>(yield.getLoc(), yieldOperands);
+
----------------
herhut wrote:
> Why not create this in the right block to start with?
Because I would need to manually replace the values, so I just let the clone do it then erase the op.
If you have strong feelings about this I can change to the other form.


================
Comment at: mlir/lib/Transforms/Utils/LoopUtils.cpp:2567
+
+  newYield.erase();
+  return newLoop;
----------------
bondhugula wrote:
> ftynse wrote:
> > This is not allowed if OpBuilder is a ConversionRewriter, use b.eraseOp instead
> Hmm... but there is no OpBuilder::eraseOp; it only exists in PatternRewriters. 
added one, let's see what @rriddle says


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80656