[PATCH] D88819: [LV] Support for Remainder loop vectorization

[Vinay Madhusudan via Phabricator via llvm-commits]

mivnay added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3383
+
+         [ ] <-- original loop iteratiib check.
+      /   |
----------------
bmahjour wrote:
> It's extremely hard to "draw" this diagram in text. It's even harder to read it. I think we should create a documentation section under https://llvm.org/docs/Vectorizers.html#loop-vectorizer and upload an image. The link can then be put into the comment for people to view and understand what is being generated.
Sure. I can do it once this patch goes through.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5636
+LoopVectorizationCostModel::selectEpilogVectorizationFactor(unsigned MaxVF) {
+  // Find next VF.
+  unsigned VF = 1;
----------------
bmahjour wrote:
> The vector epilogue loop's VF need not be smaller than the VF of the original loop for it to be profitable. For example with large interleave counts there may still be significant number of iterations to be executed and the throughput would be affected if a VF is chosen that is smaller than the widest profitable VF.
Currently, it is tuned as per SPEC CPU 2017 benchmarks. It can be fine tuned based on the further data. 


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5639
+  unsigned int Cost = 0;
+  bool HasMultipleIndvars = (Legal->getInductionVars().size() != 1) ||
+                            (Legal->getReductionVars().size() > 0);
----------------
bmahjour wrote:
> why this limitation?
There were some issues with the Resume values when multiple induction variables are involved. I am planning to handle it later.


================
Comment at: test/Transforms/LoopVectorize/X86/invariant-store-vectorization.ll:238
 ; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[B1:%.*]] = bitcast i32* [[B:%.*]] to i8*
+; CHECK-NEXT:    [[A4:%.*]] = bitcast i32* [[A:%.*]] to i8*
----------------
bmahjour wrote:
> why these casts are hoisted?
Note that the tests are auto generated using update_test_checks. It is done inside loop vectorize as there are redundant casts now in epilog vector I guess.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88819