[PATCH] D122096: [VPlan] Expand induction step in VPlan pre-header.

[Ayal Zaks via Phabricator via llvm-commits]

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

This looks good to me, thanks for accommodating, ship it!
Adding minor nits.



================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:1760
+  if (auto *E = dyn_cast<SCEVConstant>(Expr)) {
+    Step = Plan.getOrAddExternalDef(E->getValue());
+  } else if (auto *E = dyn_cast<SCEVUnknown>(Expr)) {
----------------
nit: can return Plan.getOrAddExternalDef(E->getValue()); can eliminate brackets?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:1762
+  } else if (auto *E = dyn_cast<SCEVUnknown>(Expr)) {
+    Step = Plan.getOrAddExternalDef(E->getValue());
+  } else {
----------------
ditto.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:455
+
+void VPlanTransforms::optimizeExpandSCEVRecipes(VPlan &Plan) {
+  DenseMap<const SCEV *, VPValue *> SCEV2VPV;
----------------
nit: perhaps "eliminateCommonExpandSCEVRecipes()" or "removeRedundantExpandSCEVRecipes()" would be a more descriptive name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122096