[PATCH] D34367: CodeGen: Fix address space of indirect function argument

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: lib/CodeGen/CGCall.cpp:3851
+                                         ->getType()
+                                         ->getPointerAddressSpace();
         const unsigned ArgAddrSpace =
----------------
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.


https://reviews.llvm.org/D34367