[PATCH] D80545: [AMDGPU/MemOpsCluster] Let mem ops clustering logic also consider number of clustered bytes
Jay Foad via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 29 05:56:49 PDT 2020
foad added inline comments.
================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:277-292
+const MachineOperand *
+SIInstrInfo::getDstMemOperand(const MachineInstr &LdSt) const {
+ const MachineOperand *DstOp = nullptr;
+
+ if (isMUBUF(LdSt) || isMTBUF(LdSt) || isFLAT(LdSt)) {
+ DstOp = getNamedOperand(LdSt, AMDGPU::OpName::vdata);
+ if (!DstOp)
----------------
I would prefer this logic to be folded into getMemOperandsWithOffsetWidth itself, where we already test for all the different instruction types (isMUBUF and so on). It might make sense for each of the cases in getMemOperandsWithOffsetWidth to set DstOp and then fall through to some common code at the end of the function that sets Width based on DstOp.
================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:302
+ const MachineOperand *DstOp = getDstMemOperand(LdSt);
+ Width = DstOp ? getDstOpSizeInBytes(LdSt, DstOp) : 0;
+
----------------
Hopefully if you follow the advice above, you will find DstOp is never null (because it is only ever null here in cases where getMemOperandsWithOffsetWidth is going to return false anyway).
If you do still find cases where DstOp is null then please debug them to see what has gone wrong.
================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:2775-2780
if (!MIa.hasOneMemOperand() || !MIb.hasOneMemOperand()) {
// FIXME: Handle ds_read2 / ds_write2.
return false;
}
unsigned Width0 = MIa.memoperands().front()->getSize();
unsigned Width1 = MIb.memoperands().front()->getSize();
----------------
This code can be removed now. You should be able to use the Width values returned by getMemOperandsWithOffsetWidth instead. But you can leave that for a later patch if you prefer.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80545/new/
https://reviews.llvm.org/D80545
More information about the llvm-commits
mailing list