All information is already available in VPlan. Note that there are some
test changes, because we now can correctly look through instructions
like truncates to analyze the actual users.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Note that there are some test changes, because we now can correctly look through instructions like truncates to analyze the actual users.
this should be faster(?) ... and in some cases help clarify that a loop was actually not "vectorized" - when vector loop body is free of any vector instructions.
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
8183–8184 | NeedsScalarIV is now dead? | |
8184 | When/can NeedsScalarIVOnly also be discarded? | |
llvm/lib/Transforms/Vectorize/VPlan.h | ||
2977 | nit: name is a bit cryptic, but a better alternative doesn't come to mind. | |
llvm/test/Transforms/LoopVectorize/induction-multiple-uses-in-same-instruction.ll | ||
25 | 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" | |
llvm/test/Transforms/LoopVectorize/single-value-blend-phis.ll | ||
287 | Is it important to add above CHECKs for the scalar loop? | |
332 | ditto |
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
8183–8184 | Removed. | |
8184 | 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. | |
llvm/lib/Transforms/Vectorize/VPlan.h | ||
2977 | Inlined, thanks! | |
llvm/test/Transforms/LoopVectorize/induction-multiple-uses-in-same-instruction.ll | ||
25 | 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. | |
llvm/test/Transforms/LoopVectorize/single-value-blend-phis.ll | ||
287 | Removed! | |
332 | Removed. |
Looks good to me, curious if it may lead to any regression...
Adding minor nits.
llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp | ||
---|---|---|
378 | nit (irrespective of this patch): would be good to have a more descriptive name, e.g., optimizeScalarInductions(). | |
382 | nit: a comment should explain that if no user of IV uses scalars, there's no need for an optimized scalar induction. | |
395–396 | nit: "Step" >> "Steps" |
NeedsScalarIV is now dead?