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