[Polly] Generating code for changed memory accesses

[Johannes Doerfert]

On 07/25, Tobias Grosser wrote:
> >[This also breaks the little functionality the jscop-cloog interface
> >offered, but I don't think that should stop us at this point.]
> 
> I am OK with breaking CLooG as long as we move the test cases such that
> we both preserve test coverage and do not cause failing test cases.
I moved & updated them to work with the isl codegen backend.

> >+Value *IslExprBuilder::createOpAccess(__isl_take isl_ast_expr *Expr) {
> >+  assert(isl_ast_expr_get_type(Expr) == isl_ast_expr_op &&
> >+         "isl ast expression not of type isl_ast_op");
> >+  assert(isl_ast_expr_get_op_type(Expr) == isl_ast_op_access &&
> >+         "not an access isl ast expression");
> >+  assert(isl_ast_expr_get_op_n_arg(Expr) >= 2 &&
> >+         "We need at least two operands in an n-ary operation");
> >+
> >+  Value *Base = create(isl_ast_expr_get_op_arg(Expr, 0));
> >+
> >+  SmallVector<Value *, 4> IndexArray;
> >+  for (int i = 1; i < isl_ast_expr_get_op_n_arg(Expr); ++i) {
> >+    Value *OpV = create(isl_ast_expr_get_op_arg(Expr, i));
> >+    OpV = Builder.CreateSExt(OpV, getType(Expr));
> >+    IndexArray.push_back(OpV);
> >+  }
> 
> I would prefer to just assert in case we have more than one array
> dimensions and to remove this loop. As you state above, we most likely
> don't want to push those values into IndexArray, but instead compute the
> linearized access ourselves. Instead of committing code that is
> currently unused and likely to change later, I prefer to state the
> limitation and only commit the code later.
I have to go back on what I said earlier, we need the IndexArray (we already
have a unit test for that case). The test basically looks like this:

int A[1024]

void f()
  for i
     A[i]

Now the gep needs two indices [0, i].
I catch this case by checking the base address type, but I'm not yet a hundred
percent sure that this is the right/best solution.

> >+  // TODO: In case of multiple dimensions we need to compute the offset ourselfs
> ourselves
> 
> >+  //       since the Base is likely a simple pointer. To do so we need access to
> >+  //       the memory access associated with the "original base" as it stores
> >+  //       the delinearization information.
> 
> I do not understand the second sentence.
> 
> [I would just leave out the todo or shorten it to 'Support for
> multi-dimensional arrays]
Done.

> After addressing these comments, this patch is good to go.
I made one additional change, I turned this kind of access generation on by
default. Is that OK with you or not? (The idea is to simplify/remove the
BlockGenerator step by step).

Best regards,
  Johannes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Refactor-Update-the-MemoryAccess.patch
Type: text/x-diff
Size: 9120 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140728/4582a5bb/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Expose-the-IslExprBuilder-and-fix-the-modifiable-acc.patch
Type: text/x-diff
Size: 81480 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140728/4582a5bb/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 213 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140728/4582a5bb/attachment.sig>