yaxunl marked 5 inline comments as done.
yaxunl added inline comments.
================
Comment at: lib/CodeGen/CGCall.cpp:3832
+ "indirect-arg-temp");
+ IRCallArgs[FirstIRArg] = CastToAllocaAddrSpace(Addr.getPointer());
----------------
rjmccall wrote:
> Isn't the original code here correct? You're basically just adding
> unnecessary casts.
will remove.
================
Comment at: lib/CodeGen/CGCall.cpp:3851
+ ->getType()
+ ->getPointerAddressSpace();
const unsigned ArgAddrSpace =
----------------
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?
================
Comment at: lib/CodeGen/CGCall.cpp:3865
+ "byval-temp");
+ IRCallArgs[FirstIRArg] = CastToAllocaAddrSpace(AI.getPointer());
EmitAggregateCopy(AI, Addr, I->Ty, RV.isVolatileQualified());
----------------
rjmccall wrote:
> Same thing, no reason to do the casts here.
will remove
================
Comment at: lib/CodeGen/CGDecl.cpp:1828
+ auto DestAS = getContext().getTargetAddressSpace(LangAS::Default);
+ if (SrcAS != DestAS) {
+ assert(SrcAS == CGM.getDataLayout().getAllocaAddrSpace());
----------------
rjmccall wrote:
> This should be comparing AST address spaces.
will do.
https://reviews.llvm.org/D34367
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
