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

Tue Dec 8 12:16:15 PST 2020

rampitec added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp:192
     TargetTriple(TT),
-    Gen(TT.getOS() == Triple::AMDHSA ? SEA_ISLANDS : SOUTHERN_ISLANDS),
+    Gen(!GPU.contains("generic") ? SOUTHERN_ISLANDS :
+        (TT.getOS() == Triple::AMDHSA ? SEA_ISLANDS : SOUTHERN_ISLANDS)),
----------------
pvellien wrote:
> rampitec wrote:
> > pvellien wrote:
> > > t-tye wrote:
> > > > Is this asking if the code contains use of the generic address space?
> > > No, this isn't a check for generic address space. The GPU string is set to a string "generic" by default when no target cpu is specified. It has nothing to do with address spaces access.
> > > when OS is HSA it is set to "generic-hsa" and in all other cases it is set to "generic".
> > I would probably just error out if we have a generic CPU + amdhsa requested. It seems cleaner.
> Thanks for your comment, this patch preserves the earlier functionality except the use of mubuf. Perhaps we can address your suggestion in a separate patch with some added context, and exploring the possibilities of reporting the error in other target-triple, cpu pairs? 
It is OK to have it in a separate commit, but I do not follow logic here. You are setting SI for any NON-generic chip.

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

https://reviews.llvm.org/D92483