[FIX] Introduce different SAI objects for scalar and memory accesses

  Even if a scalar and memory access have the same base pointer, we cannot use
  one SAI object as the type but also the number of dimensions are wrong. For
  the attached test case this caused a crash in the invariant load hoisting,
  though it could cause various other problems too.

This fixes bug 25428 and a execution time bug in MallocBench/cfrac.

Reported-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
llvm-svn: 252422

4 files changed, 89 insertions, 9 deletions

diff --git a/polly/include/polly/ScopInfo.h b/polly/include/polly/ScopInfo.h
index b37c7889018..e18906754d0 100644
--- a/polly/include/polly/ScopInfo.h
+++ b/polly/include/polly/ScopInfo.h
@@ -1063,7 +1063,7 @@ private:
   /// @brief The affinator used to translate SCEVs to isl expressions.
   SCEVAffinator Affinator;
 
-  typedef MapVector<std::pair<AssertingVH<const Value>, int>,
+  typedef MapVector<std::pair<AssertingVH<const Value>, std::pair<int, int>>,
                     std::unique_ptr<ScopArrayInfo>> ArrayInfoMapTy;
   /// @brief A map to remember ScopArrayInfo objects for all base pointers.
   ///
@@ -1506,9 +1506,11 @@ public:
 
   /// @brief Return the cached ScopArrayInfo object for @p BasePtr.
   ///
-  /// @param BasePtr The base pointer the object has been stored for
-  /// @param IsPHI   Are we looking for special PHI storage.
-  const ScopArrayInfo *getScopArrayInfo(Value *BasePtr, bool IsPHI = false);
+  /// @param BasePtr  The base pointer the object has been stored for.
+  /// @param IsScalar Are we looking for a scalar or memory access location.
+  /// @param IsPHI    Are we looking for special PHI storage.
+  const ScopArrayInfo *getScopArrayInfo(Value *BasePtr, bool IsScalar,
+                                        bool IsPHI = false);
 
   void setContext(isl_set *NewContext);
 
diff --git a/polly/lib/Analysis/ScopInfo.cpp b/polly/lib/Analysis/ScopInfo.cpp
index 805b52318df..319d0722dad 100644
--- a/polly/lib/Analysis/ScopInfo.cpp
+++ b/polly/lib/Analysis/ScopInfo.cpp
@@ -150,7 +150,7 @@ static const ScopArrayInfo *identifyBasePtrOriginSAI(Scop *S, Value *BasePtr) {
   if (!OriginBaseSCEVUnknown)
     return nullptr;
 
-  return S->getScopArrayInfo(OriginBaseSCEVUnknown->getValue());
+  return S->getScopArrayInfo(OriginBaseSCEVUnknown->getValue(), false);
 }
 
 ScopArrayInfo::ScopArrayInfo(Value *BasePtr, Type *ElementType, isl_ctx *Ctx,
@@ -2752,7 +2752,9 @@ void Scop::hoistInvariantLoads() {
 const ScopArrayInfo *
 Scop::getOrCreateScopArrayInfo(Value *BasePtr, Type *AccessType,
                                ArrayRef<const SCEV *> Sizes, bool IsPHI) {
-  auto &SAI = ScopArrayInfoMap[std::make_pair(BasePtr, IsPHI)];
+  bool IsScalar = Sizes.empty();
+  auto ScalarTypePair = std::make_pair(IsScalar, IsPHI);
+  auto &SAI = ScopArrayInfoMap[std::make_pair(BasePtr, ScalarTypePair)];
   if (!SAI) {
     SAI.reset(new ScopArrayInfo(BasePtr, AccessType, getIslCtx(), Sizes, IsPHI,
                                 this));
@@ -2765,8 +2767,10 @@ Scop::getOrCreateScopArrayInfo(Value *BasePtr, Type *AccessType,
   return SAI.get();
 }
 
-const ScopArrayInfo *Scop::getScopArrayInfo(Value *BasePtr, bool IsPHI) {
-  auto *SAI = ScopArrayInfoMap[std::make_pair(BasePtr, IsPHI)].get();
+const ScopArrayInfo *Scop::getScopArrayInfo(Value *BasePtr, bool IsScalar,
+                                            bool IsPHI) {
+  auto ScalarTypePair = std::make_pair(IsScalar, IsPHI);
+  auto *SAI = ScopArrayInfoMap[std::make_pair(BasePtr, ScalarTypePair)].get();
   assert(SAI && "No ScopArrayInfo available for this base pointer");
   return SAI;
 }
diff --git a/polly/lib/CodeGen/IslNodeBuilder.cpp b/polly/lib/CodeGen/IslNodeBuilder.cpp
index 9e4358f27ad..7539cb3da8f 100644
--- a/polly/lib/CodeGen/IslNodeBuilder.cpp
+++ b/polly/lib/CodeGen/IslNodeBuilder.cpp
@@ -964,7 +964,7 @@ bool IslNodeBuilder::preloadInvariantEquivClass(
 
   // If the base pointer of this class is dependent on another one we have to
   // make sure it was preloaded already.
-  auto *SAI = S.getScopArrayInfo(MA->getBaseAddr());
+  auto *SAI = S.getScopArrayInfo(MA->getBaseAddr(), false);
   if (const auto *BaseIAClass = S.lookupInvariantEquivClass(SAI->getBasePtr()))
     if (!preloadInvariantEquivClass(*BaseIAClass))
       return false;
diff --git a/polly/test/ScopInfo/same-base-address-scalar-and-array.ll b/polly/test/ScopInfo/same-base-address-scalar-and-array.ll
new file mode 100644
index 00000000000..344ee557b76
--- /dev/null
+++ b/polly/test/ScopInfo/same-base-address-scalar-and-array.ll
@@ -0,0 +1,74 @@
+; RUN: opt %loadPolly -polly-code-generator=isl -polly-scops -analyze < %s | FileCheck %s
+;
+; Verify we introduce two ScopArrayInfo objects (or virtual arrays) for the %out variable
+; as it is used as a memory base pointer (%0) but also as a scalar (%out.addr.0.lcssa).
+;
+; CHECK:         Arrays {
+; CHECK-NEXT:        float* MemRef_out[*] // Element size 0
+; CHECK-NEXT:        float* MemRef_out_addr_0_lcssa[*] // Element size 0
+; CHECK-NEXT:        float MemRef_out[*][4] // Element size 4
+; CHECK-NEXT:    }
+;
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+
+; Function Attrs: nounwind ssp uwtable
+define void @ff_celp_lp_synthesis_filterf(float* %out) #0 {
+entry:
+  br label %entry.split
+
+entry.split:                                      ; preds = %entry
+  br i1 false, label %for.end.97, label %for.body.lr.ph
+
+for.body.lr.ph:                                   ; preds = %entry.split
+  %arrayidx13 = getelementptr inbounds float, float* %out, i64 -3
+  %0 = load float, float* %arrayidx13, align 4
+  br label %for.body
+
+for.body:                                         ; preds = %for.end, %for.body.lr.ph
+  br i1 false, label %for.body.50.lr.ph, label %for.end
+
+for.body.50.lr.ph:                                ; preds = %for.body
+  br label %for.body.50
+
+for.body.50:                                      ; preds = %for.body.50, %for.body.50.lr.ph
+  br i1 false, label %for.body.50, label %for.cond.48.for.end_crit_edge
+
+for.cond.48.for.end_crit_edge:                    ; preds = %for.body.50
+  br label %for.end
+
+for.end:                                          ; preds = %for.cond.48.for.end_crit_edge, %for.body
+  %add96 = add nuw nsw i32 0, 4
+  %cmp = icmp sgt i32 %add96, 0
+  br i1 %cmp, label %for.cond.for.end.97_crit_edge, label %for.body
+
+for.cond.for.end.97_crit_edge:                    ; preds = %for.end
+  br label %for.end.97
+
+for.end.97:                                       ; preds = %for.cond.for.end.97_crit_edge, %entry.split
+  %out.addr.0.lcssa = phi float* [ undef, %for.cond.for.end.97_crit_edge ], [ %out, %entry.split ]
+  br i1 undef, label %for.body.104.lr.ph, label %for.end.126
+
+for.body.104.lr.ph:                               ; preds = %for.end.97
+  br label %for.body.104
+
+for.body.104:                                     ; preds = %for.inc.124, %for.body.104.lr.ph
+  br i1 undef, label %for.inc.124, label %for.body.111.lr.ph
+
+for.body.111.lr.ph:                               ; preds = %for.body.104
+  br label %for.body.111
+
+for.body.111:                                     ; preds = %for.body.111, %for.body.111.lr.ph
+  br i1 undef, label %for.body.111, label %for.cond.109.for.inc.124_crit_edge
+
+for.cond.109.for.inc.124_crit_edge:               ; preds = %for.body.111
+  br label %for.inc.124
+
+for.inc.124:                                      ; preds = %for.cond.109.for.inc.124_crit_edge, %for.body.104
+  br i1 undef, label %for.body.104, label %for.cond.102.for.end.126_crit_edge
+
+for.cond.102.for.end.126_crit_edge:               ; preds = %for.inc.124
+  br label %for.end.126
+
+for.end.126:                                      ; preds = %for.cond.102.for.end.126_crit_edge, %for.end.97
+  ret void
+}