[PATCH] D147965: [LV] Use VPValue to get expanded value for SCEV step expressions.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

Push unsubmitted comments



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationPlanner.h:316
+  /// a temporary workaround needed due to the current epilogue
+  /// handling.workaround needed due to the current epilogue handling.
+  DenseMap<const SCEV *, Value *> executePlan(ElementCount VF, unsigned UF,
----------------
Ayal wrote:
> Typo above.
Fixed in the committed version, thanks!


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:502
   /// epilogue vectorization, this function is overriden to handle the more
-  /// complex control flow around the loops.
-  virtual std::pair<BasicBlock *, Value *> createVectorizedLoopSkeleton();
+  /// complex control flow around the loops.  \p ExpandedSCEVs it is used to
+  /// look up SCEV expansions for expressions needed during skeleton creation.
----------------
Ayal wrote:
> 
Fixed in the committed version, thanks!



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:563
+  /// left off. \p Step is the SCEV-expanded induction step to use. In cases
+  /// where the loop skeleton is more complicated (eg. epilogue vectorization)
+  /// and the resume values can come from an additional bypass block, the \p
----------------
Ayal wrote:
> 
Fixed in the committed version, thanks!



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8914
       *PSE.getSE());
+
   VPBasicBlock *HeaderVPBB = new VPBasicBlock("vector.body");
----------------
Ayal wrote:
> nit: redundant empty line?
Fixed in the committed version, thanks!



================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:357
+  /// Returns the value corresponding to the expansion of \p Expr.
+  Value *getExpandedSCEV(const SCEV *Expr) const;
+
----------------
Ayal wrote:
> Uncalled for now?
Removed in the committed version, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147965