[PATCH] D47194: [AMDGPU] Fixed non-uniform addr64 MUBUF in shader

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/SIInstrInfo.cpp:3667
   if (isMIMG(MI) ||
       (AMDGPU::isShader(MF.getFunction().getCallingConv()) &&
+       (isMUBUF(MI) || isMTBUF(MI))
----------------
tpr wrote:
> arsenm wrote:
> > I don't like that this was checking the calling convention at all. Can we just eliminate that check altogether?
> So (from a compute point of view) you think it is ok for all non-addr64 cases of MUBUF/MTBUF to go in here? I think that is probably ok, but then I'm not sure why Nicolai added the calling convention check in the first place.
The main concerning case I'm aware of with anything touching these is for anything using the add-tid bit (and maybe strided access). In any case, the calling convention isn't actually the property that matters


================
Comment at: lib/Target/AMDGPU/SIInstrInfo.cpp:3669
+       (isMUBUF(MI) || isMTBUF(MI))
+       && !isAddr64(MI))) {
     MachineOperand *SRsrc = getNamedOperand(MI, AMDGPU::OpName::srsrc);
----------------
tpr wrote:
> arsenm wrote:
> > You should be able to avoid adding isAddr64 by just moving the getNamedOperand call for vaddr below up here
> I tried that, and I got false positives on offen/idxen/bothen variants.
Oh right, the operand name is still the same for either


Repository:
  rL LLVM

https://reviews.llvm.org/D47194