[PATCH] D20241: [LV] Respect max VF for interleaved accesses

[Adam Nemet via llvm-commits]

anemet accepted this revision.
anemet added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for starting to untangle this.

Some minor comments below.


================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:70
@@ +69,3 @@
+/// option can be disabled for correctness testing.
+static cl::opt<bool> EnableStoreLoadForwardOpt(
+    "enable-store-load-forwarding-opt", cl::Hidden,
----------------
I think that conflict detection is a better name.

================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:1215-1218
@@ -1206,4 +1214,6 @@
     if (IsTrueDataDependence &&
-        (couldPreventStoreLoadForward(Val.abs().getZExtValue(), TypeByteSize) ||
+        ((EnableStoreLoadForwardOpt &&
+          couldPreventStoreLoadForward(Val.abs().getZExtValue(),
+                                       TypeByteSize)) ||
          ATy != BTy)) {
       DEBUG(dbgs() << "LAA: Forward but may prevent st->ld forwarding\n");
----------------
Don't we want to also disable for the ATy != BTy case?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:859
@@ +858,3 @@
+  unsigned getMaxInterleaveFactor() const {
+    auto MaxFactor = 1u;
+    for (auto &Entry : InterleaveGroupMap)
----------------
wow, what made you write this instead of unsigned ...  = 1? ;)

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5199-5202
@@ -5185,5 +5198,6 @@
   unsigned MaxSafeDepDist = -1U;
   if (Legal->getMaxSafeDepDistBytes() != -1U)
-    MaxSafeDepDist = Legal->getMaxSafeDepDistBytes() * 8;
+    MaxSafeDepDist =
+        Legal->getMaxSafeDepDistBytes() * 8 / Legal->getMaxInterleaveFactor();
   WidestRegister =
       ((WidestRegister < MaxSafeDepDist) ? WidestRegister : MaxSafeDepDist);
----------------
I think that this needs a comment something along the line of the description in this review.  

We may also want to mention that this is fairly conservative because the dependence distance may be between non-interleaved accesses, etc.  (Hopefully this won't lead to any performance degradation.)


http://reviews.llvm.org/D20241