[LoopVectorize] Pass parameters explicitly to MemoryDepChecker

[anemet]

Summary

Rather than using globals. This prepares the class to be moved outside the
LoopVectorizer.

It's not great how all these are passed through in LoopAccessAnalysis but this
is all expected to change once the class start servicing the Loop Distribution
pass as well where these parameters make no sense.

NFC. This is part of the patchset that splits out the memory dependence logic
from LoopVectorizationLegality into a new class LoopAccessAnalysis.
LoopAccessAnalysis will be used by the new Loop Distribution pass.

[hfinkel]

I think it would be better to collect these into a struct describing vectorization thresholds.

[anemet]

In D7284#115788, @hfinkel wrote:

I think it would be better to collect these into a struct describing vectorization thresholds.

Sure, I can do that. (I was going to wait and see how this shapes up with LoopDist as another client. The MemoryDepChecker would have to somehow be specialized further for the Vectorizer.)

[hfinkel]

• Original Message -----

From: "Adam Nemet" <[e-mail]>
To: [e-mail], [e-mail], [e-mail]
Cc: [e-mail]
Sent: Friday, January 30, 2015 12:01:32 PM
Subject: Re: [PATCH] [LoopVectorize] Pass parameters explicitly to MemoryDepChecker

In http://reviews.llvm.org/D7284#115788, @hfinkel wrote:

I think it would be better to collect these into a struct
describing vectorization thresholds.

Sure, I can do that. (I was going to wait and see how this shapes up
with LoopDist as another client. The MemoryDepChecker would have to
somehow be specialized further for the Vectorizer.)

Understood; but I think you might as well do that now. We can always refactor things again later.

-Hal

http://reviews.llvm.org/D7284

EMAIL PREFERENCES

http://reviews.llvm.org/settings/panel/emailpreferences/

[anemet]

Rebased and addressed Hal's comments

[hfinkel]

LGTM.

lib/Transforms/Vectorize/LoopVectorize.cpp

722

MSVC2012 sometimes has problems with syntax like this, and you might need to construct by name if it complains -- the buildbots will tell you ;)

[anemet]

r227754

lib/Transforms/Vectorize/LoopVectorize.cpp

 /// is delegated to the MemoryDepChecker class.
 ///
 /// For memory dependences that cannot be determined at compile time, it
 /// generates run-time checks to prove independence.  This is done by
 /// AccessAnalysis::canCheckPtrAtRT and the checks are maintained by the
 /// RuntimePointerCheck class.
 class LoopAccessAnalysis {
 public:
+  /// \brief Collection of parameters used from the vectorizer.
+  struct VectorizerParams {
+    /// \brief Maximum simd width.
+    unsigned MaxVectorWidth;
+
+    /// \brief VF as overridden by the user.
+    unsigned VectorizationFactor;
+    /// \brief Interleave factor as overridden by the user.
+    unsigned VectorizationInterleave;
+
+    /// \\brief When performing memory disambiguation checks at runtime do not
+    /// make more than this number of comparisons.
+    unsigned RuntimeMemoryCheckThreshold;
+
+    VectorizerParams(unsigned MaxVectorWidth,
+                     unsigned VectorizationFactor,
+                     unsigned VectorizationInterleave,
+                     unsigned RuntimeMemoryCheckThreshold) :
+        MaxVectorWidth(MaxVectorWidth),
+        VectorizationFactor(VectorizationFactor),
+        VectorizationInterleave(VectorizationInterleave),
+        RuntimeMemoryCheckThreshold(RuntimeMemoryCheckThreshold) {}
+  };
+
   LoopAccessAnalysis(Function *F, Loop *L, ScalarEvolution *SE,
                      const DataLayout *DL, const TargetLibraryInfo *TLI,
-                     AliasAnalysis *AA, DominatorTree *DT) :
+                     AliasAnalysis *AA, DominatorTree *DT,
+                     const VectorizerParams &VectParams) :
       TheFunction(F), TheLoop(L), SE(SE), DL(DL), TLI(TLI), AA(AA), DT(DT),
-      NumLoads(0), NumStores(0), MaxSafeDepDistBytes(-1U) {
-  }
+      NumLoads(0), NumStores(0), MaxSafeDepDistBytes(-1U),
+      VectParams(VectParams) {}
 
   /// Return true we can analyze the memory accesses in the loop and there are
   /// no memory dependence cycles.  Replaces symbolic strides using Strides.
   bool canVectorizeMemory(ValueToValueMap &Strides);
 
   RuntimePointerCheck *getRuntimePointerCheck() { return &PtrRtCheck; }
 
   /// Return true if the block BB needs to be predicated in order for the loop
private:
   const TargetLibraryInfo *TLI;
   AliasAnalysis *AA;
   DominatorTree *DT;
 
   unsigned NumLoads;
   unsigned NumStores;
 
   unsigned MaxSafeDepDistBytes;
+
+  /// \brief Vectorizer parameters used by the analysis.
+  VectorizerParams VectParams;
 };
 } // end anonymous namespace
 
 /// LoopVectorizationLegality checks if it is legal to vectorize a loop, and
 /// to what vectorization factor.
 /// This class does not look at the profitability of vectorization, only the
 /// legality. This class has two main kinds of checks:
 /// * Memory checks - The code in canVectorizeMemory checks if vectorization
 ///   will change the order of memory accesses in a way that will change the
 ///   correctness of the program.
 /// * Scalars checks - The code in canVectorizeInstrs and canVectorizeMemory
 /// checks for a number of different conditions, such as the availability of a
 /// single induction variable, that all types are supported and vectorize-able,
 /// etc. This code reflects the capabilities of InnerLoopVectorizer.
 /// This class is also used by InnerLoopVectorizer for identifying
 /// induction variable and the different reduction variables.
 class LoopVectorizationLegality {
 public:
   LoopVectorizationLegality(Loop *L, ScalarEvolution *SE, const DataLayout *DL,
                             DominatorTree *DT, TargetLibraryInfo *TLI,
                             AliasAnalysis *AA, Function *F,
                             const TargetTransformInfo *TTI)
       : NumPredStores(0), TheLoop(L), SE(SE), DL(DL), TLI(TLI), TheFunction(F),
         TTI(TTI), Induction(nullptr), WidestIndTy(nullptr),
-        LAA(F, L, SE, DL, TLI, AA, DT), HasFunNoNaNAttr(false) {
+        LAA(F, L, SE, DL, TLI, AA, DT,
+            {MaxVectorWidth, VectorizationFactor, VectorizationInterleave,
+             RuntimeMemoryCheckThreshold}),
+        HasFunNoNaNAttr(false) {
   }
 
   /// This enum represents the kinds of reductions that we support.
   enum ReductionKind {
     RK_NoReduction, ///< Not a reduction.
     RK_IntegerAdd,  ///< Sum of integers.
     RK_IntegerMult, ///< Product of integers.
     RK_IntegerOr,   ///< Bitwise or logical OR of numbers.

 ///
 ///  * Zero distances and all accesses have the same size.
 ///
 class MemoryDepChecker {
 public:
   typedef PointerIntPair<Value *, 1, bool> MemAccessInfo;
   typedef SmallPtrSet<MemAccessInfo, 8> MemAccessInfoSet;
 
-  MemoryDepChecker(ScalarEvolution *Se, const DataLayout *Dl, const Loop *L)
+  MemoryDepChecker(ScalarEvolution *Se, const DataLayout *Dl, const Loop *L,
+                   const LoopAccessAnalysis::VectorizerParams &VectParams)
       : SE(Se), DL(Dl), InnermostLoop(L), AccessIdx(0),
-        ShouldRetryWithRuntimeCheck(false) {}
+        ShouldRetryWithRuntimeCheck(false), VectParams(VectParams) {}
 
   /// \brief Register the location (instructions are given increasing numbers)
   /// of a write access.
   void addAccess(StoreInst *SI) {
     Value *Ptr = SI->getPointerOperand();
     Accesses[MemAccessInfo(Ptr, true)].push_back(AccessIdx);
     InstMap.push_back(SI);
     ++AccessIdx;
private:
 
   // We can access this many bytes in parallel safely.
   unsigned MaxSafeDepDistBytes;
 
   /// \brief If we see a non-constant dependence distance we can still try to
   /// vectorize this loop with runtime checks.
   bool ShouldRetryWithRuntimeCheck;
 
+  /// \brief Vectorizer parameters used by the analysis.
+  LoopAccessAnalysis::VectorizerParams VectParams;
+
   /// \brief Check whether there is a plausible dependence between the two
   /// accesses.
   ///
   /// Access \p A must happen before \p B in program order. The two indices
   /// identify the index into the program order map.
   ///
   /// This function checks  whether there is a plausible dependence (or the
   /// absence of such can't be proved) between the two accesses. If there is a
bool MemoryDepChecker::couldPreventStoreLoadForward(unsigned Distance,
   // 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.
   // Store-load forwarding distance.
   const unsigned NumCyclesForStoreLoadThroughMemory = 8*TypeByteSize;
   // Maximum vector factor.
-  unsigned MaxVFWithoutSLForwardIssues = MaxVectorWidth*TypeByteSize;
+  unsigned MaxVFWithoutSLForwardIssues = VectParams.MaxVectorWidth*TypeByteSize;
   if(MaxSafeDepDistBytes < MaxVFWithoutSLForwardIssues)
     MaxVFWithoutSLForwardIssues = MaxSafeDepDistBytes;
 
   for (unsigned vf = 2*TypeByteSize; vf <= MaxVFWithoutSLForwardIssues;
        vf *= 2) {
     if (Distance % vf && Distance / vf < NumCyclesForStoreLoadThroughMemory) {
       MaxVFWithoutSLForwardIssues = (vf >>=1);
       break;
     }
   }
 
   if (MaxVFWithoutSLForwardIssues< 2*TypeByteSize) {
     DEBUG(dbgs() << "LV: Distance " << Distance <<
           " that could cause a store-load forwarding conflict\n");
     return true;
   }
 
   if (MaxVFWithoutSLForwardIssues < MaxSafeDepDistBytes &&
-      MaxVFWithoutSLForwardIssues != MaxVectorWidth*TypeByteSize)
+      MaxVFWithoutSLForwardIssues != VectParams.MaxVectorWidth*TypeByteSize)
     MaxSafeDepDistBytes = MaxVFWithoutSLForwardIssues;
   return false;
 }
 
 bool MemoryDepChecker::isDependent(const MemAccessInfo &A, unsigned AIdx,
                                    const MemAccessInfo &B, unsigned BIdx,
                                    ValueToValueMap &Strides) {
   assert (AIdx < BIdx && "Must pass arguments in program order");
if (ATy != BTy) {
     DEBUG(dbgs() <<
           "LV: ReadWrite-Write positive dependency with different types\n");
     return false;
   }
 
   unsigned Distance = (unsigned) Val.getZExtValue();
 
   // Bail out early if passed-in parameters make vectorization not feasible.
-  unsigned ForcedFactor = VectorizationFactor ? VectorizationFactor : 1;
-  unsigned ForcedUnroll = VectorizationInterleave ? VectorizationInterleave : 1;
+  unsigned ForcedFactor = (VectParams.VectorizationFactor ?
+                           VectParams.VectorizationFactor : 1);
+  unsigned ForcedUnroll = (VectParams.VectorizationInterleave ?
+                           VectParams.VectorizationInterleave : 1);
 
   // The distance must be bigger than the size needed for a vectorized version
   // of the operation and the size of the vectorized operation must not be
   // bigger than the currrent maximum size.
   if (Distance < 2*TypeByteSize ||
       2*TypeByteSize > MaxSafeDepDistBytes ||
       Distance < TypeByteSize * ForcedUnroll * ForcedFactor) {
     DEBUG(dbgs() << "LV: Failure because of Positive distance "
bool LoopAccessAnalysis::canVectorizeMemory(ValueToValueMap &Strides) {
   // Holds all the different accesses in the loop.
   unsigned NumReads = 0;
   unsigned NumReadWrites = 0;
 
   PtrRtCheck.Pointers.clear();
   PtrRtCheck.Need = false;
 
   const bool IsAnnotatedParallel = TheLoop->isAnnotatedParallel();
-  MemoryDepChecker DepChecker(SE, DL, TheLoop);
+  MemoryDepChecker DepChecker(SE, DL, TheLoop, VectParams);
 
   // For each block.
   for (Loop::block_iterator bb = TheLoop->block_begin(),
        be = TheLoop->block_end(); bb != be; ++bb) {
 
     // Scan the BB and collect legal loads and stores.
     for (BasicBlock::iterator it = (*bb)->begin(), e = (*bb)->end(); it != e;
          ++it) {
bool LoopAccessAnalysis::canVectorizeMemory(ValueToValueMap &Strides) {
 
   // If we only have one set of dependences to check pointers among we don't
   // need a runtime check.
   if (NumComparisons == 0 && NeedRTCheck)
     NeedRTCheck = false;
 
   // Check that we did not collect too many pointers or found an unsizeable
   // pointer.
-  if (!CanDoRT || NumComparisons > RuntimeMemoryCheckThreshold) {
+  if (!CanDoRT || NumComparisons > VectParams.RuntimeMemoryCheckThreshold) {
     PtrRtCheck.reset();
     CanDoRT = false;
   }
 
   if (CanDoRT) {
     DEBUG(dbgs() << "LV: We can perform a memory runtime check if needed.\n");
   }
 
if (!CanVecMem && DepChecker.shouldRetryWithRuntimeCheck()) {
 
       PtrRtCheck.reset();
       PtrRtCheck.Need = true;
 
       CanDoRT = Accesses.canCheckPtrAtRT(PtrRtCheck, NumComparisons, SE,
                                          TheLoop, Strides, true);
       // Check that we did not collect too many pointers or found an unsizeable
       // pointer.
-      if (!CanDoRT || NumComparisons > RuntimeMemoryCheckThreshold) {
+      if (!CanDoRT || NumComparisons > VectParams.RuntimeMemoryCheckThreshold) {
         if (!CanDoRT && NumComparisons > 0)
           emitAnalysis(Report()
                        << "cannot check memory dependencies at runtime");
         else
           emitAnalysis(Report()
                        << NumComparisons << " exceeds limit of "
-                       << RuntimeMemoryCheckThreshold
+                       << VectParams.RuntimeMemoryCheckThreshold
                        << " dependent memory operations checked at runtime");
         DEBUG(dbgs() << "LV: Can't vectorize with memory checks\n");
         PtrRtCheck.reset();
         return false;
       }
 
       CanVecMem = true;
     }