[PATCH] D41913: [LV] Don't call recordVectorLoopValueForInductionCast for newly-created IV from a trunc.

Matthew Simpson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 11 12:42:30 PST 2018


mssimpso added a comment.

Hi Dimitry,

This looks good to me, but it's probably a good idea to let Dorit have a look as well. Also, this should be a release blocker if it's not already. Thanks!



================
Comment at: llvm/test/Transforms/LoopVectorize/pr35773.ll:34-36
+; TODO: Not sure why these are generated, but this is a test for a crash, so leave as a TODO:
+; CHECK-NEXT:    [[BROADCAST_SPLATINSERT:%.*]] = insertelement <4 x i32> undef, i32 [[MAIN_IV]], i32 0
+; CHECK-NEXT:    [[BROADCAST_SPLAT:%.*]] = shufflevector <4 x i32> [[BROADCAST_SPLATINSERT]], <4 x i32> undef, <4 x i32> zeroinitializer
----------------
Currently, every induction variable gets vectorized in some way, even though we may know that the vector version will not be needed. So even if we know the induction variable is uniform, as %main.iv is in this case, we will still vectorize it with broadcast/splats, knowing that InstCombine will simplify it all. The vectorizer relies on InstCombine for a lot of cleanup, actually. But I think we're to the point where we don't need to do this, since we can build up vector values on-demand if needed (this wasn't always the case). You can add "-instcombine" to your run line here if you'd rather just check the cleaner output. This is done in many test cases.

For the TODO, it's probably better to add something like that in the code for greater visibility, instead of in the test case. The relevant piece of code is the block at the end of `widenIntOrFpInduction` that begins with `if (!VectorizedIV)`. This could be added as a separate patch I think.


https://reviews.llvm.org/D41913





More information about the llvm-commits mailing list