[PATCH] D5227: [Polly] Support ModRef function behaviour

Hongbin Zheng via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 22 14:46:41 PST 2016


etherzhhb added a comment.

Is there a test to demonstrate "global reads" CallInsts that read all arrays?


================
Comment at: include/polly/ScopInfo.h:330-331
@@ -329,1 +329,4 @@
 
+  /// @brief Is this array info modeling an array?
+  bool isArrayKind() const { return Kind == MK_Array; };
+
----------------
jdoerfert wrote:
> etherzhhb wrote:
> > This change can be pushed directly
> We try not to introduce unused methodes, thus I would like to push it with a user.
ok

================
Comment at: include/polly/Support/ScopHelper.h:78
@@ -77,2 +77,3 @@
   /* implicit */ MemAccInst(llvm::MemIntrinsic *MI) : I(MI) {}
+  /* implicit */ MemAccInst(llvm::CallInst *CI) : I(CI) {}
   explicit MemAccInst(llvm::Instruction &I) : I(&I) { assert(isa(I)); }
----------------
jdoerfert wrote:
> etherzhhb wrote:
> > A little bit weird, I think CallInst is too different from the other instruction above. Could we not mix CallInst with the other MemAccInst? 
> > Could we not mix CallInst with the other MemAccInst?
> 
> If we do not want to duplicate all methodes that take a MemAccInst, I guess we cannot.
> 
> > A little bit weird, I think CallInst is too different from the other instruction above. 
> 
> I would say the step from MemIntrinsic to CallInst is not that big.
> If we do not want to duplicate all methodes that take a MemAccInst, I guess we cannot.
If there is no better solution, I think this change is ok

================
Comment at: include/polly/Support/ScopHelper.h:192
@@ -180,1 +191,3 @@
+    if (isCallInst())
+      return nullptr;
     llvm_unreachable("Operation not supported on nullptr");
----------------
jdoerfert wrote:
> etherzhhb wrote:
> > This is very confusing as a CallInst can have multiple pointer operands if the call read/write the location of multiples pointer arguments
> But that "problem" is already present for MemIntrinsics. Some have 2 pointer operands but we allow only access to one of them through this interface. One has to cast it to a MemTransferInst to access the other.
Ok, we can figure out a better solution for this (if there is any) later.


http://reviews.llvm.org/D5227





More information about the llvm-commits mailing list