[llvm-branch-commits] [clang] [llvm] [mlir] [RFC][AMDGPU] Add EXECSYNC address space (PR #195613)

[Pierre van Houtryve via llvm-branch-commits]

Pierre-vh wrote:

> Couple of thoughts on this. If we are going to put barriers into their own named space, we should capitalize on it more than this change does. Splitting the address spaces but continuing to use the same bit encoding of pointers seems completely pointless to me.
> 
> The HW does have a notion of a barrier address space of sorts. The IDs consumed by the various `s_barrier` instructions can be thought of addresses. We should make this new address space match this HW ID space.
> 
> Couple of implications:
> 
> * Don't call it "execsync". That sounds pretty generic, and we may not be able to re-use the address space for future sync uses.
> * Bit representation of the pointers should just use the HW IDs
> * This means that the null pointer value in this address space can be 0

Yup definitely, I just started with something safe to get the discussion going. I didn't want to sink too much time into this, just to discover after that no one liked the idea :smile: - This is why I left the pointer layout untouched for now.
I also assumed we'd like to reuse the AS for future exec sync cases, or even for abstract HW resources (my first draft called the AS "HW_RESOURCE").

If we don't plan to do so and would rather add even more AS later if needed, that is fine (and a better idea than reusing the AS, IMO). @nhaehnle If you can confirm it's the intention I'll update the patch with that in mind:

- Make the pointer layout trivially represent Bar#, and move the shenanigans with the offset within LDS to the addrspacecast ops. Addrspacecast will be a bit more expensive, but I assume it's never going to be on the "hot path".
- Add more helper functions (as Matt suggested) so that if we add another similar AS later, it'll be a bit easier.

https://github.com/llvm/llvm-project/pull/195613