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

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

> The relationships between different address spaces is only indirectly related to adding an address space to alloca, and is more a useful side effect. I think LLVM's assumptions about pointers and address space 0 (and worse so, clang) are more widespread than you think. There is no concept of a code pointer size.

Well, address space is carried by PointerType, so it isn't true that LLVM IR has no concept of code pointer sizes.  But I hadn't realized that Function doesn't have a way to actually set an address space on creation; I can see how that would make that change more involved.  On the other hand, it seems like something that could easily just be added as a default argument to a couple of APIs; I don't see why *that's* seen as an invasive change but adding a new mandatory argument to the interfaces for creating one of the most common instruction classes is not.

> This is one area where address space 0 is treated as a lack of an address space. The code size and stack pointer do not have the same size for AMDGPU, and it is important to be able to distinguish these.

Yes, I did understand that the first time it was explained.

> There is a lot of code that assumes the default address space size is "the one true pointer size".

Are you suggesting that LLVM's address space 0 must always be at least as large as every other address space?  Where is that assumption made?  Again, that seems like something that should be fixed independent of what we decide here.

> You can't pass around a 64-bit function pointer properly if it's required to have the same properties as a 32-bit stack pointer. It isn't really OK to start treating function pointer types differently than other types.

Of course.  I am not suggesting that we should do that.

> I don't think adding this DataLayout argument is very invasive, and was less work than I anticipated

That's because you aren't changing any of the frontends, most of which are not in-tree.  I understand that LLVM does not promise to keep the C++ API stable, but there's surely *some* responsibility to not break every frontend for features they don't care about.  At the very least, yes, you should be picking up this value from the Module instead of requiring it to be passed in.

> The pressing reason for this is C++ GPU support. Clang's handling of address spaces is more problematic. The assumptions about address space 0 are more widespread and tied to LLVM's address space 0.

Clang's assumptions about address space 0 are pervasive because it represents an actual concept in the source language: C has a generic address space.  My point is that LLVM generally should not.

I don't think the assumptions in Clang that the Clang AST address space 0 equals LLVM's address space 0 are nearly as pervasive as you think.  At any rate, since you require stack allocations to be in a different LLVM address space from the generic address space, but (per C) taking the address of a local variable yields a pointer in the generic address space, you're going to have to teach Clang about these differences between address spaces anyway, because it'll have to insert address-space conversions in a number of places.


https://reviews.llvm.org/D31042