[Polly] [Refactor] Enable llvm's isa/cast/dyn_cast on MemAccInst

[Meinersbur]

include/polly/Support/ScopHelper.h

81

The distinction between implicit an explicit constructors is lost with this. In particular, I'd avoid an arbitrary Instruction to be converted to MemAccInst without a kind of explicit 'cast'. This makes a execution-time error out of it, including if passing someting that is not even an Instruction.

86

classof is intended for LLVM's object hierarchy and MemAccInst is no part of it. It is not derived from llvm::Value or similar. That means eg.

LoadInst *LI;
cast<MemAccInst>(LI)

will go horribly wrong.

97

This scheme doesn't work in general when we want something implicitly converted to the argument. Though, in this case, because nothing derives from LoadInst or StoreInst, there might be no such case. I'd still prefer the previous for readability.

102

No StaticAssertLegalT here?

106

That's want I suggested an planned as well.

115

Is there any advantage of this scheme that two ifs? Both have 5 lines.

126

I'd rather avoid using macros, but still OK.

147

Wow, I didn't know this is possible.

[Meinersbur]

I like the operator->() and simplify_type, but am sceptic about the rest.

[etherzhhb]

Will refine the patch

include/polly/Support/ScopHelper.h

81

ok

86

It works if we introduce simplify_type, and I actually experimented this and confirm it work.

I think it should be

return I && (llvm::isa<llvm::LoadInst>(I) || llvm::isa<llvm::StoreInst>(I));

97

ok

102

We call "operator=(T *)" which has the StaticAssertLegalT

115

I dont think it has advantage, this together with the following macro are copied from CallSite... I first copy here then copy to the following marco

126

We may think about using template to do the same thing when we have time.

[grosser]

I leave these patch to you guys.

[Meinersbur]

Still working in it? Due to MemIntrinsics, the macroization/templatization doesn't work anymore?.?

[etherzhhb]

In D17250#361612, @Meinersbur wrote:

Still working in it? Due to MemIntrinsics, the macroization/templatization doesn't work anymore?.?

Originally, I actually waiting @jdoerfert to checkin his patches and work on it. Now I can continue

[jdoerfert]

Sorry for the dealy. I didn't want to block you guys.

[etherzhhb]

not blocking, I just think this one is not as high priority as yours, so I
just wait to avoid unnecessary rebase on your side

[etherzhhb]

Updated

[Meinersbur]

LGTM

lib/Analysis/ScopInfo.cpp

592–593

Does this work was well?

assert(isa<MemIntrinsic>(getAccessInstruction()));

[etherzhhb]

lib/Analysis/ScopInfo.cpp

592–593

if getAccessInstruction() return nullptr.

assert(isa<MemIntrinsic>(MemAccInst(getAccessInstruction())));

Trigger an assertion failure.

assert(isa<MemIntrinsic>(getAccessInstruction()));

Trigger a SIGSEGV (suppose to ... no sure)

Maybe we should write:

assert(getAccessInstruction() && isa<MemIntrinsic>(getAccessInstruction()));

[Meinersbur]

lib/Analysis/ScopInfo.cpp

592–593

isa<XYZ*>(nullptr) triggers the following assertion:

assert(Val && "isa<> used on a null pointer");

in llvm/Support/Casting.h:81/88/95/102

[etherzhhb]

lib/Analysis/ScopInfo.cpp

592–593

ok

include/polly/Support/ScopHelper.h

 /// 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;
 
+  template <typename T> inline static T *StaticAssertLegalT(T *V) {
+    static_assert(std::is_same<T, llvm::LoadInst>::value &&
+                      std::is_same<T, llvm::StoreInst>::value,
+                  "Only LoadInst and StoreInst are supported!");
+    return V;
+  }
+
 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(MemAccInst &Inst) : I(Inst.I) {}
+
+  template <typename T>
+  /* implicit */ MemAccInst(T *I)
+      : I((I && llvm::isa<MemAccInst>(I)) ? I : nullptr) {}
+  template <typename T>
+  /* implicit */ MemAccInst(T &I) : MemAccInst(&I) {}
+
+  static bool classof(const llvm::Value *I) {
+    return llvm::isa<llvm::LoadInst>(I) || llvm::isa<llvm::StoreInst>(I);
   }
+  static bool classof(const llvm::LoadInst *) { return true; }
+  static bool classof(const llvm::StoreInst *) { return true; }
 
   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;
+
+  template <typename T> MemAccInst &operator=(T *RHS) {
+    I = StaticAssertLegalT<T>(RHS);
     return *this;
   }
-  MemAccInst &operator=(llvm::StoreInst *SI) {
-    I = SI;
-    return *this;
+
+  template <typename T> MemAccInst &operator=(T &RHS) {
+    return operator=<T>(&RHS);
   }
 
   operator llvm::Instruction *() const { return asInstruction(); }
+  llvm::Instruction *operator->() 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 {
+    assert(!isNull() && "Operation not supported on nullptr");
+    if (auto LI = llvm::dyn_cast_or_null<llvm::LoadInst>(asInstruction()))
+      return LI;
+
+    return llvm::cast<llvm::StoreInst>(asInstruction())->getValueOperand();
   }
 
-  /// @brief Get the debug location of this instruction.
-  ///
-  /// @returns The debug location of this instruction.
-  const llvm::DebugLoc &getDebugLoc() const {
-    if (I)
-      return I->getDebugLoc();
-    llvm_unreachable("Operation not supported on nullptr");
-  }
+#define MEMACCINST_DELEGATE_GETTER(METHOD)                                     \
+  assert(!isNull() && "Operation not supported on nullptr");                   \
+  if (auto LI = llvm::dyn_cast_or_null<llvm::LoadInst>(asInstruction()))       \
+    return LI->METHOD;                                                         \
+                                                                               \
+  return llvm::cast<llvm::StoreInst>(asInstruction())->METHOD;
 
-  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");
+    MEMACCINST_DELEGATE_GETTER(getPointerOperand());
   }
+
+  unsigned getAlignment() const { MEMACCINST_DELEGATE_GETTER(getAlignment()); }
+  bool isVolatile() const { MEMACCINST_DELEGATE_GETTER(isVolatile()); }
+  bool isSimple() const { MEMACCINST_DELEGATE_GETTER(isSimple()); }
   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");
+    MEMACCINST_DELEGATE_GETTER(getOrdering());
   }
+  bool isUnordered() const { MEMACCINST_DELEGATE_GETTER(isUnordered()); }
 
   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); }
 };
+}
 
+namespace llvm {
+/// @brief Specialize simplify_type for MemAccInst to enable dyn_cast and cast
+///        from a MemAccInst object.
+template <> struct simplify_type<polly::MemAccInst> {
+  typedef Instruction *SimpleType;
+  static SimpleType getSimplifiedValue(polly::MemAccInst &I) {
+    return I.asInstruction();
+  }
+};
+}
+
+namespace polly {
 /// @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);

lib/Analysis/ScopDetection.cpp

bool ScopDetection::hasAffineMemoryAccesses(DetectionContext &Context) const {
   return true;
 }
 
 bool ScopDetection::isValidMemoryAccess(MemAccInst Inst,
                                         DetectionContext &Context) const {
   Region &CurRegion = Context.CurRegion;
 
   Value *Ptr = Inst.getPointerOperand();
-  Loop *L = LI->getLoopFor(Inst.getParent());
+  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);
if (IntToPtrInst *Inst = dyn_cast<IntToPtrInst>(BaseValue))
     return invalid<ReportIntToPtr>(Context, /*Assert=*/true, Inst);
 
   if (IgnoreAliasing)
     return true;
 
   // Check if the base pointer of the memory access does alias with
   // any other pointer. This cannot be handled at the moment.
   AAMDNodes AATags;
-  Inst.getAAMetadata(AATags);
+  Inst->getAAMetadata(AATags);
   AliasSet &AS = Context.AST.getAliasSetForPointer(
       BaseValue, MemoryLocation::UnknownSize, AATags);
 
   if (!AS.isMustAlias()) {
     if (PollyUseRuntimeAliasChecks) {
       bool CanBuildRunTimeCheck = true;
       // The run-time alias check places code that involves the base pointer at
       // the beginning of the SCoP. This breaks if the base pointer is defined
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 (auto MemInst = MemAccInst::dyn_cast(Inst)) {
-    Context.hasStores |= MemInst.isLoad();
-    Context.hasLoads |= MemInst.isStore();
+  if (MemAccInst MemInst = Inst) {
+    Context.hasStores |= isa<LoadInst>(MemInst);
+    Context.hasLoads |= isa<StoreInst>(MemInst);
     if (!MemInst.isSimple())
       return invalid<ReportNonSimpleMemoryAccess>(Context, /*Assert=*/true,
                                                   &Inst);
 
     return isValidMemoryAccess(MemInst, Context);
   }
 
   // We do not know this instruction, therefore we assume it is invalid.

lib/Analysis/ScopInfo.cpp

void MemoryAccess::assumeNoOutOfBound() {
   // bail out more often than strictly necessary.
   Outside = isl_set_remove_divs(Outside);
   Outside = isl_set_complement(Outside);
   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 = MemAccInst(getAccessInstruction()).getPointerOperand();
   if (!Ptr || !SE->isSCEVable(Ptr->getType()))
     return;
 
   auto *PtrSCEV = SE->getSCEV(Ptr);
   if (isa<SCEVCouldNotCompute>(PtrSCEV))
bool ScopInfo::buildAccessMultiDimFixed(
   Value *Val = Inst.getValueOperand();
   Type *SizeType = Val->getType();
   unsigned ElementSize = DL->getTypeAllocSize(SizeType);
   Value *Address = Inst.getPointerOperand();
   const SCEV *AccessFunction = SE->getSCEVAtScope(Address, L);
   const SCEVUnknown *BasePointer =
       dyn_cast<SCEVUnknown>(SE->getPointerBase(AccessFunction));
   enum MemoryAccess::AccessType Type =
-      Inst.isLoad() ? MemoryAccess::READ : MemoryAccess::MUST_WRITE;
+      isa<LoadInst>(Inst) ? MemoryAccess::READ : MemoryAccess::MUST_WRITE;
 
   if (isa<GetElementPtrInst>(Address) || isa<BitCastInst>(Address)) {
     auto NewAddress = Address;
     if (auto *BitCast = dyn_cast<BitCastInst>(Address)) {
       auto Src = BitCast->getOperand(0);
       auto SrcTy = Src->getType();
       auto DstTy = BitCast->getType();
       if (SrcTy->getPrimitiveSizeInBits() == DstTy->getPrimitiveSizeInBits())
bool ScopInfo::buildAccessMultiDimParam(
     MemAccInst Inst, Loop *L, Region *R,
     const ScopDetection::BoxedLoopsSetTy *BoxedLoops,
     const InvariantLoadsSetTy &ScopRIL, const MapInsnToMemAcc &InsnToMemAcc) {
   Value *Address = Inst.getPointerOperand();
   Value *Val = Inst.getValueOperand();
   Type *SizeType = Val->getType();
   unsigned ElementSize = DL->getTypeAllocSize(SizeType);
   enum MemoryAccess::AccessType Type =
-      Inst.isLoad() ? MemoryAccess::READ : MemoryAccess::MUST_WRITE;
+      isa<LoadInst>(Inst) ? 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);
 
   auto AccItr = InsnToMemAcc.find(Inst);
   if (PollyDelinearize && AccItr != InsnToMemAcc.end()) {
     std::vector<const SCEV *> Sizes(
         AccItr->second.Shape->DelinearizedSizes.begin(),
         AccItr->second.Shape->DelinearizedSizes.end());
     // Remove the element size. This information is already provided by the
     // ElementSize parameter. In case the element size of this access and the
     // element size used for delinearization differs the delinearization is
     // incorrect. Hence, we invalidate the scop.
     //
     // TODO: Handle delinearization with differing element sizes.
     auto DelinearizedSize =
         cast<SCEVConstant>(Sizes.back())->getAPInt().getSExtValue();
     Sizes.pop_back();
     if (ElementSize != DelinearizedSize)
-      scop->invalidate(DELINEARIZATION, Inst.getDebugLoc());
+      scop->invalidate(DELINEARIZATION, Inst->getDebugLoc());
 
     addArrayAccess(Inst, Type, BasePointer->getValue(), ElementSize, true,
                    AccItr->second.DelinearizedSubscripts, Sizes, Val);
     return true;
   }
   return false;
 }
 
 void ScopInfo::buildAccessSingleDim(
     MemAccInst Inst, Loop *L, Region *R,
     const ScopDetection::BoxedLoopsSetTy *BoxedLoops,
     const InvariantLoadsSetTy &ScopRIL) {
   Value *Address = Inst.getPointerOperand();
   Value *Val = Inst.getValueOperand();
   Type *SizeType = Val->getType();
   unsigned ElementSize = DL->getTypeAllocSize(SizeType);
   enum MemoryAccess::AccessType Type =
-      Inst.isLoad() ? MemoryAccess::READ : MemoryAccess::MUST_WRITE;
+      isa<LoadInst>(Inst) ? 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);
 
for (Instruction &Inst : BB) {
     // 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 (auto MemInst = MemAccInst::dyn_cast(Inst))
+    if (MemAccInst MemInst = Inst)
       buildMemoryAccess(MemInst, L, &R, BoxedLoops, ScopRIL, InsnToMemAcc);
 
     if (isIgnoredIntrinsic(&Inst))
       continue;
 
     if (!PHI)
       buildScalarDependences(&Inst);
     if (!IsExitBlock)

 }
 
 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(MemAccInst.isLoad() == (Type == MemoryAccess::READ));
-  addMemoryAccess(MemAccInst.getParent(), MemAccInst, Type, BaseAddress,
+  assert(isa<LoadInst>(MemAccInst) == (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/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;
 
-  auto MemInst = MemAccInst::dyn_cast(Inst);
+  MemAccInst MemInst(Inst);
   if (!MemInst)
     return;
 
   auto *PtrSCEV = SE->getSCEV(MemInst.getPointerOperand());
   auto *BaseSCEV = SE->getPointerBase(PtrSCEV);
   auto *SU = dyn_cast<SCEVUnknown>(BaseSCEV);
 
   if (!SU)

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 MemInst = MemAccInst::dyn_cast(Inst);
+          MemAccInst MemInst(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());