[PATCH] D92483: AMDGPU - Use MUBUF instructions for global address space access

[praveen velliengiri via Phabricator via llvm-commits]

pvellien added a comment.

In D92483#2458755 <https://reviews.llvm.org/D92483#2458755>, @rampitec wrote:

> In D92483#2458659 <https://reviews.llvm.org/D92483#2458659>, @rampitec wrote:
>
>> In D92483#2458152 <https://reviews.llvm.org/D92483#2458152>, @pvellien wrote:
>>
>>> if mcpu is specified, set Gen to the correct generation that cpu belongs to, by initializing it with SOUTHERN_ISLANDS.
>>> if mcpu is not specified (in this case GPU = "generic-hsa"), set Gen to the SEA_LANDS to be in parity with previous code> Comments inline
>>
>> You are forcing SI for any SPECIFIED GPU instead:
>>
>>   !GPU.contains("generic") ? SOUTHERN_ISLANDS 
>>
>> Read this code line: if CPU is NOT generic, force it to SI.
>
> Ugh. I see what you mean now, it took debugger to understand. The real value of Gen is assigned below in the GCNSubtarget::initializeSubtargetDependencies().
>
> 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.



In D92483#2458755 <https://reviews.llvm.org/D92483#2458755>, @rampitec wrote:

> In D92483#2458659 <https://reviews.llvm.org/D92483#2458659>, @rampitec wrote:
>
>> In D92483#2458152 <https://reviews.llvm.org/D92483#2458152>, @pvellien wrote:
>>
>>> if mcpu is specified, set Gen to the correct generation that cpu belongs to, by initializing it with SOUTHERN_ISLANDS.
>>> if mcpu is not specified (in this case GPU = "generic-hsa"), set Gen to the SEA_LANDS to be in parity with previous code> Comments inline
>>
>> You are forcing SI for any SPECIFIED GPU instead:
>>
>>   !GPU.contains("generic") ? SOUTHERN_ISLANDS 
>>
>> Read this code line: if CPU is NOT generic, force it to SI.
>
> Ugh. I see what you mean now, it took debugger to understand. The real value of Gen is assigned below in the GCNSubtarget::initializeSubtargetDependencies().
>
> 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



In D92483#2458755 <https://reviews.llvm.org/D92483#2458755>, @rampitec wrote:

> In D92483#2458659 <https://reviews.llvm.org/D92483#2458659>, @rampitec wrote:
>
>> In D92483#2458152 <https://reviews.llvm.org/D92483#2458152>, @pvellien wrote:
>>
>>> if mcpu is specified, set Gen to the correct generation that cpu belongs to, by initializing it with SOUTHERN_ISLANDS.
>>> if mcpu is not specified (in this case GPU = "generic-hsa"), set Gen to the SEA_LANDS to be in parity with previous code> Comments inline
>>
>> You are forcing SI for any SPECIFIED GPU instead:
>>
>>   !GPU.contains("generic") ? SOUTHERN_ISLANDS 
>>
>> Read this code line: if CPU is NOT generic, force it to SI.
>
> Ugh. I see what you mean now, it took debugger to understand. The real value of Gen is assigned below in the GCNSubtarget::initializeSubtargetDependencies().
>
> 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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92483/new/

https://reviews.llvm.org/D92483