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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In https://reviews.llvm.org/D31042#711863, @rjmccall wrote:

>




>> 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.

Not necessarily, it's just an occasional sticking point in some remaining that code isn't aware of multiple pointer sizes. One area was in debug info. Patches to fix that were rejected a few times in the past. If you grep for argumentless getPointerSizes in CodeGen+MC, most of them seem to be for debug info. I have another patch I've been holding on to which works around the DAG picking the wrong value type for pointers to function types. Pretty much everything in the DAG for dealing specifically with pointer types is unworkable with different sized pointers, but most of that is possible to avoid using.

> 
> 
>> 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.

So your objection is just to the API change? I can change that if you insist. The actual mechanical adding of the argument isn't the source of the work. As someone who has maintained out of tree uses, this class of change is the least irritating. Most of the clang IR gen work is in updating the downstream users which use hardcoded pointer types, which users not using this don't need to care about it.

> 
> 
>> 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.

Yes, I expect there will be many addrspacecasts inserted from the alloca, which then the InferAddressSpaces pass would hopefully optimize away.


https://reviews.llvm.org/D31042