From: Michael Kuperstein <mkuper@google.com>
Date: Fri, 3 Feb 2017 19:32:50 +0000 (+0000)
Subject: [SLP] Make sortMemAccesses explicitly return an error. NFC.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=55c04887b46a5f338dc2115a25e525e7023b53a5;p=llvm

[SLP] Make sortMemAccesses explicitly return an error. NFC.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@294029 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/include/llvm/Analysis/LoopAccessAnalysis.h b/include/llvm/Analysis/LoopAccessAnalysis.h
index 1d9ade4468c..1cc679afa2c 100644
--- a/include/llvm/Analysis/LoopAccessAnalysis.h
+++ b/include/llvm/Analysis/LoopAccessAnalysis.h
@@ -692,13 +692,12 @@ int64_t getPtrStride(PredicatedScalarEvolution &PSE, Value *Ptr, const Loop *Lp,
 
 /// \brief Try to sort an array of loads / stores.
 ///
-/// If all pointers refer to the same object, and the differences between all
-/// pointer operands are known to be constant, the array is sorted by offset,
-/// and returned in \p Sorted.
-/// 
-/// If those conditions do not hold, the output array is an arbitrary
-/// permutation of the input.
-void sortMemAccesses(ArrayRef<Value *> VL, const DataLayout &DL,
+/// An array of loads / stores can only be sorted if all pointer operands
+/// refer to the same object, and the differences between these pointers 
+/// are known to be constant. If that is the case, this returns true, and the
+/// sorted array is returned in \p Sorted. Otherwise, this returns false, and
+/// \p Sorted is invalid.
+bool sortMemAccesses(ArrayRef<Value *> VL, const DataLayout &DL,
                      ScalarEvolution &SE, SmallVectorImpl<Value *> &Sorted);
 
 /// \brief Returns true if the memory operations \p A and \p B are consecutive.
diff --git a/lib/Analysis/LoopAccessAnalysis.cpp b/lib/Analysis/LoopAccessAnalysis.cpp
index 4e076f5f004..f035ccda6f5 100644
--- a/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/lib/Analysis/LoopAccessAnalysis.cpp
@@ -1058,9 +1058,9 @@ static unsigned getAddressSpaceOperand(Value *I) {
   return -1;
 }
 
-void llvm::sortMemAccesses(ArrayRef<Value *> VL, const DataLayout &DL,
-                         ScalarEvolution &SE,
-                         SmallVectorImpl<Value *> &Sorted) {
+bool llvm::sortMemAccesses(ArrayRef<Value *> VL, const DataLayout &DL,
+                           ScalarEvolution &SE,
+                           SmallVectorImpl<Value *> &Sorted) {
   SmallVector<std::pair<int64_t, Value *>, 4> OffValPairs;
   OffValPairs.reserve(VL.size());
   Sorted.reserve(VL.size());
@@ -1077,10 +1077,8 @@ void llvm::sortMemAccesses(ArrayRef<Value *> VL, const DataLayout &DL,
     // If a pointer refers to a different underlying object, bail - the
     // pointers are by definition incomparable.
     Value *CurrObj = GetUnderlyingObject(Ptr, DL);
-    if (CurrObj != Obj0) {
-      Sorted.append(VL.begin(), VL.end());
-      return;
-    }
+    if (CurrObj != Obj0)
+      return false;
 
     const SCEVConstant *Diff =
         dyn_cast<SCEVConstant>(SE.getMinusSCEV(SE.getSCEV(Ptr), Scev0));
@@ -1088,10 +1086,8 @@ void llvm::sortMemAccesses(ArrayRef<Value *> VL, const DataLayout &DL,
     // The pointers may not have a constant offset from each other, or SCEV
     // may just not be smart enough to figure out they do. Regardless,
     // there's nothing we can do.
-    if (!Diff) {
-      Sorted.append(VL.begin(), VL.end());
-      return;
-    }
+    if (!Diff)
+      return false;
 
     OffValPairs.emplace_back(Diff->getAPInt().getSExtValue(), Val);
   }
@@ -1102,8 +1098,10 @@ void llvm::sortMemAccesses(ArrayRef<Value *> VL, const DataLayout &DL,
               return Left.first < Right.first;
             });
 
-  for (auto& it : OffValPairs)
+  for (auto &it : OffValPairs)
     Sorted.push_back(it.second);
+
+  return true;
 }
 
 /// Returns true if the memory operations \p A and \p B are consecutive.
diff --git a/lib/Transforms/Vectorize/SLPVectorizer.cpp b/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 8a608ad71ec..dc64c39d385 100644
--- a/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -467,7 +467,9 @@ private:
                         ScalarEvolution &SE) const {
       assert(VL.size() == Scalars.size() && "Invalid size");
       SmallVector<Value *, 8> List;
-      sortMemAccesses(VL, DL, SE, List);
+      if (!sortMemAccesses(VL, DL, SE, List))
+        return false;
+
       return std::equal(List.begin(), List.end(), Scalars.begin());
     }
 
@@ -1223,18 +1225,19 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth) {
 
       if (VL.size() > 2 && !ReverseConsecutive) {
         bool ShuffledLoads = true;
-        SmallVector<Value *, 8> List;
-        sortMemAccesses(VL, *DL, *SE, List);
-        auto NewVL = makeArrayRef(List.begin(), List.end());
-        for (unsigned i = 0, e = NewVL.size() - 1; i < e; ++i) {
-          if (!isConsecutiveAccess(NewVL[i], NewVL[i + 1], *DL, *SE)) {
-            ShuffledLoads = false;
-            break;
+        SmallVector<Value *, 8> Sorted;
+        if (sortMemAccesses(VL, *DL, *SE, Sorted)) {
+          auto NewVL = makeArrayRef(Sorted.begin(), Sorted.end());
+          for (unsigned i = 0, e = NewVL.size() - 1; i < e; ++i) {
+            if (!isConsecutiveAccess(NewVL[i], NewVL[i + 1], *DL, *SE)) {
+              ShuffledLoads = false;
+              break;
+            }
+          }
+          if (ShuffledLoads) {
+            newTreeEntry(NewVL, true, true);
+            return;
           }
-        }
-        if (ShuffledLoads) {
-          newTreeEntry(NewVL, true, true);
-          return;
         }
       }