[PATCH] D78287: [VPlan] Clean up tryToCreate(Widen)Recipe. (NFC)

[Ayal Zaks via Phabricator via llvm-commits]

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

Thanks for following up on this! Looks good to me with a couple of nits.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6867
     return new VPWidenIntOrFpInductionRecipe(cast<PHINode>(I->getOperand(0)),
                                              cast<TruncInst>(I));
   return nullptr;
----------------
No need to cast I into TruncInst.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6892
   }
   return new VPBlendRecipe(Phi, Operands);
 }
----------------
Orthogonal thought: perhaps if the constructor of VPBlendRecipe would receive two iterator_ranges, one for the values and one for the masks, and fuse them itself, tryToBlend() (or createBlendRecipe()) could be simplified using map_ranges.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7085
+
+    return new VPWidenPHIRecipe(Phi);
   }
----------------
It would be clearer to further refactor the handling of Phi's into

```
  if (Phi->getParent() != OrigLoop->getHeader())
    return tryToBlend(Phi, Plan); // Rename to createBlendRecipe()?
  if (Recipe = tryToOptimizeInductionPHI(Phi))
    return Recipe;
  return new VPWidenPHIRecipe(Phi); // Rename to VPWidenHeaderPHIRecipe()?
```


================
Comment at: llvm/lib/Transforms/Vectorize/VPRecipeBuilder.h:108
   /// may be decreased to ensure same decision from \p Range.Start to
   /// \p Range.End.
+  VPWidenIntOrFpInductionRecipe *tryToOptimizeInductionPHI(PHINode *Phi);
----------------
Above comment needs updating.


================
Comment at: llvm/lib/Transforms/Vectorize/VPRecipeBuilder.h:121
+
+  /// Handle call instructions. If \p CI is can be widened for \p Range.Start,
+  /// return a new VPWidenCallRecipe. Range.End may be decreased to ensure same
----------------
nit: [is] can


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78287