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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added reviewers: gilr, rengolin, Ayal.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8774
+          Value *UV = NewVPV->getUnderlyingValue();
+          VPValue *OldVPV = OriginalPlan.getVPValue(UV);
+          OldVPV->replaceAllUsesWith(NewVPV);
----------------
a.elovikov wrote:
> 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?
> 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.

Yes, unfortunately there are still a few places that rely on accessing the VPValue via the underlying value, mostly to fix up phis after executing the VPlan. The plan is to update those places to deal with recipes directly, hence not having to access the VPValue through the underlying value. 

This means at the moment, the code is a bit bigger than it will be in the end but it is just temporary so we don't have to block progress in multiple directions. For example, D99293 removes the need to use `getVPValue` with an underlying IR value when fixing reduction PHIs (except for the value for the loop-exit instruction).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97711