[PATCH] D84683: [VPlan] Use VPValue def for VPWidenGEPRecipe.

[Dave Green via Phabricator via llvm-commits]

dmgreen accepted this revision.
dmgreen added a comment.
This revision is now accepted and ready to land.

LGTM along with the others



================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:127-128
     return V;
+  if (auto *V = dyn_cast<VPWidenGEPRecipe>(this))
+    return V;
   return nullptr;
----------------
Lets try and remove this again sooner rather than later ;)


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:910
                    Loop *OrigLoop)
-      : VPRecipeBase(VPWidenGEPSC), VPUser(Operands), GEP(GEP),
-        IsIndexLoopInvariant(GEP->getNumIndices(), false) {
+      : VPRecipeBase(VPWidenGEPSC), VPValue(VPValue::VPVWidenGEPSC, GEP),
+        VPUser(Operands), IsIndexLoopInvariant(GEP->getNumIndices(), false) {
----------------
-> VPRecipeBase::VPWidenGEPSC


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:907
+
+  GetElementPtrInst *getUnderlyingInstruction() {
+    return cast<GetElementPtrInst>(getUnderlyingValue());
----------------
fhahn wrote:
> dmgreen wrote:
> > Should this be called getUnderlyingGEP? Or should they standardize on getUnderlyingInstruction? If so I might recommend getUnderlyingInstr at least to cut down the lengths of the lines, without losing much by way of readability.
> I think it's probably easiest to standardize on `getUnderlyingInstr`. I updated D84680 to add `getUnderlyingInstr` to `VPRecipeBase`, which allows us to get rid of the code here. The GEP pointer (vs the Instruction ptr) is only needed once, and most recipes are similar in that respect.
Yeah OK. One cast isn't too bad.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84683