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

[Michael Zolotukhin via llvm-commits]

mzolotukhin added a comment.

Hi James,

The patch LGTM, some comments inline.


================
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
----------------
Following the logic from D12477 - should we jump to `ScalarPH` here too? (And in the following checks)

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2885-2886
@@ +2884,4 @@
+  // the new vector loop.
+  IRBuilder<> BypassBuilder(LoopBypassBlocks.back()->getTerminator());
+  setDebugLocFromInst(BypassBuilder,
+                      getDebugLocFromInstOrOperands(OldInduction));
----------------
`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. 


Repository:
  rL LLVM

http://reviews.llvm.org/D12476