[PATCH] D12476: [LV] Refactor all runtime check emissions into helper functions.

[James Molloy via llvm-commits]

jmolloy closed this revision.
jmolloy added a comment.

r246634, with the IRBuilder move in r246635.

Thanks!


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2862
@@ +2861,3 @@
+  // jump to the scalar loop.
+  emitVectorLoopEnteredCheck(Lp, MiddleBlock);
+  // Generate the code to check that the strides we assumed to be one are really
----------------
mzolotukhin wrote:
> Following the logic from D12477 - should we jump to `ScalarPH` here too? (And in the following checks)
Yes; in fact D12477 does exactly that - D12477 is based upon this revision. I'd prefer to keep this revision NFC if that's alright, and keep the switch to ScalarPH in D12477.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2885-2886
@@ +2884,4 @@
+  // the new vector loop.
+  IRBuilder<> BypassBuilder(LoopBypassBlocks.back()->getTerminator());
+  setDebugLocFromInst(BypassBuilder,
+                      getDebugLocFromInstOrOperands(OldInduction));
----------------
mzolotukhin wrote:
> `BypassBuilder` is now only used in one place (where `Value *CRD` is built). Maybe we could define it closer to the actual use (or get rid of it entirely). If you prefer, that might be a separate patch too.
> And I still suspect that we're changing existing behavior here, because we might emit "cmp.zero" check in another block, but probably it's not a big deal. 
I have a patch in my rebase history to do exactly that, I just didn't bother putting it up for review because it's trivial. I'll commit it with this :)


Repository:
  rL LLVM

http://reviews.llvm.org/D12476