[PATCH] D31042: Allow DataLayout to specify addrspace for allocas.

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D31042#711611, @arsenm wrote:

> In https://reviews.llvm.org/D31042#711596, @rjmccall wrote:
>
> > What else in LLVM actually hard-codes 0 as the assumed address space?  Intrinsic function declarations?  Optimizations that create new globals (if there are any?)  Wouldn't it be much less invasive to change those and leave the assumption that the stack is in addrspace 0?
>
>
> I'm not sure what you are suggesting. The concept of assumed address space doesn't make any sense on AMDGPU. Everything has an address space. This isn't changing how address space 0 is treated, and is specifically avoiding doing so. AMDGPU does want the assumed address space 0 properties, just for some of the other address spaces that are not the stack address space, which is one of the advantages of making this change. The problem isn't really places hard coding 0 as the address space, it's the properties of the stack address space do not behave like LLVM's notion of the generic address space as I mentioned in the RFC.
>
> As far as 0 being a valid stack pointer, we currently have a workaround of not allocating user objects there. The bigger issues in my mind is that the pointer size for the stack does not match the code pointer size, which is a problematic area which keeps popping up. Another is it is not valid to addrspacecast any other address space to the stack address space. One partial alternative would be to introduce a code pointer size to the datalayout or code address space or something along those lines. I think that would still leave a conceptual gap between AMDGPU's stack and alloca, and be more of a hassle. (For example you still cannot addrspacecast any pointer to the address space of the stack).
>
> There are some intrinsics which hard code 0, some of which need to be fixed (such as lifetime intrinsics which https://reviews.llvm.org/D31041 and https://reviews.llvm.org/D31043 take care of) but other than that one that's mostly a different question. There is a jump table optimization which creates new globals (which is disabled if BR_JT is illegal). Moving AMDGPU's flat address space to match LLVM's generic address space would also have the advantage of allowing legal addrspacecasts to address space 0 for some of these intrinsics.


You keep talking about LLVM having a "generic address space".  I'm trying to figure out what you mean by that, because in LLVM, global values can be declared in an arbitrary address space, pointer values all carry an address space, instructions work on arbitrary address spaces, the memory intrinsics are generally overloaded by pointer address spaces, etc.  And in general, the idea that there exists a generic address space which subsumes every other address space is just wrong.  As an example, on x86-64 we use address spaces as a way of requesting segmented addressing; even if the memory there was addressable in the standard segment (which is not guaranteed), AFAIK there's no way to actually perform that conversion.

Given that, there's nothing wrong with LLVM deciding to hard-code a specific address space index as the address space of the stack.  That's worth doing if it significantly simplifies the IR, e.g. by not requiring a data layout to be passed to CreateAlloca.  It just means that frontends that *do* assume there's a generic address space may need to insert addrspace conversions in some cases.

it's okay to call out some specific address space, like addrspace 0, and say that it's always the address space of the stack.

I certainly hope that nothing in LLVM is ever implicitly inserting addrspace conversions or assuming anything about them without target-specific information.  In general, it is not true that there exists an address space that subsumes all others.


https://reviews.llvm.org/D31042