[PATCH] D123541: [VPlan] Replace remaining use of needsScalarIV.

[Florian Hahn via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8189
 
   bool NeedsScalarIV = LoopVectorizationPlanner::getDecisionAndClampRange(
       [&](ElementCount VF) {
----------------
Ayal wrote:
> NeedsScalarIV is now dead?
Removed.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8201
       Range);
   bool NeedsScalarIVOnly = LoopVectorizationPlanner::getDecisionAndClampRange(
       [&](ElementCount VF) {
----------------
Ayal wrote:
> When/can NeedsScalarIVOnly also be discarded?
I think the main remaining difference is the handling of vector compares for predication, where we would now not generate vector phis. I'll take a look.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:3036
+/// \p Def.
+bool scalarsUsed(VPValue *Def);
+
----------------
Ayal wrote:
> nit: name is a bit cryptic, but a better alternative doesn't come to mind.
> Suffice to have a lambda next to the single user?
Inlined, thanks!


================
Comment at: llvm/test/Transforms/LoopVectorize/induction-multiple-uses-in-same-instruction.ll:25
+; CHECK-NEXT:    [[TMP7:%.*]] = icmp eq i64 [[INDEX_NEXT]], 100
+; CHECK-NEXT:    br i1 [[TMP7]], label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], !llvm.loop [[LOOP0:![0-9]+]]
 ; CHECK:       middle.block:
----------------
Ayal wrote:
> hmm, is this still considered to be vectorized by 2 and interleaved by 1, rather that the converse, contrary to the prescribed "-force-vector-width=2 -force-vector-interleave=1"
Yeah, now it's a bit clearer, the main intention of the test is to check a case where %iv has multiple uses in the same instruction.


================
Comment at: llvm/test/Transforms/LoopVectorize/single-value-blend-phis.ll:311
+; CHECK:       exit:
+; CHECK-NEXT:    ret void
 ;
----------------
Ayal wrote:
> Is it important to add above CHECKs for the scalar loop?
Removed!


================
Comment at: llvm/test/Transforms/LoopVectorize/single-value-blend-phis.ll:381
+; CHECK:       exit:
+; CHECK-NEXT:    ret void
 ;
----------------
Ayal wrote:
> ditto
Removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123541