[PATCH] D80545: [AMDGPU/MemOpsCluster] Let mem ops clustering logic also consider number of clustered bytes

[Jay Foad via Phabricator via llvm-commits]

foad accepted this revision.
foad added a comment.
This revision is now accepted and ready to land.

Looks OK to me with some minor cleanups (see inline comments).



================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:267
 
-bool SIInstrInfo::getMemOperandsWithOffset(
+unsigned SIInstrInfo::getOperandSizeInBytes(const MachineInstr &LdSt,
+                                            const MachineOperand *MOp) const {
----------------
Do you need this function at all? Can't you use RI.getOpSize instead?


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:272-274
+  const TargetRegisterClass *DstRC = Register::isVirtualRegister(Reg)
+                                         ? MRI.getRegClass(Reg)
+                                         : RI.getPhysRegClass(Reg);
----------------
Could use RI.getRegClassForReg here.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:275
+                                         : RI.getPhysRegClass(Reg);
+  return (RI.getRegSizeInBits(*DstRC) / 8);
+}
----------------
You don't need the outer parentheses here.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:377
       return true;
     }
 
----------------
You can remove some duplicated code by making this an "else", so there is only one "return true" for all BUF instructions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80545