[PATCH] D115111: [VPlan] Add InductionDescriptor to VPWidenIntOrFpInduction. (NFC)

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 3 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8591
+  auto I = Legal->getInductionVars().find(Phi);
+  if (I == Legal->getInductionVars().end())
+    return nullptr;
----------------
Ayal wrote:
> This change from lookup() to find() is done in order to obtain a reference to an existing InductionDescriptor to be recorded in VPWidenIntOrFpInduction recipe, rather than a copy thereof. Right?
> 
> Can alternatively do
> 
> ```
>   if (!Legal->isInductionPhi(Phi))
>     return nullptr;
>   InductionDescriptor &II = Legal->getInductionVars().find(Phi)->second;
> 
> ```
I went ahead and added a `getIntOrFpInductionDescriptor` helper as suggested below.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:64
 class VPlanSlp;
+class InductionDescriptor;
 
----------------
Ayal wrote:
> Lex order
Moved to correct position.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1067
+  /// Returns to induction descriptor for the recipe.
+  InductionDescriptor &getDescriptor() { return IndDesc; }
 };
----------------
Ayal wrote:
> `IndDesc` >> `Descriptor`?
> Or, to better match VPReductionPHIRecipe, `getDescriptor()` >> `getInductionDescriptor()`?
updated to `getInductionDescriptor`.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:48
+        auto II = Inductions.find(Phi);
+        if (II != Inductions.end() &&
+            (II->second.getKind() == InductionDescriptor::IK_IntInduction ||
----------------
Ayal wrote:
> A similar case, albeit w/o direct access to Legal.
> Should Legal support all above cases by providing something like:
> 
> ```
> Optional<InductionDescriptor *> getInductionDescriptor(PHINode *Phi) {
>   if (!isInductionPhi(Phi))
>     return None;
>   return &getInductionVars().find(Phi)->second;
> }
> 
> ```
> or even
> 
> ```
> Optional<InductionDescriptor *> getIntOrFpInductionDescriptor(PHINode *Phi) {
>   if (!isInductionPhi(Phi))
>     return None;
>   auto &ID = getInductionVars().find(Phi)->second;
>   if (ID.getKind() == InductionDescriptor::IK_IntInduction ||
>       ID.getKind() == InductionDescriptor::IK_FpInduction)
>     return &ID;
>   return None;
> }
> 
> ```
> ?
I went ahead and added a `getIntOrFpInductionDescriptor` helper. It is used in LV and here. `VPInstructionsToVPRecipes` has been updated to take a function_ref to get use it. That should reduce duplication to a minimum


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115111