[PATCH] D81416: [LV] Interleave to expose ILP for small loops with scalar reductions.

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added a comment.

Given that interleaving alone (without vectorization) can still cause major performance improvements shows that this is really an interleaving profitability issue, so the heuristic and the option seem ok to me. To enable the option by default we would need more testing on other platforms. That can be done in a separate patch.

Other than the minor comments I've left in the code, the changes look good to me. I'll approve once they are addressed unless of course there are objections or more comments from the reviewers.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:253
+    "interleave-small-loop-scalar-reduction", cl::init(false), cl::Hidden,
+    cl::desc("Enable interleaving for small loops with scalar reductions "
+             "to expose ILP."));
----------------
small loops with scalar reduction-> loops with small iteration counts that contain scalar reductions


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5400
+  const bool HasReductions = !Legal->getReductionVars().empty();
+  const bool ScalarReductionCond =
+      InterleaveSmallLoopScalarReduction && HasReductions && VF == 1;
----------------
Please remove `ScalarReductionCond` and just use the conditions directly in the if statement below. Please also update the comments to remove references to this variable.


================
Comment at: llvm/test/Transforms/LoopVectorize/PowerPC/interleave_IC.ll:61
+define dso_local void @test(%0* %arg, %17* dereferenceable(88) %arg1) comdat align 2 {
+  %tpm14 = getelementptr %0, %0* %arg, i64 0, i32 0, i32 3, i32 0, i32 0, i32 0
+  %tpm15 = load i32**, i32*** %tpm14, align 8
----------------
[nit] add `bb:`
%bb apears in the comment on line 66.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81416/new/

https://reviews.llvm.org/D81416