[PATCH] D27919: [Loop Vectorizer] Interleave vs Gather - in some cases Gather is better.

[Elena Demikhovsky via Phabricator via llvm-commits]

delena marked 5 inline comments as done.
delena added inline comments.


================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:1929
+    auto UniformsPerVF = Uniforms.find(VF);
+    return UniformsPerVF->second.count(I);
+  }
----------------
mkuper wrote:
> Why not just "return Uniforms[VF]->second.count(I);"? I don't think the verbosity helps here, and we don't actually care about the difference between find() and operator[] due to the assert.
> Or, to save a lookup in an asserts build, you could find() and then assert on the result of the find().
The "const" qualifier does not allow the Uniforms[VF] form.


================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:2120
+    if (VF == 1 || Uniforms.count(VF))
+      return;
+    setCostBasedWideningDecision(VF);
----------------
mkuper wrote:
> I'm still not sure I understand why this gets called twice for a user-provided VF. Could you explain again?
We call this function from multiple places. From calculateRegisterUsage(), selectVectorizationFactor() - user-defined-VF and from expectedCost(). 
I just prevent recalculation.


================
Comment at: ../lib/Transforms/Vectorize/LoopVectorize.cpp:7036
+        if (InterleaveCost < NumAccesses * 2) {
+          // The interleaving cost is good enough.
+          setWideningDecision(Group, VF, CM_DECISION_INTERLEAVE,
----------------
mkuper wrote:
> Why the NumAccesses * 2 cut-off?
I consider a cost per instruction. In this case InterleaveCost / NumAccesses == 1. (Matthew asked to avoid divisions).
About 1 inst per access is good enough. I added more comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D27919