[PATCH] D144313: [AMDGPU] Improve the lowering of raw_buffer_load_{i8,i16} and struct_buffer_load_{i8,i16} intrinsics
Jay Foad via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 21 01:17:10 PST 2023
foad added a comment.
Looks good with a few suggested changes.
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCombinerHelper.cpp:400-404
+ if (SubwordBufferLoad->getOpcode() == AMDGPU::G_AMDGPU_BUFFER_LOAD_UBYTE ||
+ SubwordBufferLoad->getOpcode() == AMDGPU::G_AMDGPU_BUFFER_LOAD_USHORT)
+ return true;
+
+ return false;
----------------
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCombinerHelper.cpp:411-412
+ MachineInstr &MI, MachineInstr *&SubwordBufferLoad) {
+ Register Op0Reg = MI.getOperand(1).getReg();
+ SubwordBufferLoad = (*(MRI.def_begin(Op0Reg))).getParent();
+ // Modify the opcode and the destination of buffer_load_{u8, u16}:
----------------
Don't need these two lines. SubwordBufferLoad is passed in as an argument now.
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCombinerHelper.cpp:416
+ const GCNSubtarget &Subtarget = MI.getMF()->getSubtarget<GCNSubtarget>();
+ const SIInstrInfo *TII = Subtarget.getInstrInfo();
+ unsigned Opc =
----------------
I think all this new code should probably live in `AMDGPUPostLegalizerCombinerHelper` instead of `AMDGPUCombinerHelper`, and you should add `TII` as a member variable in `AMDGPUPostLegalizerCombinerHelper` following the example of `AMDGPURegBankCombinerHelper`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144313/new/
https://reviews.llvm.org/D144313
More information about the llvm-commits
mailing list