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
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to