[Polly][PATCH 5/8] Allow the IslExprBuilder to build address of expressions

[Johannes Doerfert]

On 08/11, Tobias Grosser wrote:
> On 10/08/2014 09:50, Johannes Doerfert wrote:
> >---
> >  include/polly/CodeGen/IslExprBuilder.h |  1 +
> >  lib/CodeGen/IslExprBuilder.cpp         | 26 ++++++++++++++++++++++++++
> >  2 files changed, 27 insertions(+)
> >
> >diff --git a/include/polly/CodeGen/IslExprBuilder.h b/include/polly/CodeGen/IslExprBuilder.h
> >index c87cd57..39e7044 100644
> >--- a/include/polly/CodeGen/IslExprBuilder.h
> >+++ b/include/polly/CodeGen/IslExprBuilder.h
> >@@ -128,6 +128,7 @@ private:
> >    llvm::Value *createOpBoolean(__isl_take isl_ast_expr *Expr);
> >    llvm::Value *createId(__isl_take isl_ast_expr *Expr);
> >    llvm::Value *createInt(__isl_take isl_ast_expr *Expr);
> >+  llvm::Value *createOpAddrOf(__isl_take isl_ast_expr *Expr);
> >  };
> >  }
> >
> >diff --git a/lib/CodeGen/IslExprBuilder.cpp b/lib/CodeGen/IslExprBuilder.cpp
> >index b9bdabe..5eeab39 100644
> >--- a/lib/CodeGen/IslExprBuilder.cpp
> >+++ b/lib/CodeGen/IslExprBuilder.cpp
> >@@ -389,11 +389,37 @@ Value *IslExprBuilder::createOp(__isl_take isl_ast_expr *Expr) {
> >    case isl_ast_op_ge:
> >    case isl_ast_op_gt:
> >      return createOpICmp(Expr);
> >+  case isl_ast_op_addr_of:
> >+    return createOpAddrOf(Expr);
> 
> I would prefer AddressOf
Sure.

> >    }
> >
> >    llvm_unreachable("Unsupported isl_ast_expr_op kind.");
> >  }
> >
> >+Value *IslExprBuilder::createOpAddrOf(__isl_take isl_ast_expr *Expr) {
> >+  assert(isl_ast_expr_get_type(Expr) == isl_ast_expr_op &&
> >+         "Expected an isl_ast_expr_op expression.");
> >+  assert(isl_ast_expr_get_op_n_arg(Expr) == 1 && "Address of should be unary.");
> >+
> >+  isl_ast_expr *Op = isl_ast_expr_get_op_arg(Expr, 0);
> >+  assert(isl_ast_expr_get_type(Op) == isl_ast_expr_op &&
> >+         "Expected address of operator to be an isl_ast_expr_op expression.");
> >+  assert(isl_ast_expr_get_op_type(Op) == isl_ast_op_access &&
> >+         "Expected address of operator to be an access expression.");
> >+
> >+  Value *V = create(Op);
> >+
> >+  LoadInst *LI = dyn_cast<LoadInst>(V);
> >+  assert(LI && "Address of operand should result in a load.");
> >+
> >+  V = LI->getPointerOperand();
> >+  LI->eraseFromParent();
> 
> This is a little hackish. I would prefer that we factor out the code that
> computes the address of an access and that we just call it from both, this
> location and the location where we generate the load.
This is not as hackish as it looks like. The reason is the restriction on
the new isl_ast_expr address_of operator. So far it is only applicable to access
operations (I can add a comment as the assert seems not to explain it
very well). In the future it might be applicable to more but the "types"
still have to match, thus in our code generation the operand should
result in a load/store (the store case is missing at the moment though).

If you want me to factor it out, sure...

> Also, I am surprised that create(Op) actually returns a load instruction? My
> understanding is that we do not create load/stores for access expressions,
> but that we only create the ptr value?
I change the behaviour of createOpAccess to generate the load and the
reason is pretty simple:
  A isl_ast_expr which dumps like MemRef_sum[0] or MemRef_A[i] should
  not result in a LLVM-Value representing &MemRef_sum[0] or
  &MemRef_A[i]. What we did before for isl ast access expressions was
  actually implementing the address of the access not the actual
  access.
But if you want the ptr value you can just use the new address of
operator.

> (I found it. In the next patch, you actually adjust the semantics of
> createOpAccess. Relying on a change in a future patch does not really make
> this obvious)
I rebased it before I send the patches and I messed up the ordering...

-- 

Johannes Doerfert
Researcher / PhD Student

Compiler Design Lab (Prof. Hack)
Saarland University, Computer Science
Building E1.3, Room 4.26

Tel. +49 (0)681 302-57521 : doerfert at cs.uni-saarland.de
Fax. +49 (0)681 302-3065  : http://www.cdl.uni-saarland.de/people/doerfert
-------------- 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/20140811/d175b515/attachment.sig>