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

Andrei Elovikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 11 13:35:43 PST 2018


a.elovikov added a comment.

In https://reviews.llvm.org/D41913#973706, @mssimpso wrote:

> Also, this should be a release blocker if it's not already. Thanks!


Does it mean that I'll be required to commit the fix into some other branch after it lands to trunk?



================
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
----------------
mssimpso wrote:
> 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.
instcombine changes the output way too much, IMO. I'd rather keep the RUN line and checks as is because they describe what exactly the vectorizer is doing.  I'll remove the "TODO" in such case.

Another possibility is to add "-instcombine" but don't run FileCheck at all - just verify that "opt" is not crashing. Not sure if that's common in the tests though.


https://reviews.llvm.org/D41913





More information about the llvm-commits mailing list