[PATCH] [Polly] Introduce the ScopArrayInfo class.

[Johannes Doerfert]

Responses. I would try to change the code according to the comments but I still like the changes to the tests ({{(i32}i64)}} instread of i32)

================
Comment at: include/polly/CodeGen/IslExprBuilder.h:84
@@ -83,2 +83,3 @@
   ///
+  /// @param S       The current SCoP.
   /// @param Builder The IRBuilder used to construct the isl_ast_expr[ession].
----------------
grosser wrote:
> It is unclear to me why the IslExpr Builder would need a SCoP now.
To access the ScopInfoArray for a base pointer.

================
Comment at: lib/Analysis/ScopInfo.cpp:475
@@ -444,1 +474,3 @@
+  void *User = const_cast<ScopArrayInfo *>(SAI);
+  isl_id *BaseAddrId = isl_id_alloc(Ctx, getBaseName().c_str(), User);
 
----------------
grosser wrote:
> Conceptually it probably makes sense to allocate one isl_id in the ScopArrayInfo class and then give users the option to obtain copies of this id. This makes clear that all these isl_ids are identical. (The current code also works at the moment AFAIK, as isl assumes two ids are identical if their name and the user pointer are identical).
I'll change that. Good point.

================
Comment at: lib/CodeGen/IslCodeGeneration.cpp:58
@@ -58,5 +57,3 @@
 public:
-  IslNodeBuilder(PollyIRBuilder &Builder, ScopAnnotator &Annotator, Pass *P,
-                 LoopInfo &LI, ScalarEvolution &SE, DominatorTree &DT)
-      : Builder(Builder), Annotator(Annotator), ExprBuilder(Builder, IDToValue),
-        P(P), LI(LI), SE(SE), DT(DT) {}
+  IslNodeBuilder(Scop &S, PollyIRBuilder &Builder, ScopAnnotator &Annotator,
+                 Pass *P, LoopInfo &LI, ScalarEvolution &SE, DominatorTree &DT)
----------------
grosser wrote:
> I am surprised we need the Scop here.
Just to pass it on to the IslExprBuilder.

================
Comment at: lib/CodeGen/IslExprBuilder.cpp:113
@@ +112,3 @@
+
+  const ScopArrayInfo *SAI = S.getScopArrayInfo(Base);
+  if (Base->getType() != SAI->getType())
----------------
grosser wrote:
> Instead of passing the scop to this place and use the base ptr to look up the ScopArrayInfo, you could possibly also obtain the isl_id from the isl_ast_expr_get_op_arg(Expr, 0) and then use a "static ScopArrayInfo::getFromid(Id)" function.
> 
> Also, instead of obtaining the Base pointer by calling create(), we could obtain
> it directly from this ScopArrayInfo object. This would allow us to remove 
> IslNodeBuilder::addMemoryAccesses(), which was mostly added due to us missing a ScopArrayInfo class.
True, I'll try that and keep it if it works

================
Comment at: test/Isl/CodeGen/MemAccess/codegen_simple.ll:43
@@ -42,2 +42,2 @@
 }
-; CHECK: load i32* getelementptr inbounds ([100 x i32]* @A, i64 0, i64 0)
+; CHECK: load i32* getelementptr inbounds ([100 x i32]* @A, i{{(32|64)}} 0, i{{(32|64)}} 0)
----------------
grosser wrote:
> It is unclear to me why this test case needs to be modified and especially why the generated IR now can be have one of two forms. Is the IR we generate not supposed to be predictable?
It has a different form now because for A[100]
  CreateBitcast(A[0]) == yields ==> getelemenptr inbounds [100 x i32]* @A, i32 0, i32 0
where the indices are of type i32.

Before the patch we added the indices and used the typ of the only real index op to create "zeros" (thus they were i64). Now I don't want to hardcode i32 because a change in the bitcast function or in our code could revert this or mix the indice types **without** a semantical change, thus I don't see the reason to fix one type here.

http://reviews.llvm.org/D5613