rjmccall added inline comments.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:1997
case ABIArgInfo::Ignore:
+ case ABIArgInfo::IndirectAliased:
return false;
----------------
arsenm wrote:
> arsenm wrote:
> > rjmccall wrote:
> > > arsenm wrote:
> > > > rjmccall wrote:
> > > > > In principle, this can be `inreg` just as much as Indirect can.
> > > > The IR verifier currently will reject byref + inreg
> > > Why? `inreg` is essentially orthogonal.
> > Mostly inherited from the other similar attribute handling. It can be
> > lifted if there's a use
> Plus the name here is isArgInAlloca; this is not necessarily passed in an
> alloca
I agree that we don't need to update this.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:8816
+ // FIXME: Should use byref when promoting pointers in structs, but this
+ // requires adding implementing the coercion.
+ if (!getContext().getLangOpts().OpenCL && LTy == OrigLTy &&
----------------
arsenm wrote:
> rjmccall wrote:
> > I don't see why you'd use `byref` when promoting pointers in structs.
> > Maybe it works as a hack with your backend, but it seems *extremely*
> > special-case and should not be hacked into the general infrastructure.
> The whole point is to reinterpret the address space of the pointers in memory
> since we know if it's a kernel argument it has to be an addrspace(1) pointer
> or garbage. We can't infer the address space of a generic pointer loaded from
> memory.
>
> byref doesn't change that, it just makes the fact that these are passed in
> memory explicit
`byref` is interpreted by your backend passes as an instruction that the
argument value is actually the address of an object that's passed to the kernel
by value, so you need to expand the memory in the kernel argument marshalling.
Why would that be something you'd want to trigger when passing a struct with a
pointer in it? You're not going to recursively copy and pass down the pointee
values of those pointers.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79744/new/
https://reviews.llvm.org/D79744
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
