Use per-Purpose overloads for MemoryAccess creation

This makes the intent of each created object clearer and allows to add more specific asserts. The bug fixed in r248535 has been discovered this way.

No functional change intended; everything should behave as before.

llvm-svn: 248603

2 files changed, 111 insertions, 46 deletions

diff --git a/polly/include/polly/ScopInfo.h b/polly/include/polly/ScopInfo.h
index 1dc059853ae..ac2c440fb73 100644
--- a/polly/include/polly/ScopInfo.h
+++ b/polly/include/polly/ScopInfo.h
@@ -1490,22 +1490,60 @@ class ScopInfo : public RegionPass {
                        ArrayRef<const SCEV *> Subscripts,
                        ArrayRef<const SCEV *> Sizes, bool IsPHI);
 
-  void addMemoryAccess(BasicBlock *BB, Instruction *Inst,
-                       MemoryAccess::AccessType Type, Value *BaseAddress,
-                       unsigned ElemBytes, bool Affine, Value *AccessValue,
-                       bool IsPHI = false) {
-    addMemoryAccess(BB, Inst, Type, BaseAddress, ElemBytes, Affine, AccessValue,
-                    ArrayRef<const SCEV *>(), ArrayRef<const SCEV *>(), IsPHI);
-  }
+  /// @brief Create a MemoryAccess that represents either a LoadInst or
+  /// StoreInst.
+  ///
+  /// @param MemAccInst  The LoadInst or StoreInst.
+  /// @param Type        The kind of access.
+  /// @param BaseAddress The accessed array's base address.
+  /// @param ElemBytes   Size of accessed array element.
+  /// @param IsAffine    Whether all subscripts are affine expressions.
+  /// @param Subscripts  Access subscripts per dimension.
+  /// @param Sizes       The array dimension's sizes.
+  /// @param AccessValue Value read or written.
+  void addExplicitAccess(Instruction *MemAccInst, MemoryAccess::AccessType Type,
+                         Value *BaseAddress, unsigned ElemBytes, bool IsAffine,
+                         ArrayRef<const SCEV *> Subscripts,
+                         ArrayRef<const SCEV *> Sizes, Value *AccessValue);
 
-  void addMemoryAccess(BasicBlock *BB, Instruction *Inst,
-                       MemoryAccess::AccessType Type, Value *BaseAddress,
-                       unsigned ElemBytes, bool Affine,
-                       ArrayRef<const SCEV *> Subscripts,
-                       ArrayRef<const SCEV *> Sizes, Value *AccessValue) {
-    addMemoryAccess(BB, Inst, Type, BaseAddress, ElemBytes, Affine, AccessValue,
-                    Subscripts, Sizes, false);
-  }
+  /// @brief Create a MemoryAccess for writing to .s2a memory.
+  ///
+  /// The access will be created at the @p Value's definition.
+  ///
+  /// @param Value The value to be written into the .s2a memory.
+  void addScalarWriteAccess(Instruction *Value);
+
+  /// @brief Create a MemoryAccess for reading from .s2a memory.
+  ///
+  /// @param Value The scalar expected to be loaded.
+  /// @param User  User of the scalar, where the access is added.
+  void addScalarReadAccess(Value *Value, Instruction *User);
+
+  /// @brief Create a MemoryAccess for reading from .s2a memory.
+  ///
+  /// This is for PHINodes using the scalar. It is used when leaving the
+  /// incoming block to write to the .phiops location.
+  ///
+  /// @param Value  The scalar expected to be loaded.
+  /// @param User   The PHI node referencing @p Value.
+  /// @param UserBB Incoming block for the incoming @p Value.
+  void addScalarReadAccess(Value *Value, PHINode *User, BasicBlock *UserBB);
+
+  /// @brief Create a MemoryAccess for writing to .phiops memory.
+  ///
+  /// @param PHI           PHINode under consideration.
+  /// @param IncomingBlock Some predecessor block.
+  /// @param IncomingValue @p PHI's value when coming from @p IncomingBlock.
+  /// @param IsExitBlock   When true, use the .s2a alloca instead. Used for
+  ///                      values escaping through a PHINode in the SCoP
+  ///                      region's exit block.
+  void addPHIWriteAccess(PHINode *PHI, BasicBlock *IncomingBlock,
+                         Value *IncomingValue, bool IsExitBlock);
+
+  /// @brief Create a MemoryAccess for reading from .phiops memory.
+  ///
+  /// @param PHI PHINode under consideration. READ access will be added here.
+  void addPHIReadAccess(PHINode *PHI);
 
 public:
   static char ID;
diff --git a/polly/lib/Analysis/ScopInfo.cpp b/polly/lib/Analysis/ScopInfo.cpp
index b844c25bbe5..b23ddcdb32a 100644
--- a/polly/lib/Analysis/ScopInfo.cpp
+++ b/polly/lib/Analysis/ScopInfo.cpp
@@ -2703,26 +2703,18 @@ void ScopInfo::buildPHIAccesses(PHINode *PHI, Region &R,
       // we have to insert a scalar dependence from the definition of OpI to
       // OpBB if the definition is not in OpBB.
       if (OpIBB != OpBB) {
-        addMemoryAccess(OpBB, PHI, MemoryAccess::READ, OpI, 1, true, OpI);
-        addMemoryAccess(OpIBB, OpI, MemoryAccess::MUST_WRITE, OpI, 1, true,
-                        OpI);
+        addScalarReadAccess(OpI, PHI, OpBB);
+        addScalarWriteAccess(OpI);
       }
     } else if (ModelReadOnlyScalars && !isa<Constant>(Op)) {
-      addMemoryAccess(OpBB, PHI, MemoryAccess::READ, Op, 1, true, Op);
+      addScalarReadAccess(Op, PHI, OpBB);
     }
 
-    // Always use the terminator of the incoming basic block as the access
-    // instruction.
-    OpI = OpBB->getTerminator();
-
-    addMemoryAccess(OpBB, OpI, MemoryAccess::MUST_WRITE, PHI, 1, true, Op,
-                    /* IsPHI */ !IsExitBlock);
+    addPHIWriteAccess(PHI, OpBB, Op, IsExitBlock);
   }
 
-  if (!OnlyNonAffineSubRegionOperands) {
-    addMemoryAccess(PHI->getParent(), PHI, MemoryAccess::READ, PHI, 1, true,
-                    PHI,
-                    /* IsPHI */ !IsExitBlock);
+  if (!OnlyNonAffineSubRegionOperands && !IsExitBlock) {
+    addPHIReadAccess(PHI);
   }
 }
 
@@ -2778,7 +2770,7 @@ bool ScopInfo::buildScalarDependences(Instruction *Inst, Region *R,
     // Do not build a read access that is not in the current SCoP
     // Use the def instruction as base address of the MemoryAccess, so that it
     // will become the name of the scalar access in the polyhedral form.
-    addMemoryAccess(UseParent, UI, MemoryAccess::READ, Inst, 1, true, Inst);
+    addScalarReadAccess(Inst, UI);
   }
 
   if (ModelReadOnlyScalars && !isa<PHINode>(Inst)) {
@@ -2793,8 +2785,7 @@ bool ScopInfo::buildScalarDependences(Instruction *Inst, Region *R,
       if (isa<Constant>(Op))
         continue;
 
-      addMemoryAccess(Inst->getParent(), Inst, MemoryAccess::READ, Op, 1, true,
-                      Op);
+      addScalarReadAccess(Op, Inst);
     }
   }
 
@@ -2865,8 +2856,8 @@ void ScopInfo::buildMemoryAccess(
         SizesSCEV.push_back(SE->getSCEV(ConstantInt::get(
             IntegerType::getInt64Ty(BasePtr->getContext()), Size)));
 
-        addMemoryAccess(Inst->getParent(), Inst, Type, BasePointer->getValue(),
-                        Size, true, Subscripts, SizesSCEV, Val);
+        addExplicitAccess(Inst, Type, BasePointer->getValue(), Size, true,
+                          Subscripts, SizesSCEV, Val);
         return;
       }
     }
@@ -2874,9 +2865,9 @@ void ScopInfo::buildMemoryAccess(
 
   auto AccItr = InsnToMemAcc.find(Inst);
   if (PollyDelinearize && AccItr != InsnToMemAcc.end()) {
-    addMemoryAccess(Inst->getParent(), Inst, Type, BasePointer->getValue(),
-                    Size, true, AccItr->second.DelinearizedSubscripts,
-                    AccItr->second.Shape->DelinearizedSizes, Val);
+    addExplicitAccess(Inst, Type, BasePointer->getValue(), Size, true,
+                      AccItr->second.DelinearizedSubscripts,
+                      AccItr->second.Shape->DelinearizedSizes, Val);
     return;
   }
 
@@ -2893,15 +2884,16 @@ void ScopInfo::buildMemoryAccess(
   bool IsAffine = !isVariantInNonAffineLoop &&
                   isAffineExpr(R, AccessFunction, *SE, BasePointer->getValue());
 
-  SmallVector<const SCEV *, 4> Subscripts, Sizes;
-  Subscripts.push_back(AccessFunction);
-  Sizes.push_back(SE->getConstant(ZeroOffset->getType(), Size));
+  // FIXME: Size if the number of bytes of an array element, not the number of
+  // elements as probably intended here.
+  const SCEV *SizeSCEV = SE->getConstant(ZeroOffset->getType(), Size);
 
   if (!IsAffine && Type == MemoryAccess::MUST_WRITE)
     Type = MemoryAccess::MAY_WRITE;
 
-  addMemoryAccess(Inst->getParent(), Inst, Type, BasePointer->getValue(), Size,
-                  IsAffine, Subscripts, Sizes, Val);
+  addExplicitAccess(Inst, Type, BasePointer->getValue(), Size, IsAffine,
+                    ArrayRef<const SCEV *>(AccessFunction),
+                    ArrayRef<const SCEV *>(SizeSCEV), Val);
 }
 
 void ScopInfo::buildAccessFunctions(Region &R, Region &SR) {
@@ -2946,8 +2938,7 @@ void ScopInfo::buildAccessFunctions(Region &R, BasicBlock &BB,
 
     if (buildScalarDependences(Inst, &R, NonAffineSubRegion)) {
       if (!isa<StoreInst>(Inst))
-        addMemoryAccess(&BB, Inst, MemoryAccess::MUST_WRITE, Inst, 1, true,
-                        Inst);
+        addScalarWriteAccess(Inst);
     }
   }
 }
@@ -2957,8 +2948,7 @@ void ScopInfo::addMemoryAccess(BasicBlock *BB, Instruction *Inst,
                                Value *BaseAddress, unsigned ElemBytes,
                                bool Affine, Value *AccessValue,
                                ArrayRef<const SCEV *> Subscripts,
-                               ArrayRef<const SCEV *> Sizes,
-                               bool IsPHI = false) {
+                               ArrayRef<const SCEV *> Sizes, bool IsPHI) {
   AccFuncSetType &AccList = AccFuncMap[BB];
   size_t Identifier = AccList.size();
 
@@ -2972,6 +2962,43 @@ void ScopInfo::addMemoryAccess(BasicBlock *BB, Instruction *Inst,
                        Subscripts, Sizes, AccessValue, IsPHI, BaseName);
 }
 
+void ScopInfo::addExplicitAccess(
+    Instruction *MemAccInst, MemoryAccess::AccessType Type, Value *BaseAddress,
+    unsigned ElemBytes, bool IsAffine, ArrayRef<const SCEV *> Subscripts,
+    ArrayRef<const SCEV *> Sizes, Value *AccessValue) {
+  assert(isa<LoadInst>(MemAccInst) || isa<StoreInst>(MemAccInst));
+  assert(isa<LoadInst>(MemAccInst) == (Type == MemoryAccess::READ));
+  addMemoryAccess(MemAccInst->getParent(), MemAccInst, Type, BaseAddress,
+                  ElemBytes, IsAffine, AccessValue, Subscripts, Sizes, false);
+}
+void ScopInfo::addScalarWriteAccess(Instruction *Value) {
+  addMemoryAccess(Value->getParent(), Value, MemoryAccess::MUST_WRITE, Value, 1,
+                  true, Value, ArrayRef<const SCEV *>(),
+                  ArrayRef<const SCEV *>(), false);
+}
+void ScopInfo::addScalarReadAccess(Value *Value, Instruction *User) {
+  assert(!isa<PHINode>(User));
+  addMemoryAccess(User->getParent(), User, MemoryAccess::READ, Value, 1, true,
+                  Value, ArrayRef<const SCEV *>(), ArrayRef<const SCEV *>(),
+                  false);
+}
+void ScopInfo::addScalarReadAccess(Value *Value, PHINode *User,
+                                   BasicBlock *UserBB) {
+  addMemoryAccess(UserBB, User, MemoryAccess::READ, Value, 1, true, Value,
+                  ArrayRef<const SCEV *>(), ArrayRef<const SCEV *>(), false);
+}
+void ScopInfo::addPHIWriteAccess(PHINode *PHI, BasicBlock *IncomingBlock,
+                                 Value *IncomingValue, bool IsExitBlock) {
+  addMemoryAccess(IncomingBlock, IncomingBlock->getTerminator(),
+                  MemoryAccess::MUST_WRITE, PHI, 1, true, IncomingValue,
+                  ArrayRef<const SCEV *>(), ArrayRef<const SCEV *>(),
+                  !IsExitBlock);
+}
+void ScopInfo::addPHIReadAccess(PHINode *PHI) {
+  addMemoryAccess(PHI->getParent(), PHI, MemoryAccess::READ, PHI, 1, true, PHI,
+                  ArrayRef<const SCEV *>(), ArrayRef<const SCEV *>(), true);
+}
+
 Scop *ScopInfo::buildScop(Region &R, DominatorTree &DT) {
   unsigned MaxLoopDepth = getMaxLoopDepthInRegion(R, *LI, *SD);
   Scop *S = new Scop(R, AccFuncMap, *SE, DT, ctx, MaxLoopDepth);