[LAA] MaxSafeVectorWidthBits depends on changes to MinDepDist

[michaelmaitland]

Summary

If MinDepDist changes, then it is possible that MaxSafeVectorWidthInBits
changes since it depends on MinDepDist. Whenever we may change
MinDepDist, we must change MaxSafeVectorWidthInBits.

[michaelmaitland]

After taking a closer look, I can make all the changes needed in D150706.

llvm/include/llvm/Analysis/LoopAccessAnalysis.h

private:
   Dependence::DepType isDependent(const MemAccessInfo &A, unsigned AIdx,
                                   const MemAccessInfo &B, unsigned BIdx,
                                   const DenseMap<Value *, const SCEV *> &Strides);
 
   /// Check whether the data dependence could prevent store-load
   /// forwarding.
   ///
   /// \return false if we shouldn't vectorize at all or avoid larger
-  /// vectorization factors by limiting MinDepDistBytes.
-  bool couldPreventStoreLoadForward(uint64_t Distance, uint64_t TypeByteSize);
+  /// vectorization factors by limiting MinDepDistBytes and
+  /// MaxSafeVectorWidthInBits.
+  bool couldPreventStoreLoadForward(uint64_t Distance, uint64_t TypeByteSize,
+                                    uint64_t Stride);
 
   /// Updates the current safety status with \p S. We can go from Safe to
   /// either PossiblySafeWithRtChecks or Unsafe and from
   /// PossiblySafeWithRtChecks to Unsafe.
   void mergeInStatus(VectorizationSafetyStatus S);
 };
 
 class RuntimePointerChecking;

llvm/lib/Analysis/LoopAccessAnalysis.cpp

bool MemoryDepChecker::Dependence::isForward() const {
   case Backward:
   case BackwardVectorizableButPreventsForwarding:
     return false;
   }
   llvm_unreachable("unexpected DepType!");
 }
 
 bool MemoryDepChecker::couldPreventStoreLoadForward(uint64_t Distance,
-                                                    uint64_t TypeByteSize) {
+                                                    uint64_t TypeByteSize,
+                                                    uint64_t Stride) {
   // If loads occur at a distance that is not a multiple of a feasible vector
   // factor store-load forwarding does not take place.
   // Positive dependences might cause troubles because vectorizing them might
   // prevent store-load forwarding making vectorized code run a lot slower.
   //   a[i] = a[i-3] ^ a[i-8];
   //   The stores to a[i:i+1] don't align with the stores to a[i-3:i-2] and
   //   hence on your typical architecture store-load forwarding does not take
   //   place. Vectorizing in such cases does not make sense.
if (MaxVFWithoutSLForwardIssues < 2 * TypeByteSize) {
     LLVM_DEBUG(
         dbgs() << "LAA: Distance " << Distance
                << " that could cause a store-load forwarding conflict\n");
     return true;
   }
 
   if (MaxVFWithoutSLForwardIssues < MinDepDistBytes &&
       MaxVFWithoutSLForwardIssues !=
-          VectorizerParams::MaxVectorWidth * TypeByteSize)
+          VectorizerParams::MaxVectorWidth * TypeByteSize) {
     MinDepDistBytes = MaxVFWithoutSLForwardIssues;
+    // An update to MaxSafeDepDistBytes requires an update to
+    // MaxSafeVectorWidthInBits.
+    uint64_t MaxVF = MinDepDistBytes / (TypeByteSize * Stride);
+    uint64_t MaxVFInBits = MaxVF * TypeByteSize * 8;
+    MaxSafeVectorWidthInBits = std::min(MaxSafeVectorWidthInBits, MaxVFInBits);
+  }
   return false;
 }
 
 void MemoryDepChecker::mergeInStatus(VectorizationSafetyStatus S) {
   if (Status < S)
     Status = S;
 }
 
if (std::abs(Distance) > 0 && Stride > 1 && HasSameSize &&
     LLVM_DEBUG(dbgs() << "LAA: Strided accesses are independent\n");
     return Dependence::NoDep;
   }
 
   // Negative distances are not plausible dependencies.
   if (Val.isNegative()) {
     bool IsTrueDataDependence = (AIsWrite && !BIsWrite);
     if (IsTrueDataDependence && EnableForwardingConflictDetection &&
-        (couldPreventStoreLoadForward(Val.abs().getZExtValue(), TypeByteSize) ||
+        (couldPreventStoreLoadForward(Val.abs().getZExtValue(), TypeByteSize,
+                                      Stride) ||
          !HasSameSize)) {
       LLVM_DEBUG(dbgs() << "LAA: Forward but may prevent st->ld forwarding\n");
       return Dependence::ForwardButPreventsForwarding;
     }
 
     LLVM_DEBUG(dbgs() << "LAA: Dependence is negative\n");
     return Dependence::Forward;
   }
MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
   // This case is currently unsafe according to the max safe distance. If we
   // analyze the two accesses on array B, the max safe dependence distance
   // is 2. Then we analyze the accesses on array A, the minimum distance needed
   // is 8, which is less than 2 and forbidden vectorization, But actually
   // both A and B could be vectorized by 2 iterations.
   MinDepDistBytes =
       std::min(static_cast<uint64_t>(Distance), MinDepDistBytes);
 
-  bool IsTrueDataDependence = (!AIsWrite && BIsWrite);
-  if (IsTrueDataDependence && EnableForwardingConflictDetection &&
-      couldPreventStoreLoadForward(Distance, TypeByteSize))
-    return Dependence::BackwardVectorizableButPreventsForwarding;
-
+  // An update to MaxSafeDepDistBytes requires an update to
+  // MaxSafeVectorWidthInBits.
   uint64_t MaxVF = MinDepDistBytes / (TypeByteSize * Stride);
   LLVM_DEBUG(dbgs() << "LAA: Positive distance " << Val.getSExtValue()
                     << " with max VF = " << MaxVF << '\n');
   uint64_t MaxVFInBits = MaxVF * TypeByteSize * 8;
   MaxSafeVectorWidthInBits = std::min(MaxSafeVectorWidthInBits, MaxVFInBits);
+
+  bool IsTrueDataDependence = (!AIsWrite && BIsWrite);
+  if (IsTrueDataDependence && EnableForwardingConflictDetection &&
+      couldPreventStoreLoadForward(Distance, TypeByteSize, Stride))
+    return Dependence::BackwardVectorizableButPreventsForwarding;
+
   return Dependence::BackwardVectorizable;
 }
 
 bool MemoryDepChecker::areDepsSafe(DepCandidates &AccessSets,
                                    MemAccessInfoList &CheckDeps,
                                    const DenseMap<Value *, const SCEV *> &Strides) {
 
   MinDepDistBytes = -1;