[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D79744#2069324 <https://reviews.llvm.org/D79744#2069324>, @arsenm wrote:

> In D79744#2050498 <https://reviews.llvm.org/D79744#2050498>, @rjmccall wrote:
>
> > > For the purpose here, only the callee exists. This is essentially a freestanding function, the entry point to the program.
> >
> > I'm definitely not going to let you add a new "generic" argument-passing convention and then only implement exactly the parts you need; that's just adding technical debt.
>
>
> I'm not sure what you mean here. I don't really want or need a totally new generic argument passing convention. Not every IR feature is expected to be implemented by every backend. For instance, inalloca/preallocated exist solely to implement the x86 windows ABI and no other target tries to handle them. This is just another flavor of the same fundamental concept of a parameter attribute needed for a target specific calling convention.


I mean the Clang code for supporting this new convention, not the IR support.  Of course LLVM has target-specific attributes.

>>> The load-from-constant nature needs to be exposed earlier, so I think this necessarily involves changing the convention lowering in some way and it's just a question of what it looks like. To summarize the 2.5 options I've come up with are
>>> 
>>> 1. Use constant byval parameters, as this patch does. This requires the least implementation effort but doesn't exactly fit in with byval as defined.
>> 
>> I assume you generate a `byval` caller in some later pass, which will then implicitly copy.  Or do you lower `byval` in a nonstandard way in your backend?
> 
> No, the caller is only an external driver of some kind. Since the address spaces don't match (and the source address space is read only), anything that behaves like a stack byval doesn't really make sense from the beginning which is why this patch changes the address space. If we were to leave this as a stack address space, we would have to add an alloca and insert a memcpy anyway. This would still leave an unusable byval argument around a pass could still theoretically try to write into.
> 
> D79630 <https://reviews.llvm.org/D79630> adds the lowering that uses this. Because reasons (some of which I referenced in the previous comments), we have 3 different implementations of the ABI. The byval pointer value is simply replaced with a new pointer derived from a constant offset from an intrinsic call (this is most obvious in the AMDGPULowerKernelArguments IR version)
> 
>>> 1. Replace all IR argument uses with loads from a constant offset from an intrinsic call. This still needs to leave the IR arguments in place because we do need to know the original argument sizes and offsets, but they would never have a use (or I would need to invent some other method of tracking this information)
>> 
>> I guess passing aggregate arguments using a normal indirect convention has this same tracking problem.  You'd also have to copy on the caller side to maintain semantics, which is probably hard to eliminate, but I would guess `byval` has the same problem?
> 
> Isn't using byval the normal indirect convention? Really the only properties I want that byval gives me is a pointee size and alignment. The most abstract attribute I could probably come up with is a pointee(type) annotation that I could use, without the stack copying implications of byval

`byval` is not an indirect convention.  It looks like one in IR, but it means "it's passed in the argument area of the stack", which is essentially more like being passed directly than otherwise.

In IR, the normal indirect convention is just to pass a pointer without any extra treatment.  Clang does set `nonnull dereferenceable(size) align 4` for optimization purposes, though.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79744/new/

https://reviews.llvm.org/D79744