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

[Tobias Grosser]

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

>     }
>
>     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.

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 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)

Cheers,
Tobias