[PATCH] D92483: AMDGPU - Use MUBUF instructions for global address space access
Tony Tye via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 21 14:35:03 PST 2020
t-tye added a comment.
In D92483#2466180 <https://reviews.llvm.org/D92483#2466180>, @pvellien wrote:
> In D92483#2466144 <https://reviews.llvm.org/D92483#2466144>, @scott.linder wrote:
>
>> In D92483#2463773 <https://reviews.llvm.org/D92483#2463773>, @rampitec wrote:
>>
>>> In D92483#2463314 <https://reviews.llvm.org/D92483#2463314>, @pvellien wrote:
>>>
>>>> In D92483#2458755 <https://reviews.llvm.org/D92483#2458755>, @rampitec wrote:
>>>>
>>>>> But I still do not think this is a right thing to do to accept amdhsa on SI, even if you turn off flat instructions. SI does not support HSA.
>>>>
>>>> As far as I the support of HSA in SI processors. Tony suggested that gfx60x processors should support AMDHSA OS because it is capable of supporting OpenCL 1.2 which not need generic pointers. This topic was discussed in amdgcn weekly and using MUBUF instructions for global address space in gfx60x is decided. The same is also documented in AMDGPU User Guide earlier.
>>>
>>> But it is not. In fact AMDGPUUsage.rst says that SI does NOT support amdhsa and it is only supported starting from gfx700:
>>>
>>> **GCN GFX6 (Southern Islands (SI))** [AMD-GCN-GFX6]_
>>> -----------------------------------------------------------------------------------------------------------------------
>>> ``gfx600`` - ``tahiti`` ``amdgcn`` dGPU - Does not - *pal-amdpal*
>>> support
>>> generic
>>> address
>>> space
>>> ``gfx601`` - ``pitcairn`` ``amdgcn`` dGPU - Does not - *pal-amdpal*
>>> - ``verde`` support
>>> generic
>>> address
>>> space
>>> ``gfx602`` - ``hainan`` ``amdgcn`` dGPU - Does not - *pal-amdpal*
>>> - ``oland`` support
>>> generic
>>> address
>>> space
>>>
>>> It is only amdpal listed here.
>>>
>>> What was discussed is that we can use buffer instructions for global on SI even if amdhsa is not supported.
>>
>> At one point, while you were on vacation, the consensus was "support amdhsa OS with gfx6, and fail to compile in the presence of e.g. generic pointers".
>>
>> Has there been more discussion since then?
>
> Yeah, support amdhsa OS
>
> In D92483#2466151 <https://reviews.llvm.org/D92483#2466151>, @rampitec wrote:
>
>> In D92483#2466144 <https://reviews.llvm.org/D92483#2466144>, @scott.linder wrote:
>>
>>> In D92483#2463773 <https://reviews.llvm.org/D92483#2463773>, @rampitec wrote:
>>>
>>>> In D92483#2463314 <https://reviews.llvm.org/D92483#2463314>, @pvellien wrote:
>>>>
>>>>> In D92483#2458755 <https://reviews.llvm.org/D92483#2458755>, @rampitec wrote:
>>>>>
>>>>>> But I still do not think this is a right thing to do to accept amdhsa on SI, even if you turn off flat instructions. SI does not support HSA.
>>>>>
>>>>> As far as I the support of HSA in SI processors. Tony suggested that gfx60x processors should support AMDHSA OS because it is capable of supporting OpenCL 1.2 which not need generic pointers. This topic was discussed in amdgcn weekly and using MUBUF instructions for global address space in gfx60x is decided. The same is also documented in AMDGPU User Guide earlier.
>>>>
>>>> But it is not. In fact AMDGPUUsage.rst says that SI does NOT support amdhsa and it is only supported starting from gfx700:
>>>>
>>>> **GCN GFX6 (Southern Islands (SI))** [AMD-GCN-GFX6]_
>>>> -----------------------------------------------------------------------------------------------------------------------
>>>> ``gfx600`` - ``tahiti`` ``amdgcn`` dGPU - Does not - *pal-amdpal*
>>>> support
>>>> generic
>>>> address
>>>> space
>>>> ``gfx601`` - ``pitcairn`` ``amdgcn`` dGPU - Does not - *pal-amdpal*
>>>> - ``verde`` support
>>>> generic
>>>> address
>>>> space
>>>> ``gfx602`` - ``hainan`` ``amdgcn`` dGPU - Does not - *pal-amdpal*
>>>> - ``oland`` support
>>>> generic
>>>> address
>>>> space
>>>>
>>>> It is only amdpal listed here.
>>>>
>>>> What was discussed is that we can use buffer instructions for global on SI even if amdhsa is not supported.
>>>
>>> At one point, while you were on vacation, the consensus was "support amdhsa OS with gfx6, and fail to compile in the presence of e.g. generic pointers".
>>>
>>> Has there been more discussion since then?
>>
>> I see that is not what we document. @t-tye whatis your opinion?
>
> @scott.linder there is no discussion since then.
> @rampitec the final consensus is to use buffer instructions for global address space accesses in SI in presence of amdhsa. In other OS types for SI, backend generates buffer instructions for global addr space access already.
I updated AMDGPUUsage to reflect the current state which is that gfx6* does not support amdhsa. But if this patch allows gfx6* to be supported without generic addresses then that table should also be updated as part of the patch.
We did discuss this further and decided that gfx6* should report it supports amdhsa with the restriction that it does not support generic addresses. AMDGPUUsage already notes the restriction in all relevant sections. The LIT tests would also need updating accordingly. The COMgr tests would also need updating.
The alternative would be that the compiler reports an error message for gfx6* if the triple specified amdhsa (rather than crash as it does currently). But it was felt that the amdhsa ABI can allow targets that do not support generic to be included provided it is documented. There is no need to artificially restrict them as a user may want to try using the target on an HSA complaint runtime to implement OpenCL 1.2.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92483/new/
https://reviews.llvm.org/D92483
More information about the llvm-commits
mailing list