yaxunl marked 6 inline comments as done.
yaxunl added inline comments.
================
Comment at: lib/CodeGen/CGCall.cpp:3851
+ ->getType()
+ ->getPointerAddressSpace();
const unsigned ArgAddrSpace =
----------------
rjmccall wrote:
> yaxunl wrote:
> > rjmccall wrote:
> > > Hmm. Is there actually a test case where
> > > Addr.getType()->getAddressSpace() is not the lowering of LangAS::Default?
> > > The correctness of the performAddrSpaceCast call you make depends on
> > > that.
> > >
> > > If there isn't, I think it would be better to add a target hook to
> > > attempt to peephole an addrspace cast:
> > > llvm::Value *tryPeepholeAddrSpaceCast(llvm::Value*, unsigned valueAS,
> > > unsigned targetAS);
> > >
> > > And then you can just do
> > > bool HasAddrSpaceMismatch = CGM.getASTAllocaAddrSpace() !=
> > > LangAS::Default);
> > > if (HasAddrSpaceMismatch) {
> > > if (llvm::Value *ptr = tryPeepholeAddrSpaceCast(Addr.getPointer(),
> > > LangAS::Default, getASTAllocAddrSpace())) {
> > > Addr = Address(ptr, Addr.getAlignment()); // might need to cast the
> > > element type for this
> > > HasAddrSpaceMismatch = false;
> > > }
> > > }
> > It is possible RVAddrSpace is not default addr space. e.g. in OpenCL
> >
> > ```
> > global struct S g_s;
> >
> > void f(struct S s);
> >
> > void g() {
> > f(g_s);
> > }
> >
> > ```
> >
> > One thing that confuses me is that why creating temp and copying can be
> > skipped if RVAddrSpace equals alloca addr space. The callee is supposed to
> > work on a copy of the arg, not the arg itself, right? Shouldn't the arg
> > always be coped to a temp then pass to the callee?
> The result of emitting an agg expression should already be a temporary. All
> sorts of things would be broken if it we still had a direct reference to g_s
> when we got here.
For the above example, the AST for the call statement `f(g_s)` is
```
`-CallExpr 0x60a9fc0 <line:10:3, col:8> 'void'
|-ImplicitCastExpr 0x60a9fa8 <col:3> 'void (*)(struct S)'
<FunctionToPointerDecay>
| `-DeclRefExpr 0x60a9f00 <col:3> 'void (struct S)' Function 0x60a9d80
'f' 'void (struct S)'
`-ImplicitCastExpr 0x60a9ff0 <col:5> 'struct S':'struct S'
<LValueToRValue>
`-DeclRefExpr 0x60a9f28 <col:5> '__global struct S':'__global struct S'
lvalue Var 0x607efb0 'g_s' '__global struct S':'__global struct S'
```
When clang executes line 3846 of CGCall.cpp, RV contains
```
@g_s = external addrspace(1) global %struct.S, align 4
```
Therefore the temporary var has not been created yet. It seems the temporary
var will be created by line 3864.
So at line 3848, RVAddrSpace has non-default address space 1.
https://reviews.llvm.org/D34367
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits