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

[Johannes Doerfert via llvm-commits]

jdoerfert added inline comments.

================
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; };
+
----------------
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.

================
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)); }
----------------
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.

================
Comment at: include/polly/Support/ScopHelper.h:192
@@ -180,1 +191,3 @@
+    if (isCallInst())
+      return nullptr;
     llvm_unreachable("Operation not supported on nullptr");
----------------
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.


http://reviews.llvm.org/D5227