[PATCH] D35227: [LV] Don't allow outside uses of IVs if the SCEV is predicated on loop conditions

[Michael Kuperstein via Phabricator via llvm-commits]

mkuper added a comment.

Thanks, Gil!



================
Comment at: llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp:5321
+  // currently means re-using this SCEV outside the loop.
+  if (PSE.getUnionPredicate().isAlwaysTrue()) {
+    AllowedExit.insert(Phi);
----------------
gilr wrote:
> Can we narrow the check as documented above to only check if the phi's SCEV relies on predicates?
I don't think so - IIUC, once we've added the predicate to PSCEV, all further SCEV expressions we get from it can rely the same predicate, and I'm not sure there's a way to query whether a specific one did.


================
Comment at: llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp:5322
+  if (PSE.getUnionPredicate().isAlwaysTrue()) {
+    AllowedExit.insert(Phi);
+    AllowedExit.insert(Phi->getIncomingValueForBlock(TheLoop->getLoopLatch()));
----------------
gilr wrote:
> The phi node is presumably guaranteed not to overflow also on the last iteration, so should be correct as a live out, right?
Oh, yes, I think you're right - and this meshes well with what sanjoy told me!
Feel free to refine it.


================
Comment at: llvm/trunk/test/Transforms/LoopVectorize/pr33706.ll:8
+; CHECK-LABEL: @PR33706
+; CHECK-NOT: <2 x i32>
+define void @PR33706(float* nocapture readonly %arg, float* nocapture %arg1, i32 %arg2) local_unnamed_addr {
----------------
gilr wrote:
> It would be good to document the specific problem in vectorizing this loop, which is the live-out value %tmp20. When %arg2 == 1, the trip count is 65536 and %tmp20 coming out of the loop should be 0 (65536 & 65535), but currently LV pre-computes the live-out in the pre-header as 65536.
> Right?
> 
> Can the test be minimized?
Right. I thought PR33706 has sufficient details, and pointing to it was fine. I can add the explanation here if you want.

I actually spent a while trying to minimize the test, but it's pretty annoying - bugpoint minimizes to the wrong thing, because we can also vectorize similar things using first-order recurrences.


Repository:
  rL LLVM

https://reviews.llvm.org/D35227