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

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D79744#2030535 <https://reviews.llvm.org/D79744#2030535>, @rjmccall wrote:

> In D79744#2030481 <https://reviews.llvm.org/D79744#2030481>, @arsenm wrote:
>
> > In D79744#2030294 <https://reviews.llvm.org/D79744#2030294>, @rjmccall wrote:
> >
> > > The parameter variable is formally considered to be in a particular address space, and taking the address of it yields a pointer in that address space.  That can only work for an indirect argument if either (1) the address space of the pointer that's actually passed can be freely converted to that formal address space (because it's a subspace, or because it's a superspace but known to be in the right subspace) or (2) we're willing to copy the object into the right address space on the callee side (and able to — obviously this only works for POD types).  I do see the merit of allowing an address space to be specified for targets that consider locals to be in a different formal address space than the stack would naturally be (e.g. a generic address space); I don't remember if that applies to AMDGPU.
> >
> >
> > Kernel arguments aren't directly visible to the user program, so this is an implementation detail. The user variable is the alloca that we need to explicitly copy to as you mentioned, which this patch does.
>
>
> It's usually a goal of indirect arguments to not need this copy.  We usually bind parameters directly to the pointer that was passed in.
>
> > It's possible to poke at these through intrinsics, but the kernel address space isn't part of the language. POD type or not, we're going to have to do something to unload values from the constant buffer onto the stack.
> > 
> >> `byval` in LLVM is not a generic "this is a by-value argument" annotation; it specifically means that the value is actually passed on the stack despite the formally indirect convention, and therefore there's an implicit memcpy on the caller side.  That is why `byval` is inherently tied to the `alloca` address space: there's no actual pointer being passed, so it doesn't make sense to pretend it might have been promoted into a different address space.  That is also why there is no restriction to writing into the pointer: there's no reason to prevent writing to the argument slot.
> > 
> > In this case there is never a call. Only the callee read exists as this is the entry point. What would the alternative be? Add another flavor of byval attribute that's nearly identical?
>
> It's hard to answer this because I'm not sure what you're trying to accomplish by marking the arguments `byval`.


The short answer is I'm trying to avoid fighting the optimizer's handling of aggregate load and stores. Passes already understand byval, but are actively harmful to aggregate loads and stores. Having explicit loads in the IR also brings it closer to what these are ultimately lowered to. Currently we emit a raw struct argument, which produces an aggregate store to alloca. Both SROA and instcombine unhelpfully split up the aggregate store, which doesn't optimize nicely like the memcpy from constant memory to alloca. The end result is we end up with more allocas and copies from constant memory that could have just read directly from the constant pointer.

The most honest calling convention handling would be to have an explicit constant pointer that all arguments are read from, and the function would have an empty argument list. This is somewhat impractical, since we still need to track the argument sizes and offsets somehow in the IR. It also becomes much more difficult to emit nicely annotated IR, since things like noalias still largely expect to be attached to a function argument. We do have an optimization pass that rewrites the arguments to look like this to enable vectorization later in the backend, but it suffers from losing useful alias annotations.


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

https://reviews.llvm.org/D79744