[PATCH] D70947: Add a default address space for globals to DataLayout

[Eric Christopher via llvm-commits]

On Fri, Nov 20, 2020 at 3:49 PM Eric Christopher <echristo at gmail.com> wrote:

>
>
> On Fri, Nov 20, 2020 at 3:44 PM Alexander Richardson via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> arichardson added a comment.
>>
>> In D70947#2408741 <https://reviews.llvm.org/D70947#2408741>, @echristo
>> wrote:
>>
>> > Why is this useful rather than having targets set the address space
>> used in the backend explicitly? i.e. how is what amdgpu was doing up to
>> this point not ok? It's not really clear here.
>>
>> That's fine for globals created in target-specific passes, but other
>> passes that create new global variables need to know which address space to
>> place them in. In our CHERI fork (using a default address space of 200), we
>> got lots of assertion failures/invalid IR due to passes creating globals in
>> AS0 (or assuming that all global pointers are AS0) until I added a default
>> globals address space to the datalayout so  those passes can choose the
>> address space that our target needs. Does that make sense?
>>
>
> It does. It highlights a problem that tools aren't forward compatible
> though :) I think it's fine, it's just a little annoying.
>
> Arguably this is a major enough change that it should have gone to
> llvm-dev as well. Would you mind sending that note letting people know what
> you've done? You may get rollback requests, but changing the data layout
> string is a major change.
>
>
One more question: I'm assuming this is a default different from the rest
of the address space information for a reason? The docs around it could
probably use explaining (multiple split address spaces etc). FWIW if there
wasn't an in-tree use for AMD I'd probably not want this change in.

+Matt Arsenault <Matthew.Arsenault at amd.com> This is necessary for AMD yes?
Not just updating AMD for a change that would default to 0?

-eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201120/88954004/attachment.html>