[PATCH] D97711: [VPlan] Introduce VPWidenIntOrFpInductionRecipes to original plan.

[Andrei Elovikov via Phabricator via llvm-commits]

a.elovikov added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8763
+    // created here.
+    for (auto I = OriginalVPBB->begin(), E = OriginalVPBB->end(); I != E;) {
+      VPRecipeBase *Ingredient = &*I++;
----------------
nit: can we use `make_early_inc_range` here isntead?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8774
+          Value *UV = NewVPV->getUnderlyingValue();
+          VPValue *OldVPV = OriginalPlan.getVPValue(UV);
+          OldVPV->replaceAllUsesWith(NewVPV);
----------------
Are these modifications to OriginalPlan expected to be done in all the transformations that replace the recipes? If so, I'm afraid that it would be error-prone and would decrease readability of the code.

I'm not familiar with the code to understand why it's needed (yet) so my question might not make much sense, but still. Is it possible we can "hide" those details by making a higher-level interface? Or are we expecting that such maintenance wouldn't be needed once we uplift VPlan to full VPValue-based/more independent of the original LLVM IR?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97711