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

[praveen velliengiri via Phabricator via llvm-commits]

pvellien added a comment.

fixed a typo in comments.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp:193
+    Gen(!GPU.contains("generic") ? SOUTHERN_ISLANDS :
+        (TT.getOS() == Triple::AMDHSA ? SEA_ISLANDS : SOUTHERN_ISLANDS)),
+
----------------
pvellien wrote:
> t-tye wrote:
> > This seems a very strange way for carrying this knowledge to the later code. Why not use a separate variable with a clear name?
> In the previous version for this piece of code, the Gen member of this class is initialized to sea_islands when OS is HSA and southern_islands for all other cases and then the requested 'gen' is picked  by the tablegen generated function "ParseSubtargetFeatures" in initializeSubtargetDependencies. 
> 
> The above mentioned behavior is not correct, because it initializes the Gen is sea_islands for "-mtriple=amdgcn-amd-amdhsa -mcpu=gfx600" where the correct "Gen" would be southern islands. 
> 
> This piece of change fixes this bug, and also preserves the existing functionality. I don't think we need a separate variable since this is the only place we are handling. please correct me if I'm wrong. 
Sorry there is a typo, in the second passage it should be:

" The above mentioned behavior is not correct, because it initializes the Gen to 
 sea_islands for "-mtriple=amdgcn-amd-amdhsa -mcpu=gfx600" when the correct 
 'Gen' would be southern islands." 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92483