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

[Meinersbur]

include/polly/Support/ScopHelper.h

81 ↗

(On Diff #47939)

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 ↗

(On Diff #47939)

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 ↗

(On Diff #47939)

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 ↗

(On Diff #47939)

No StaticAssertLegalT here?

106 ↗

(On Diff #47939)

That's want I suggested an planned as well.

115 ↗

(On Diff #47939)

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

126 ↗

(On Diff #47939)

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

147 ↗

(On Diff #47939)

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 ↗

(On Diff #47939)

ok

86 ↗

(On Diff #47939)

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 ↗

(On Diff #47939)

ok

102 ↗

(On Diff #47939)

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

115 ↗

(On Diff #47939)

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 ↗

(On Diff #47939)

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

619 ↗

(On Diff #49190)

Does this work was well?

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

[etherzhhb]

lib/Analysis/ScopInfo.cpp

619 ↗

(On Diff #49190)

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

619 ↗

(On Diff #49190)

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

619 ↗

(On Diff #49190)

ok

polly/trunk/include/polly/Support/ScopHelper.h

if (isMemIntrinsic())
       return !asMemIntrinsic()->isVolatile();
     if (isCallInst())
       return true;
     llvm_unreachable("Operation not supported on nullptr");
   }
 
   bool isNull() const { return !I; }
   bool isInstruction() const { return I; }
+
+  llvm::Instruction *asInstruction() const { return I; }
+
+private:
   bool isLoad() const { return I && llvm::isa<llvm::LoadInst>(I); }
   bool isStore() const { return I && llvm::isa<llvm::StoreInst>(I); }
   bool isCallInst() const { return I && llvm::isa<llvm::CallInst>(I); }
   bool isMemIntrinsic() const { return I && llvm::isa<llvm::MemIntrinsic>(I); }
   bool isMemSetInst() const { return I && llvm::isa<llvm::MemSetInst>(I); }
   bool isMemTransferInst() const {
     return I && llvm::isa<llvm::MemTransferInst>(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); }
   llvm::CallInst *asCallInst() const { return llvm::cast<llvm::CallInst>(I); }
   llvm::MemIntrinsic *asMemIntrinsic() const {
     return llvm::cast<llvm::MemIntrinsic>(I);
   }
   llvm::MemSetInst *asMemSetInst() const {
     return llvm::cast<llvm::MemSetInst>(I);
   }
   llvm::MemTransferInst *asMemTransferInst() const {
     return llvm::cast<llvm::MemTransferInst>(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.

polly/trunk/lib/Analysis/ScopDetection.cpp

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.isStore();
-    Context.hasLoads |= MemInst.isLoad();
+    Context.hasStores |= isa<StoreInst>(MemInst);
+    Context.hasLoads |= isa<LoadInst>(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.

polly/trunk/lib/Analysis/ScopInfo.cpp

void MemoryAccess::assumeNoOutOfBound() {
   Outside = isl_set_complement(Outside);
   Statement->getParent()->addAssumption(
       INBOUNDS, Outside,
       getAccessInstruction() ? getAccessInstruction()->getDebugLoc() : nullptr);
   isl_space_free(Space);
 }
 
 void MemoryAccess::buildMemIntrinsicAccessRelation() {
-  auto MAI = MemAccInst(getAccessInstruction());
-  (void)MAI;
-  assert(MAI.isMemIntrinsic());
+  assert(isa<MemIntrinsic>(getAccessInstruction()));
   assert(Subscripts.size() == 2 && Sizes.size() == 0);
 
   auto *SubscriptPWA = Statement->getPwAff(Subscripts[0]);
   auto *SubscriptMap = isl_map_from_pw_aff(SubscriptPWA);
 
   isl_map *LengthMap;
   if (Subscripts[1] == nullptr) {
     LengthMap = isl_map_universe(isl_map_get_space(SubscriptMap));
void MemoryAccess::buildMemIntrinsicAccessRelation() {
   AccessRelation = isl_map_set_tuple_id(LengthMap, isl_dim_in,
                                         getStatement()->getDomainId());
 }
 
 void MemoryAccess::computeBoundsOnAccessRelation(unsigned ElementSize) {
   ScalarEvolution *SE = Statement->getParent()->getSE();
 
   auto MAI = MemAccInst(getAccessInstruction());
-  if (MAI.isMemIntrinsic())
+  if (isa<MemIntrinsic>(MAI))
     return;
 
   Value *Ptr = MAI.getPointerOperand();
   if (!Ptr || !SE->isSCEVable(Ptr->getType()))
     return;
 
   auto *PtrSCEV = SE->getSCEV(Ptr);
   if (isa<SCEVCouldNotCompute>(PtrSCEV))
if (StmtDomainEmpty)
       continue;
 
     for (MemoryAccess *MA : Stmt) {
       if (MA->isScalarKind())
         continue;
       if (!MA->isRead())
         HasWriteAccess.insert(MA->getBaseAddr());
       MemAccInst Acc(MA->getAccessInstruction());
-      if (MA->isRead() && Acc.isMemTransferInst())
-        PtrToAcc[Acc.asMemTransferInst()->getSource()] = MA;
+      if (MA->isRead() && isa<MemTransferInst>(Acc))
+        PtrToAcc[cast<MemTransferInst>(Acc)->getSource()] = MA;
       else
         PtrToAcc[Acc.getPointerOperand()] = MA;
       AST.add(Acc);
     }
   }
 
   SmallVector<AliasGroupTy, 4> AliasGroups;
   for (AliasSet &AS : AST) {
bool ScopInfo::buildAccessMultiDimFixed(
     const InvariantLoadsSetTy &ScopRIL) {
   Value *Val = Inst.getValueOperand();
   Type *ElementType = Val->getType();
   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 (auto *BitCast = dyn_cast<BitCastInst>(Address)) {
     auto *Src = BitCast->getOperand(0);
     auto *SrcTy = Src->getType();
     auto *DstTy = BitCast->getType();
     if (SrcTy->getPrimitiveSizeInBits() == DstTy->getPrimitiveSizeInBits())
       Address = Src;
   }
bool ScopInfo::buildAccessMultiDimParam(
   if (!PollyDelinearize)
     return false;
 
   Value *Address = Inst.getPointerOperand();
   Value *Val = Inst.getValueOperand();
   Type *ElementType = Val->getType();
   unsigned ElementSize = DL->getTypeAllocSize(ElementType);
   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);
 
addArrayAccess(Inst, Type, BasePointer->getValue(), ElementType, true,
                  AccItr->second.DelinearizedSubscripts, Sizes, Val);
   return true;
 }
 
 bool ScopInfo::buildAccessMemIntrinsic(
     MemAccInst Inst, Loop *L, Region *R,
     const ScopDetection::BoxedLoopsSetTy *BoxedLoops,
     const InvariantLoadsSetTy &ScopRIL) {
-  if (!Inst.isMemIntrinsic())
+  auto *MemIntr = dyn_cast_or_null<MemIntrinsic>(Inst);
+
+  if (MemIntr == nullptr)
     return false;
 
-  auto *LengthVal = SE->getSCEVAtScope(Inst.asMemIntrinsic()->getLength(), L);
+  auto *LengthVal = SE->getSCEVAtScope(MemIntr->getLength(), L);
   assert(LengthVal);
 
   // Check if the length val is actually affine or if we overapproximate it
   InvariantLoadsSetTy AccessILS;
   bool LengthIsAffine = isAffineExpr(R, LengthVal, *SE, nullptr, &AccessILS);
   for (LoadInst *LInst : AccessILS)
     if (!ScopRIL.count(LInst))
       LengthIsAffine = false;
   if (!LengthIsAffine)
     LengthVal = nullptr;
 
-  auto *DestPtrVal = Inst.asMemIntrinsic()->getDest();
+  auto *DestPtrVal = MemIntr->getDest();
   assert(DestPtrVal);
   auto *DestAccFunc = SE->getSCEVAtScope(DestPtrVal, L);
   assert(DestAccFunc);
   auto *DestPtrSCEV = dyn_cast<SCEVUnknown>(SE->getPointerBase(DestAccFunc));
   assert(DestPtrSCEV);
   DestAccFunc = SE->getMinusSCEV(DestAccFunc, DestPtrSCEV);
   addArrayAccess(Inst, MemoryAccess::MUST_WRITE, DestPtrSCEV->getValue(),
                  IntegerType::getInt8Ty(DestPtrVal->getContext()), false,
                  {DestAccFunc, LengthVal}, {}, Inst.getValueOperand());
 
-  if (!Inst.isMemTransferInst())
+  auto *MemTrans = dyn_cast<MemTransferInst>(MemIntr);
+  if (!MemTrans)
     return true;
 
-  auto *SrcPtrVal = Inst.asMemTransferInst()->getSource();
+  auto *SrcPtrVal = MemTrans->getSource();
   assert(SrcPtrVal);
   auto *SrcAccFunc = SE->getSCEVAtScope(SrcPtrVal, L);
   assert(SrcAccFunc);
   auto *SrcPtrSCEV = dyn_cast<SCEVUnknown>(SE->getPointerBase(SrcAccFunc));
   assert(SrcPtrSCEV);
   SrcAccFunc = SE->getMinusSCEV(SrcAccFunc, SrcPtrSCEV);
   addArrayAccess(Inst, MemoryAccess::READ, SrcPtrSCEV->getValue(),
                  IntegerType::getInt8Ty(SrcPtrVal->getContext()), false,
                  {SrcAccFunc, LengthVal}, {}, Inst.getValueOperand());
 
   return true;
 }
 
 bool ScopInfo::buildAccessCallInst(
     MemAccInst Inst, Loop *L, Region *R,
     const ScopDetection::BoxedLoopsSetTy *BoxedLoops,
     const InvariantLoadsSetTy &ScopRIL) {
-  if (!Inst.isCallInst())
+  auto *CI = dyn_cast_or_null<CallInst>(Inst);
+
+  if (CI == nullptr)
     return false;
 
-  auto &CI = *Inst.asCallInst();
-  if (CI.doesNotAccessMemory() || isIgnoredIntrinsic(&CI))
+  if (CI->doesNotAccessMemory() || isIgnoredIntrinsic(CI))
     return true;
 
   bool ReadOnly = false;
-  auto *AF = SE->getConstant(IntegerType::getInt64Ty(CI.getContext()), 0);
-  auto *CalledFunction = CI.getCalledFunction();
+  auto *AF = SE->getConstant(IntegerType::getInt64Ty(CI->getContext()), 0);
+  auto *CalledFunction = CI->getCalledFunction();
   switch (AA->getModRefBehavior(CalledFunction)) {
   case llvm::FMRB_UnknownModRefBehavior:
     llvm_unreachable("Unknown mod ref behaviour cannot be represented.");
   case llvm::FMRB_DoesNotAccessMemory:
     return true;
   case llvm::FMRB_OnlyReadsMemory:
-    GlobalReads.push_back(&CI);
+    GlobalReads.push_back(CI);
     return true;
   case llvm::FMRB_OnlyReadsArgumentPointees:
     ReadOnly = true;
   // Fall through
   case llvm::FMRB_OnlyAccessesArgumentPointees:
     auto AccType = ReadOnly ? MemoryAccess::READ : MemoryAccess::MAY_WRITE;
-    for (const auto &Arg : CI.arg_operands()) {
+    for (const auto &Arg : CI->arg_operands()) {
       if (!Arg->getType()->isPointerTy())
         continue;
 
       auto *ArgSCEV = SE->getSCEVAtScope(Arg, L);
       if (ArgSCEV->isZero())
         continue;
 
       auto *ArgBasePtr = cast<SCEVUnknown>(SE->getPointerBase(ArgSCEV));
       addArrayAccess(Inst, AccType, ArgBasePtr->getValue(),
-                     ArgBasePtr->getType(), false, {AF}, {}, &CI);
+                     ArgBasePtr->getType(), false, {AF}, {}, CI);
     }
     return true;
   }
 
   return true;
 }
 
 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 *ElementType = Val->getType();
   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);