[PATCH] D57399: AMDGPU/GlobalISel: Add support for wide loads >= 256-bits

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:82
 
-}
+class ApplyLoadSplitMapping : public GISelChangeObserver {
+private:
----------------
This is identical to ApplySALUMapping, so maybe just add an argument to the constructor for the register bank?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:930-932
+static MachineInstr *getOtherVRegDef(const MachineRegisterInfo &MRI,
+                                     Register Reg,
+                                     const MachineInstr &MI) {
----------------
I don't understand this, because in SSA there must be exactly 1 def


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:933
+                                     const MachineInstr &MI) {
+  // Is there some way we can asset that there are exactly 2 def instructions?
+  for (MachineInstr &Other : MRI.def_instructions(Reg)) {
----------------
s/asset/assert


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:969
+  // operand to a pointer type.
+  unsigned BasePtrReg = SrcRegs[0];
+  LLT PtrTy = MRI.getType(MI.getOperand(1).getReg());
----------------
Registser


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57399