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

[Ayal Zaks via Phabricator via llvm-commits]

Ayal 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;
----------------
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;

```


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8627
+    InductionDescriptor &II =
+        Legal->getInductionVars().find(cast<PHINode>(I->getOperand(0)))->second;
     VPValue *Start = Plan.getOrAddVPValue(II.getStartValue());
----------------
This is a similar setting of II as suggested above, but where checking isInductionPhi() is redundant.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:64
 class VPlanSlp;
+class InductionDescriptor;
 
----------------
Lex order


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

```
?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115111