[PATCH] D50982: [AMDGPU] Legalize VGPR Rsrc operands for MUBUF instructions

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In https://reviews.llvm.org/D50982#1207797, @scott.linder wrote:

> I will add some IR tests for at least those cases.
>
> I don't understand what you mean about the const-32bit addrspace; can you explain more?


I think part of the shader assume uniform hack is we claim that loads from the 32-bit const address space are uniform, such that it makes it "ok" to use the single readlane. This was always a broken concept though. See isSDNodeAlwaysUniform handling of load, and the documentation



================
Comment at: lib/Target/AMDGPU/SIInstrInfo.cpp:3481-3486
+static void emitLoadSRsrcFromVGPRLoop(const SIInstrInfo &TII,
+                                      MachineRegisterInfo &MRI,
+                                      MachineBasicBlock &OrigBB,
+                                      MachineBasicBlock &LoopBB,
+                                      const DebugLoc &DL,
+                                      MachineOperand &Rsrc) {
----------------
scott.linder wrote:
> arsenm wrote:
> > I was hoping for this to a be a more general function that could be re-used for other operations in the future, and probably sharable with the vector indexing code.
> > 
> > I was thinking about something like a list of operands that need to be SGPRs, and iterating over all of the subregisters rather than assuming the SGPR128 +  sometimes SGPR32 the buffer operations use
> I can work on this; would you prefer I do that in this patch?
That can be a second patch


================
Comment at: lib/Target/AMDGPU/SIInstrInfo.cpp:3490
+  unsigned VRsrc = Rsrc.getReg();
+  unsigned VRsrcUndef = getUndefRegState(Rsrc.isUndef());
+
----------------
scott.linder wrote:
> arsenm wrote:
> > This is a bool
> I thought `isUndef` was a bool and `getUndefRegState` returned flags.
Yes, you're right


Repository:
  rL LLVM

https://reviews.llvm.org/D50982