[Polly] Introduce MemAccInst helper class; NFC

[Meinersbur]

Summary

MemAccInst wraps the common members of LoadInst and StoreInst. Add uses of this class to

• ScopInfo::buildMemoryAccess
• BlockGenerator::generateLocationAccessed
• ScopInfo::addArrayAccess
• Scop::buildAliasGroups
• wherever polly::getPointerOperand was used

The class has been extracted out of D12975 (DeLICM) where it is used more extensively.

[llvm-commits]

+/ getValueOperand() and getPointerOperand() have different implementations in
+/ LoadInst and StoreInst, but for most other members the implementation is
+/ identical. Those have been reproduced here to avoid useless branching to
+/ identical implementations. If those implementations change in
+/// LoadInst/StoreInst, they must be changed here as well.

The last sentence alone guarantees problems. Duplication is (almost)
never a solution, especially not to save a call/jump/branch.

• signature.asc219 BDownload
• msg-5106-603.txt162 BDownload

[Meinersbur]

In D16530#334955, @llvm-commits wrote:

+/ getValueOperand() and getPointerOperand() have different implementations in
+/ LoadInst and StoreInst, but for most other members the implementation is
+/ identical. Those have been reproduced here to avoid useless branching to
+/ identical implementations. If those implementations change in
+/// LoadInst/StoreInst, they must be changed here as well.

The last sentence alone guarantees problems. Duplication is (almost)
never a solution, especially not to save a call/jump/branch.

We'd notice immediately, but I can change to calling the proper functions, no problem.

Note that this is also duplicated in LLVM itself.

[Meinersbur]

Do not duplicate code from LLVM.

[grosser]

Hi Michael,

I think this patch makes a lot of sense. Unfortunately all these casts make it look rather ugly as they introduce more noise than is actually removed. I just looked quickly at the implementation of CallSite and it seems the implementation is slightly different. Specifically, it does not inherit from instruction, but rather creates a proxy class around the CallInst/InvokeInst. Like this it is possible to add default constructors for CallSite(Invoke Ty) and CallSite(CallTy) that allow for automatic implicit type conversion. Could this be a solution that would allow us to remove the need for explicit casts?

Tobias

[Meinersbur]

In D16530#335899, @grosser wrote:

Hi Michael,

I think this patch makes a lot of sense. Unfortunately all these casts make it look rather ugly as they introduce more noise than is actually removed. I just looked quickly at the implementation of CallSite and it seems the implementation is slightly different. Specifically, it does not inherit from instruction, but rather creates a proxy class around the CallInst/InvokeInst. Like this it is possible to add default constructors for CallSite(Invoke Ty) and CallSite(CallTy) that allow for automatic implicit type conversion. Could this be a solution that would allow us to remove the need for explicit casts?

Unfortunately their type is LoadInst* instead of LoadInst&. C++ does not support overloading operators/implicit conversions of pointer types. I could change their declarations to references and implement these implicit constructors, but this would create a new MemAccInst every time, i.e. create a new instruction because it is derived from Instruction.

The implementation of CallSite is quite different. It is a stack object containing a pointer to the CallInst or InvokeInst (you called it proxy). This allows "it can also create a null initialized CallSiteBase object for something which is NOT a call site". I don't think we need this, we can just test for nullptr. It also does not allow implicit conversions from pointers and users then are required to think about the lifetime of the stack object. It would allow implicit conversions from LoadInst& though.

Anyway, it is only one of the uses where I thought it could make sense. We could also just drop the BlockGenerator::generateLocationAccessed part.

One more use could be InstructionToAccess mapping, because since D13676 we only map loads/stores.

[grosser]

Meinersbur added a comment.

In http://reviews.llvm.org/D16530#335899, @grosser wrote:

Hi Michael,

I think this patch makes a lot of sense. Unfortunately all these casts make it look rather ugly as they introduce more noise than is actually removed. I just looked quickly at the implementation of CallSite and it seems the implementation is slightly different. Specifically, it does not inherit from instruction, but rather creates a proxy class around the CallInst/InvokeInst. Like this it is possible to add default constructors for CallSite(Invoke Ty) and CallSite(CallTy) that allow for automatic implicit type conversion. Could this be a solution that would allow us to remove the need for explicit casts?

Unfortunately their type is LoadInst* instead of LoadInst&. C++ does not support overloading operators/implicit conversions of pointer types. I could change their declarations to references and implement these implicit constructors, but this would create a new MemAccInst every time, i.e. create a new instruction because it is derived from Instruction.

I think it would be OK to change the type to LoadInst& and to construct
new instances of MemAccInst that keep just a pointer to LoadInst/StoreInst.

Now, I am not sure if there is a real need to actually derive from
Instruction at all. This seems to add overhead and does not seem to be
used. CallSiteInst works e.g. also without being derived from Instruction.

The implementation of CallSite is quite different. It is a stack object containing a pointer to the CallInst or InvokeInst (you called it proxy). This allows "it can also create a null initialized CallSiteBase object for something which is NOT a call site". I don't think we need this, we can just test for nullptr. It also does not allow implicit conversions from pointers and users then are required to think about the lifetime of the stack object. It would allow implicit conversions from LoadInst& though.

Right, the last point is what I am after.

Anyway, it is only one of the uses where I thought it could make sense. We could also just drop the BlockGenerator::generateLocationAccessed part.

If it can not even be used here, this new class does not seem to be very
useful. Hence, I am trying to figure out if we can address my last concerns.

One more use could be InstructionToAccess mapping, because since http://reviews.llvm.org/D13676 we only map loads/stores.

Possibly. However, I think the use in generationLocationAccessed would
be good enough to justify its introduction.

Best,
Tobias

[Meinersbur]

Proxy object variant (like CallSite, StringRef, ArrayRef)

[grosser]

Hi Michael,

this looks a lot better. Johannes pointed out to me that there is already a function polly::getPointerOperand in ScopHelper.cpp. Having both is probably redundant. I think the GetElementPtrInst is never used in this function (please check). So we can probably eliminate it and just use your class instead of the getPointerOperand instruction. Otherwise, this is probably good to go.

[Meinersbur]

Replace polly::getPointerOperand(); Move class MemAccInst to ScopHelper.h to make it available in ScopDetection.h

[grosser]

LGTM.

include/polly/CodeGen/BlockGenerators.h

protected:
   /// @param Inst The instruction for which we get the loop.
   /// @return The innermost loop that surrounds the instruction.
   Loop *getLoopForInst(const Instruction *Inst);
 
   /// @brief Generate the operand address
   /// @param NewAccesses A map from memory access ids to new ast expressions,
   ///                    which may contain new access expressions for certain
   ///                    memory accesses.
-  Value *generateLocationAccessed(ScopStmt &Stmt, const Instruction *Inst,
-                                  Value *Pointer, ValueMapT &BBMap,
-                                  LoopToScevMapT &LTS,
+  Value *generateLocationAccessed(ScopStmt &Stmt, MemAccInst Inst,
+                                  ValueMapT &BBMap, LoopToScevMapT &LTS,
                                   isl_id_to_ast_expr *NewAccesses);
 
   /// @param NewAccesses A map from memory access ids to new ast expressions,
   ///                    which may contain new access expressions for certain
   ///                    memory accesses.
   Value *generateScalarLoad(ScopStmt &Stmt, LoadInst *load, ValueMapT &BBMap,
                             LoopToScevMapT &LTS,
                             isl_id_to_ast_expr *NewAccesses);

include/polly/ScopDetection.h

private:
   bool isInvariant(const Value &Val, const Region &Reg) const;
 
   /// @brief Check if a memory access can be part of a Scop.
   ///
   /// @param Inst The instruction accessing the memory.
   /// @param Context The context of scop detection.
   ///
   /// @return True if the memory access is valid, false otherwise.
-  bool isValidMemoryAccess(Instruction &Inst, DetectionContext &Context) const;
+  bool isValidMemoryAccess(MemAccInst Inst, DetectionContext &Context) const;
 
   /// @brief Check if an instruction has any non trivial scalar dependencies
   ///        as part of a Scop.
   ///
   /// @param Inst The instruction to check.
   /// @param RefRegion The region in respect to which we check the access
   ///                  function.
   ///

include/polly/ScopInfo.h

 
 namespace polly {
 
 class MemoryAccess;
 class Scop;
 class ScopStmt;
 class ScopInfo;
 
+typedef DenseMap<ScopStmt *, Value *> OutgoingValueMapTy;
+
 //===---------------------------------------------------------------------===//
 
 /// @brief Enumeration of assumptions Polly can take.
 enum AssumptionKind {
   ALIASING,
   INBOUNDS,
   WRAPPING,
   ALIGNMENT,
class ScopInfo : public RegionPass {
 
   /// @brief Build an instance of MemoryAccess from the Load/Store instruction.
   ///
   /// @param Inst       The Load/Store instruction that access the memory
   /// @param L          The parent loop of the instruction
   /// @param R          The region on which to build the data access dictionary.
   /// @param BoxedLoops The set of loops that are overapproximated in @p R.
   /// @param ScopRIL    The required invariant loads equivalence classes.
-  void buildMemoryAccess(Instruction *Inst, Loop *L, Region *R,
+  void buildMemoryAccess(MemAccInst Inst, Loop *L, Region *R,
                          const ScopDetection::BoxedLoopsSetTy *BoxedLoops,
                          const InvariantLoadsSetTy &ScopRIL);
 
   /// @brief Analyze and extract the cross-BB scalar dependences (or,
   ///        dataflow dependencies) of an instruction.
   ///
   /// @param Inst               The instruction to be analyzed
   /// @param R                  The SCoP region
class ScopInfo : public RegionPass {
   /// @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.
   /// @see ScopArrayInfo::MemoryKind
-  void addArrayAccess(Instruction *MemAccInst, MemoryAccess::AccessType Type,
+  void addArrayAccess(MemAccInst MemAccInst, MemoryAccess::AccessType Type,
                       Value *BaseAddress, unsigned ElemBytes, bool IsAffine,
                       ArrayRef<const SCEV *> Subscripts,
                       ArrayRef<const SCEV *> Sizes, Value *AccessValue);
 
   /// @brief Create a MemoryAccess for writing an llvm::Value.
   ///
   /// The access will be created at the @p Value's definition.
   ///

include/polly/Support/ScopHelper.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef POLLY_SUPPORT_IRHELPER_H
 #define POLLY_SUPPORT_IRHELPER_H
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/IR/Instructions.h"
 #include "llvm/IR/ValueHandle.h"
 
 namespace llvm {
-class Type;
-class Instruction;
-class LoadInst;
 class LoopInfo;
-class Loop;
 class ScalarEvolution;
 class SCEV;
-class Value;
-class PHINode;
 class Region;
 class Pass;
-class BasicBlock;
-class StringRef;
-class DataLayout;
 class DominatorTree;
 class RegionInfo;
-class TerminatorInst;
-class ScalarEvolution;
 }
 
 namespace polly {
 class Scop;
 
 /// @brief Type to remap values.
 using ValueMapT = llvm::DenseMap<llvm::AssertingVH<llvm::Value>,
                                  llvm::AssertingVH<llvm::Value>>;
 
 /// @brief Type for a set of invariant loads.
 using InvariantLoadsSetTy = llvm::SetVector<llvm::AssertingVH<llvm::LoadInst>>;
 
+/// @brief Utility proxy to wrap the common members of LoadInst and StoreInst.
+///
+/// This works like the LLVM utility class CallSite, ie. it forwards all calls
+/// to either a LoadInst or StoreInst.
+/// It is similar to LLVM's utility classes IntrinsicInst, MemIntrinsic,
+/// MemTransferInst, etc. in that it offers a common interface, but does not act
+/// as a fake base class.
+/// It is similar to StringRef and ArrayRef in that it holds a pointer to the
+/// referenced object and should be passed by-value as it is small enough.
+///
+/// This proxy can either represent a LoadInst instance, a StoreInst instance,
+/// or a nullptr (only creatable using the default constructor); never an
+/// Instruction that is not a load or store.
+/// When representing a nullptr, only the following methods are defined:
+/// isNull(), isInstruction(), isLoad(), isStore(), operator bool(), operator!()
+///
+/// The functions isa, cast, cast_or_null, dyn_cast are modeled te resemble
+/// those from llvm/Support/Casting.h. Partial template function specialization
+/// is currently not supported in C++ such that those cannot be used directly.
+/// (llvm::isa could, but then llvm:cast etc. would not have the expected
+/// behaviour)
+class MemAccInst {
+private:
+  llvm::Instruction *I;
+
+public:
+  MemAccInst() : I(nullptr) {}
+  MemAccInst(const MemAccInst &Inst) : I(Inst.I) {}
+  /* implicit */ MemAccInst(llvm::LoadInst &LI) : I(&LI) {}
+  /* implicit */ MemAccInst(llvm::LoadInst *LI) : I(LI) {}
+  /* implicit */ MemAccInst(llvm::StoreInst &SI) : I(&SI) {}
+  /* implicit */ MemAccInst(llvm::StoreInst *SI) : I(SI) {}
+  explicit MemAccInst(llvm::Instruction &I) : I(&I) { assert(isa(I)); }
+  explicit MemAccInst(llvm::Instruction *I) : I(I) { assert(isa(I)); }
+
+  static bool isa(const llvm::Value &V) {
+    return llvm::isa<llvm::LoadInst>(V) || llvm::isa<llvm::StoreInst>(V);
+  }
+  static bool isa(const llvm::Value *V) {
+    return llvm::isa<llvm::LoadInst>(V) || llvm::isa<llvm::StoreInst>(V);
+  }
+  static MemAccInst cast(llvm::Value &V) {
+    return MemAccInst(llvm::cast<llvm::Instruction>(V));
+  }
+  static MemAccInst cast(llvm::Value *V) {
+    return MemAccInst(llvm::cast<llvm::Instruction>(V));
+  }
+  static MemAccInst cast_or_null(llvm::Value &V) {
+    return MemAccInst(llvm::cast<llvm::Instruction>(V));
+  }
+  static MemAccInst cast_or_null(llvm::Value *V) {
+    if (!V)
+      return MemAccInst();
+    return MemAccInst(llvm::cast<llvm::Instruction>(V));
+  }
+  static MemAccInst dyn_cast(llvm::Value &V) {
+    if (isa(V))
+      return MemAccInst(llvm::cast<llvm::Instruction>(V));
+    return MemAccInst();
+  }
+  static MemAccInst dyn_cast(llvm::Value *V) {
+    assert(V);
+    if (isa(V))
+      return MemAccInst(llvm::cast<llvm::Instruction>(V));
+    return MemAccInst();
+  }
+
+  MemAccInst &operator=(const MemAccInst &Inst) {
+    I = Inst.I;
+    return *this;
+  }
+  MemAccInst &operator=(llvm::LoadInst &LI) {
+    I = &LI;
+    return *this;
+  }
+  MemAccInst &operator=(llvm::LoadInst *LI) {
+    I = LI;
+    return *this;
+  }
+  MemAccInst &operator=(llvm::StoreInst &SI) {
+    I = &SI;
+    return *this;
+  }
+  MemAccInst &operator=(llvm::StoreInst *SI) {
+    I = SI;
+    return *this;
+  }
+
+  operator llvm::Instruction *() const { return asInstruction(); }
+  explicit operator bool() const { return isInstruction(); }
+  bool operator!() const { return isNull(); }
+
+  llvm::Value *getOperand(unsigned i) const { return I->getOperand(i); }
+  llvm::BasicBlock *getParent() const { return I->getParent(); }
+  llvm::LLVMContext &getContext() const { return I->getContext(); }
+  void getAAMetadata(llvm::AAMDNodes &N, bool Merge = false) const {
+    I->getAAMetadata(N, Merge);
+  }
+
+  llvm::Value *getValueOperand() const {
+    if (isLoad())
+      return asLoad();
+    if (isStore())
+      return asStore()->getValueOperand();
+    llvm_unreachable("Operation not supported on nullptr");
+  }
+  llvm::Value *getPointerOperand() const {
+    if (isLoad())
+      return asLoad()->getPointerOperand();
+    if (isStore())
+      return asStore()->getPointerOperand();
+    llvm_unreachable("Operation not supported on nullptr");
+  }
+
+  unsigned getAlignment() const {
+    if (isLoad())
+      return asLoad()->getAlignment();
+    if (isStore())
+      return asStore()->getAlignment();
+    llvm_unreachable("Operation not supported on nullptr");
+  }
+  bool isVolatile() const {
+    if (isLoad())
+      return asLoad()->isVolatile();
+    if (isStore())
+      return asStore()->isVolatile();
+    llvm_unreachable("Operation not supported on nullptr");
+  }
+  bool isSimple() const {
+    if (isLoad())
+      return asLoad()->isSimple();
+    if (isStore())
+      return asStore()->isSimple();
+    llvm_unreachable("Operation not supported on nullptr");
+  }
+  llvm::AtomicOrdering getOrdering() const {
+    if (isLoad())
+      return asLoad()->getOrdering();
+    if (isStore())
+      return asStore()->getOrdering();
+    llvm_unreachable("Operation not supported on nullptr");
+  }
+  bool isUnordered() const {
+    if (isLoad())
+      return asLoad()->isUnordered();
+    if (isStore())
+      return asStore()->isUnordered();
+    llvm_unreachable("Operation not supported on nullptr");
+  }
+
+  bool isNull() const { return !I; }
+  bool isInstruction() const { return I; }
+  bool isLoad() const { return I && llvm::isa<llvm::LoadInst>(I); }
+  bool isStore() const { return I && llvm::isa<llvm::StoreInst>(I); }
+
+  llvm::Instruction *asInstruction() const { return I; }
+  llvm::LoadInst *asLoad() const { return llvm::cast<llvm::LoadInst>(I); }
+  llvm::StoreInst *asStore() const { return llvm::cast<llvm::StoreInst>(I); }
+};
+
 /// @brief Check if the PHINode has any incoming Invoke edge.
 ///
 /// @param PN The PHINode to check.
 ///
 /// @return If the PHINode has an incoming BB that jumps to the parent BB
 ///         of the PHINode with an invoke instruction, return true,
 ///         otherwise, return false.
 bool hasInvokeEdge(const llvm::PHINode *PN);
 
-llvm::Value *getPointerOperand(llvm::Instruction &Inst);
-
 /// @brief Simplify the region to have a single unconditional entry edge and a
 /// single exit edge.
 ///
 /// Although this function allows DT and RI to be null, regions only work
 /// properly if the DominatorTree (for Region::contains) and RegionInfo are kept
 /// up-to-date.
 ///
 /// @param R  The region to be simplified

lib/Analysis/ScopDetection.cpp

if (!hasBaseAffineAccesses(Context, BasePointer)) {
       if (KeepGoing)
         continue;
       else
         return false;
     }
   return true;
 }
 
-bool ScopDetection::isValidMemoryAccess(Instruction &Inst,
+bool ScopDetection::isValidMemoryAccess(MemAccInst Inst,
                                         DetectionContext &Context) const {
   Region &CurRegion = Context.CurRegion;
 
-  Value *Ptr = getPointerOperand(Inst);
+  Value *Ptr = Inst.getPointerOperand();
   Loop *L = LI->getLoopFor(Inst.getParent());
   const SCEV *AccessFunction = SE->getSCEVAtScope(Ptr, L);
   const SCEVUnknown *BasePointer;
   Value *BaseValue;
 
   BasePointer = dyn_cast<SCEVUnknown>(SE->getPointerBase(AccessFunction));
 
   if (!BasePointer)
-    return invalid<ReportNoBasePtr>(Context, /*Assert=*/true, &Inst);
+    return invalid<ReportNoBasePtr>(Context, /*Assert=*/true, Inst);
 
   BaseValue = BasePointer->getValue();
 
   if (isa<UndefValue>(BaseValue))
-    return invalid<ReportUndefBasePtr>(Context, /*Assert=*/true, &Inst);
+    return invalid<ReportUndefBasePtr>(Context, /*Assert=*/true, Inst);
 
   // Check that the base address of the access is invariant in the current
   // region.
   if (!isInvariant(*BaseValue, CurRegion))
     return invalid<ReportVariantBasePtr>(Context, /*Assert=*/true, BaseValue,
-                                         &Inst);
+                                         Inst);
 
   AccessFunction = SE->getMinusSCEV(AccessFunction, BasePointer);
 
-  const SCEV *Size = SE->getElementSize(&Inst);
+  const SCEV *Size = SE->getElementSize(Inst);
   if (Context.ElementSize.count(BasePointer)) {
     if (Context.ElementSize[BasePointer] != Size)
       return invalid<ReportDifferentArrayElementSize>(Context, /*Assert=*/true,
-                                                      &Inst, BaseValue);
+                                                      Inst, BaseValue);
   } else {
     Context.ElementSize[BasePointer] = Size;
   }
 
   bool isVariantInNonAffineLoop = false;
   SetVector<const Loop *> Loops;
   findLoops(AccessFunction, Loops);
   for (const Loop *L : Loops)
     if (Context.BoxedLoopsSet.count(L))
       isVariantInNonAffineLoop = true;
 
   if (PollyDelinearize && !isVariantInNonAffineLoop) {
-    Context.Accesses[BasePointer].push_back({&Inst, AccessFunction});
+    Context.Accesses[BasePointer].push_back({Inst, AccessFunction});
 
     if (!isAffine(AccessFunction, Context, BaseValue))
       Context.NonAffineAccesses.insert(BasePointer);
   } else if (!AllowNonAffine) {
     if (isVariantInNonAffineLoop ||
         !isAffine(AccessFunction, Context, BaseValue))
       return invalid<ReportNonAffineAccess>(Context, /*Assert=*/true,
-                                            AccessFunction, &Inst, BaseValue);
+                                            AccessFunction, Inst, BaseValue);
   }
 
   // FIXME: Think about allowing IntToPtrInst
   if (IntToPtrInst *Inst = dyn_cast<IntToPtrInst>(BaseValue))
     return invalid<ReportIntToPtr>(Context, /*Assert=*/true, Inst);
 
   if (IgnoreAliasing)
     return true;
if (PollyUseRuntimeAliasChecks) {
           CanBuildRunTimeCheck = false;
           break;
         }
       }
 
       if (CanBuildRunTimeCheck)
         return true;
     }
-    return invalid<ReportAlias>(Context, /*Assert=*/true, &Inst, AS);
+    return invalid<ReportAlias>(Context, /*Assert=*/true, Inst, AS);
   }
 
   return true;
 }
 
 bool ScopDetection::isValidInstruction(Instruction &Inst,
                                        DetectionContext &Context) const {
   for (auto &Op : Inst.operands()) {
bool ScopDetection::isValidInstruction(Instruction &Inst,
   if (!Inst.mayWriteToMemory() && !Inst.mayReadFromMemory()) {
     if (!isa<AllocaInst>(Inst))
       return true;
 
     return invalid<ReportAlloca>(Context, /*Assert=*/true, &Inst);
   }
 
   // Check the access function.
-  if (isa<LoadInst>(Inst) || isa<StoreInst>(Inst)) {
-    Context.hasStores |= isa<StoreInst>(Inst);
-    Context.hasLoads |= isa<LoadInst>(Inst);
-    if (auto *Load = dyn_cast<LoadInst>(&Inst))
-      if (!Load->isSimple())
-        return invalid<ReportNonSimpleMemoryAccess>(Context, /*Assert=*/true,
-                                                    &Inst);
-    if (auto *Store = dyn_cast<StoreInst>(&Inst))
-      if (!Store->isSimple())
-        return invalid<ReportNonSimpleMemoryAccess>(Context, /*Assert=*/true,
-                                                    &Inst);
+  if (auto MemInst = MemAccInst::dyn_cast(Inst)) {
+    Context.hasStores |= MemInst.isLoad();
+    Context.hasLoads |= MemInst.isStore();
+    if (!MemInst.isSimple())
 
-    return isValidMemoryAccess(Inst, Context);
+    return isValidMemoryAccess(MemInst, Context);
   }
 
   // We do not know this instruction, therefore we assume it is invalid.
   return invalid<ReportUnknownInst>(Context, /*Assert=*/true, &Inst);
 }
 
 bool ScopDetection::canUseISLTripCount(Loop *L,
                                        DetectionContext &Context) const {

lib/Analysis/ScopInfo.cpp

Statement->getParent()->addAssumption(
       INBOUNDS, Outside,
       getAccessInstruction() ? getAccessInstruction()->getDebugLoc() : nullptr);
   isl_space_free(Space);
 }
 
 void MemoryAccess::computeBoundsOnAccessRelation(unsigned ElementSize) {
   ScalarEvolution *SE = Statement->getParent()->getSE();
 
-  Value *Ptr = getPointerOperand(*getAccessInstruction());
+  Value *Ptr = MemAccInst(getAccessInstruction()).getPointerOperand();
   if (!Ptr || !SE->isSCEVable(Ptr->getType()))
     return;
 
   auto *PtrSCEV = SE->getSCEV(Ptr);
   if (isa<SCEVCouldNotCompute>(PtrSCEV))
     return;
 
   auto *BasePtrSCEV = SE->getPointerBase(PtrSCEV);
for (ScopStmt &Stmt : *this) {
     if (StmtDomainEmpty)
       continue;
 
     for (MemoryAccess *MA : Stmt) {
       if (MA->isScalarKind())
         continue;
       if (!MA->isRead())
         HasWriteAccess.insert(MA->getBaseAddr());
-      Instruction *Acc = MA->getAccessInstruction();
-      PtrToAcc[getPointerOperand(*Acc)] = MA;
+      MemAccInst Acc(MA->getAccessInstruction());
+      PtrToAcc[Acc.getPointerOperand()] = MA;
       AST.add(Acc);
     }
   }
 
   SmallVector<AliasGroupTy, 4> AliasGroups;
   for (AliasSet &AS : AST) {
     if (AS.isMustAlias() || AS.isForwardingAliasSet())
       continue;
bool ScopInfo::buildScalarDependences(Instruction *Inst, Region *R,
   }
 
   return AnyCrossStmtUse;
 }
 
 extern MapInsnToMemAcc InsnToMemAcc;
 
 void ScopInfo::buildMemoryAccess(
-    Instruction *Inst, Loop *L, Region *R,
+    MemAccInst Inst, Loop *L, Region *R,
     const ScopDetection::BoxedLoopsSetTy *BoxedLoops,
     const InvariantLoadsSetTy &ScopRIL) {
-  unsigned Size;
-  Type *SizeType;
-  Value *Val;
-  enum MemoryAccess::AccessType Type;
-
-  if (LoadInst *Load = dyn_cast<LoadInst>(Inst)) {
-    SizeType = Load->getType();
-    Size = TD->getTypeAllocSize(SizeType);
-    Type = MemoryAccess::READ;
-    Val = Load;
-  } else {
-    StoreInst *Store = cast<StoreInst>(Inst);
-    SizeType = Store->getValueOperand()->getType();
-    Size = TD->getTypeAllocSize(SizeType);
-    Type = MemoryAccess::MUST_WRITE;
-    Val = Store->getValueOperand();
-  }
 
-  auto Address = getPointerOperand(*Inst);
+  Value *Address = Inst.getPointerOperand();
+  Value *Val = Inst.getValueOperand();
+  Type *SizeType = Val->getType();
+  unsigned Size = TD->getTypeAllocSize(SizeType);
+  enum MemoryAccess::AccessType Type =
+      Inst.isLoad() ? MemoryAccess::READ : MemoryAccess::MUST_WRITE;
 
   const SCEV *AccessFunction = SE->getSCEVAtScope(Address, L);
   const SCEVUnknown *BasePointer =
       dyn_cast<SCEVUnknown>(SE->getPointerBase(AccessFunction));
 
   assert(BasePointer && "Could not find base pointer");
   AccessFunction = SE->getMinusSCEV(AccessFunction, BasePointer);
 
void ScopInfo::buildMemoryAccess(
 
   for (LoadInst *LInst : AccessILS)
     if (!ScopRIL.count(LInst))
       IsAffine = false;
 
   // FIXME: Size of the number of bytes of an array element, not the number of
   // elements as probably intended here.
   const SCEV *SizeSCEV =
-      SE->getConstant(TD->getIntPtrType(Inst->getContext()), Size);
+      SE->getConstant(TD->getIntPtrType(Inst.getContext()), Size);
 
   if (!IsAffine && Type == MemoryAccess::MUST_WRITE)
     Type = MemoryAccess::MAY_WRITE;
 
   addArrayAccess(Inst, Type, BasePointer->getValue(), Size, IsAffine,
                  ArrayRef<const SCEV *>(AccessFunction),
                  ArrayRef<const SCEV *>(SizeSCEV), Val);
 }
for (BasicBlock::iterator I = BB.begin(), E = --BB.end(); I != E; ++I) {
     // For the exit block we stop modeling after the last PHI node.
     if (!PHI && IsExitBlock)
       break;
 
     // TODO: At this point we only know that elements of ScopRIL have to be
     //       invariant and will be hoisted for the SCoP to be processed. Though,
     //       there might be other invariant accesses that will be hoisted and
     //       that would allow to make a non-affine access affine.
-    if (isa<LoadInst>(Inst) || isa<StoreInst>(Inst))
-      buildMemoryAccess(Inst, L, &R, BoxedLoops, ScopRIL);
+    if (auto MemInst = MemAccInst::dyn_cast(Inst))
+      buildMemoryAccess(MemInst, L, &R, BoxedLoops, ScopRIL);
 
     if (isIgnoredIntrinsic(Inst))
       continue;
 
     // Do not build scalar dependences for required invariant loads as we will
     // hoist them later on anyway or drop the SCoP if we cannot.
     if (ScopRIL.count(dyn_cast<LoadInst>(Inst)))
       continue;
if (!isKnownMustAccess && Type == MemoryAccess::MUST_WRITE)
     Type = MemoryAccess::MAY_WRITE;
 
   AccList.emplace_back(Stmt, Inst, Type, BaseAddress, ElemBytes, Affine,
                        Subscripts, Sizes, AccessValue, Kind, BaseName);
   Stmt->addAccess(&AccList.back());
   return &AccList.back();
 }
 
-void ScopInfo::addArrayAccess(Instruction *MemAccInst,
+void ScopInfo::addArrayAccess(MemAccInst 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,
+  assert(MemAccInst.isLoad() == (Type == MemoryAccess::READ));
+  addMemoryAccess(MemAccInst.getParent(), MemAccInst, Type, BaseAddress,
                   ElemBytes, IsAffine, AccessValue, Subscripts, Sizes,
                   ScopArrayInfo::MK_Array);
 }
 void ScopInfo::ensureValueWrite(Instruction *Value) {
   ScopStmt *Stmt = scop->getStmtForBasicBlock(Value->getParent());
 
   // Value not defined within this SCoP.
   if (!Stmt)

lib/CodeGen/BlockGenerators.cpp

void BlockGenerator::copyInstScalar(ScopStmt &Stmt, Instruction *Inst,
 
   Builder.Insert(NewInst);
   BBMap[Inst] = NewInst;
 
   if (!NewInst->getType()->isVoidTy())
     NewInst->setName("p_" + Inst->getName());
 }
 
-Value *BlockGenerator::generateLocationAccessed(
-    ScopStmt &Stmt, const Instruction *Inst, Value *Pointer, ValueMapT &BBMap,
-    LoopToScevMapT &LTS, isl_id_to_ast_expr *NewAccesses) {
+Value *
+BlockGenerator::generateLocationAccessed(ScopStmt &Stmt, MemAccInst Inst,
+                                         ValueMapT &BBMap, LoopToScevMapT &LTS,
+                                         isl_id_to_ast_expr *NewAccesses) {
   const MemoryAccess &MA = Stmt.getArrayAccessFor(Inst);
 
   isl_ast_expr *AccessExpr = isl_id_to_ast_expr_get(NewAccesses, MA.getId());
 
   if (AccessExpr) {
     AccessExpr = isl_ast_expr_address_of(AccessExpr);
     auto Address = ExprBuilder->create(AccessExpr);
 
if (OldPtrTy != NewPtrTy) {
       assert(OldPtrTy->getPointerElementType()->getPrimitiveSizeInBits() ==
                  NewPtrTy->getPointerElementType()->getPrimitiveSizeInBits() &&
              "Pointer types to elements with different size found");
       Address = Builder.CreateBitOrPointerCast(Address, OldPtrTy);
     }
     return Address;
   }
 
-  return getNewValue(Stmt, Pointer, BBMap, LTS, getLoopForInst(Inst));
+  return getNewValue(Stmt, Inst.getPointerOperand(), BBMap, LTS,
+                     getLoopForInst(Inst));
 }
 
 Loop *BlockGenerator::getLoopForInst(const llvm::Instruction *Inst) {
   return LI.getLoopFor(Inst->getParent());
 }
 
 Value *BlockGenerator::generateScalarLoad(ScopStmt &Stmt, LoadInst *Load,
                                           ValueMapT &BBMap, LoopToScevMapT &LTS,
                                           isl_id_to_ast_expr *NewAccesses) {
   if (Value *PreloadLoad = GlobalMap.lookup(Load))
     return PreloadLoad;
 
-  auto *Pointer = Load->getPointerOperand();
   Value *NewPointer =
-      generateLocationAccessed(Stmt, Load, Pointer, BBMap, LTS, NewAccesses);
+      generateLocationAccessed(Stmt, Load, BBMap, LTS, NewAccesses);
   Value *ScalarLoad = Builder.CreateAlignedLoad(
       NewPointer, Load->getAlignment(), Load->getName() + "_p_scalar_");
 
   if (DebugPrinting)
     RuntimeDebugBuilder::createCPUPrinter(Builder, "Load from ", NewPointer,
                                           ": ", ScalarLoad, "\n");
 
   return ScalarLoad;
 }
 
 void BlockGenerator::generateScalarStore(ScopStmt &Stmt, StoreInst *Store,
                                          ValueMapT &BBMap, LoopToScevMapT &LTS,
                                          isl_id_to_ast_expr *NewAccesses) {
-  auto *Pointer = Store->getPointerOperand();
   Value *NewPointer =
-      generateLocationAccessed(Stmt, Store, Pointer, BBMap, LTS, NewAccesses);
+      generateLocationAccessed(Stmt, Store, BBMap, LTS, NewAccesses);
   Value *ValueOperand = getNewValue(Stmt, Store->getValueOperand(), BBMap, LTS,
                                     getLoopForInst(Store));
 
   if (DebugPrinting)
     RuntimeDebugBuilder::createCPUPrinter(Builder, "Store to  ", NewPointer,
                                           ": ", ValueOperand, "\n");
 
   Builder.CreateAlignedStore(ValueOperand, NewPointer, Store->getAlignment());
Value *VectorBlockGenerator::generateStrideOneLoad(
     ScopStmt &Stmt, LoadInst *Load, VectorValueMapT &ScalarMaps,
     __isl_keep isl_id_to_ast_expr *NewAccesses, bool NegativeStride = false) {
   unsigned VectorWidth = getVectorWidth();
   auto *Pointer = Load->getPointerOperand();
   Type *VectorPtrType = getVectorPtrTy(Pointer, VectorWidth);
   unsigned Offset = NegativeStride ? VectorWidth - 1 : 0;
 
   Value *NewPointer = nullptr;
-  NewPointer = generateLocationAccessed(Stmt, Load, Pointer, ScalarMaps[Offset],
+  NewPointer = generateLocationAccessed(Stmt, Load, ScalarMaps[Offset],
                                         VLTS[Offset], NewAccesses);
   Value *VectorPtr =
       Builder.CreateBitCast(NewPointer, VectorPtrType, "vector_ptr");
   LoadInst *VecLoad =
       Builder.CreateLoad(VectorPtr, Load->getName() + "_p_vec_full");
   if (!Aligned)
     VecLoad->setAlignment(8);
 
Value *VectorBlockGenerator::generateStrideOneLoad(
   return VecLoad;
 }
 
 Value *VectorBlockGenerator::generateStrideZeroLoad(
     ScopStmt &Stmt, LoadInst *Load, ValueMapT &BBMap,
     __isl_keep isl_id_to_ast_expr *NewAccesses) {
   auto *Pointer = Load->getPointerOperand();
   Type *VectorPtrType = getVectorPtrTy(Pointer, 1);
-  Value *NewPointer = generateLocationAccessed(Stmt, Load, Pointer, BBMap,
-                                               VLTS[0], NewAccesses);
+  Value *NewPointer =
+      generateLocationAccessed(Stmt, Load, BBMap, VLTS[0], NewAccesses);
   Value *VectorPtr = Builder.CreateBitCast(NewPointer, VectorPtrType,
                                            Load->getName() + "_p_vec_p");
   LoadInst *ScalarLoad =
       Builder.CreateLoad(VectorPtr, Load->getName() + "_p_splat_one");
 
   if (!Aligned)
     ScalarLoad->setAlignment(8);
 
Value *VectorBlockGenerator::generateUnknownStrideLoad(
   int VectorWidth = getVectorWidth();
   auto *Pointer = Load->getPointerOperand();
   VectorType *VectorType = VectorType::get(
       dyn_cast<PointerType>(Pointer->getType())->getElementType(), VectorWidth);
 
   Value *Vector = UndefValue::get(VectorType);
 
   for (int i = 0; i < VectorWidth; i++) {
-    Value *NewPointer = generateLocationAccessed(
-        Stmt, Load, Pointer, ScalarMaps[i], VLTS[i], NewAccesses);
+    Value *NewPointer = generateLocationAccessed(Stmt, Load, ScalarMaps[i],
+                                                 VLTS[i], NewAccesses);
     Value *ScalarLoad =
         Builder.CreateLoad(NewPointer, Load->getName() + "_p_scalar_");
     Vector = Builder.CreateInsertElement(
         Vector, ScalarLoad, Builder.getInt32(i), Load->getName() + "_p_vec_");
   }
 
   return Vector;
 }
Value *Vector = getVectorValue(Stmt, Store->getValueOperand(), VectorMap,
                                  ScalarMaps, getLoopForInst(Store));
 
   // Make sure we have scalar values available to access the pointer to
   // the data location.
   extractScalarValues(Store, VectorMap, ScalarMaps);
 
   if (Access.isStrideOne(isl_map_copy(Schedule))) {
     Type *VectorPtrType = getVectorPtrTy(Pointer, getVectorWidth());
-    Value *NewPointer = generateLocationAccessed(
-        Stmt, Store, Pointer, ScalarMaps[0], VLTS[0], NewAccesses);
+    Value *NewPointer = generateLocationAccessed(Stmt, Store, ScalarMaps[0],
+                                                 VLTS[0], NewAccesses);
 
     Value *VectorPtr =
         Builder.CreateBitCast(NewPointer, VectorPtrType, "vector_ptr");
     StoreInst *Store = Builder.CreateStore(Vector, VectorPtr);
 
     if (!Aligned)
       Store->setAlignment(8);
   } else {
     for (unsigned i = 0; i < ScalarMaps.size(); i++) {
       Value *Scalar = Builder.CreateExtractElement(Vector, Builder.getInt32(i));
-      Value *NewPointer = generateLocationAccessed(
-          Stmt, Store, Pointer, ScalarMaps[i], VLTS[i], NewAccesses);
+      Value *NewPointer = generateLocationAccessed(Stmt, Store, ScalarMaps[i],
+                                                   VLTS[i], NewAccesses);
       Builder.CreateStore(Scalar, NewPointer);
     }
   }
 }
 
 bool VectorBlockGenerator::hasVectorOperands(const Instruction *Inst,
                                              ValueMapT &VectorMap) {
   for (Value *Operand : Inst->operands())

lib/CodeGen/IRBuilder.cpp

void ScopAnnotator::annotate(Instruction *Inst) {
 
   if (!ParallelLoops.empty())
     Inst->setMetadata("llvm.mem.parallel_loop_access", ParallelLoops.back());
 
   // TODO: Use the ScopArrayInfo once available here.
   if (!AliasScopeDomain)
     return;
 
-  if (!(isa<StoreInst>(Inst) || isa<LoadInst>(Inst)))
+  auto MemInst = MemAccInst::dyn_cast(Inst);
+  if (!MemInst)
     return;
 
-  auto *PtrSCEV = SE->getSCEV(getPointerOperand(*Inst));
+  auto *PtrSCEV = SE->getSCEV(MemInst.getPointerOperand());
   auto *BaseSCEV = SE->getPointerBase(PtrSCEV);
   auto *SU = dyn_cast<SCEVUnknown>(BaseSCEV);
 
   if (!SU)
     return;
 
   auto *BasePtr = SU->getValue();
 

lib/CodeGen/IslNodeBuilder.cpp

for (auto *Val : Values) {
       if (auto *Inst = dyn_cast<Instruction>(Val)) {
         if (S.getRegion().contains(Inst)) {
           bool IsDead = true;
 
           // Check for "undef" loads first, then if there is a statement for
           // the parent of Inst and lastly if the parent of Inst has an empty
           // domain. In the first and last case the instruction is dead but if
           // there is a statement or the domain is not empty Inst is not dead.
-          auto *Address = getPointerOperand(*Inst);
+          auto MemInst = MemAccInst::dyn_cast(Inst);
+          auto Address = MemInst ? MemInst.getPointerOperand() : nullptr;
           if (Address &&
               SE.getUnknown(UndefValue::get(Address->getType())) ==
                   SE.getPointerBase(SE.getSCEV(Address))) {
           } else if (S.getStmtForBasicBlock(Inst->getParent())) {
             IsDead = false;
           } else {
             auto *Domain = S.getDomainConditions(Inst->getParent());
             IsDead = isl_set_is_empty(Domain);

lib/Support/ScopHelper.cpp

 #include "llvm/Support/Debug.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 
 using namespace llvm;
 using namespace polly;
 
 #define DEBUG_TYPE "polly-scop-helper"
 
-Value *polly::getPointerOperand(Instruction &Inst) {
-  if (LoadInst *load = dyn_cast<LoadInst>(&Inst))
-    return load->getPointerOperand();
-  else if (StoreInst *store = dyn_cast<StoreInst>(&Inst))
-    return store->getPointerOperand();
-  else if (GetElementPtrInst *gep = dyn_cast<GetElementPtrInst>(&Inst))
-    return gep->getPointerOperand();
-
-  return 0;
-}
-
 bool polly::hasInvokeEdge(const PHINode *PN) {
   for (unsigned i = 0, e = PN->getNumIncomingValues(); i < e; ++i)
     if (InvokeInst *II = dyn_cast<InvokeInst>(PN->getIncomingValue(i)))
       if (II->getParent() == PN->getIncomingBlock(i))
         return true;
 
   return false;
 }