[PATCH] D26555: [LV] Keep predicated instructions in the same block

[Michael Kuperstein via llvm-commits]

mkuper added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:482
 
-  /// This instruction is un-vectorizable. Implement it as a sequence
-  /// of scalars. If \p IfPredicateInstr is true we need to 'hide' each
-  /// scalarized instruction behind an if block predicated on the control
-  /// dependence of the instruction.
-  virtual void scalarizeInstruction(Instruction *Instr,
-                                    bool IfPredicateInstr = false);
+  /// This instruction is un-vectorizable. Implement it as a sequence of
+  /// scalars.
----------------
As long as you're changing this comment - this is no longer only for "un-vectorizable" instructions.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4319
 
   // For each instruction I marked for predication on value C, split I into its
   // own basic block to form an if-then construct over C. Since I may be fed by
----------------
This changes now, right?


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4486
+        // corresponding to each instruction in ScalarLoopPredInsts.
+        VectorLoopPredInsts.clear();
+        for (Instruction *I : ScalarLoopPredInsts) {
----------------
Do we generally prefer this to defining the vector inside the loop, from a performance standpoint? (The latter would be clearer, I think).


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4488
+        for (Instruction *I : ScalarLoopPredInsts) {
+          if (Lane > 0 && Legal->isUniformAfterVectorization(I))
+            continue;
----------------
What happens in the "Lane == 0 && Legal->isUniformAfterVectorization(I)" case?
I'm having a bit of trouble imagining what the code ends up looking.


https://reviews.llvm.org/D26555