From 718a229da600e530ecda7c53a16350990d975cb2 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Tue, 19 Mar 2024 16:06:19 -0700 Subject: [PATCH 1/3] [RISCV] Preserve MMO when expanding PseudoRV32ZdinxSD/PseudoRV32ZdinxLD. This allows the asm printer to print the stack spill/reload messages. --- .../Target/RISCV/RISCVExpandPseudoInsts.cpp | 79 +++++++++++++------ llvm/test/CodeGen/RISCV/zdinx-large-spill.mir | 32 ++++---- 2 files changed, 71 insertions(+), 40 deletions(-) diff --git a/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp b/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp index 0a314fdd41cbe..080a37c9a05ed 100644 --- a/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp +++ b/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp @@ -312,26 +312,40 @@ bool RISCVExpandPseudo::expandRV32ZdinxStore(MachineBasicBlock &MBB, TRI->getSubReg(MBBI->getOperand(0).getReg(), RISCV::sub_gpr_even); Register Hi = TRI->getSubReg(MBBI->getOperand(0).getReg(), RISCV::sub_gpr_odd); - BuildMI(MBB, MBBI, DL, TII->get(RISCV::SW)) - .addReg(Lo, getKillRegState(MBBI->getOperand(0).isKill())) - .addReg(MBBI->getOperand(1).getReg()) - .add(MBBI->getOperand(2)); + auto MIBLo = BuildMI(MBB, MBBI, DL, TII->get(RISCV::SW)) + .addReg(Lo, getKillRegState(MBBI->getOperand(0).isKill())) + .addReg(MBBI->getOperand(1).getReg()) + .add(MBBI->getOperand(2)); + + MachineMemOperand *MMOHi = nullptr; + if (MBBI->hasOneMemOperand()) { + MachineMemOperand *OldMMO = MBBI->memoperands().front(); + MachineFunction *MF = MBB.getParent(); + MachineMemOperand *MMOLo = MF->getMachineMemOperand(OldMMO, 0, 4); + MMOHi = MF->getMachineMemOperand(OldMMO, 4, 4); + MIBLo.setMemRefs(MMOLo); + } + if (MBBI->getOperand(2).isGlobal() || MBBI->getOperand(2).isCPI()) { // FIXME: Zdinx RV32 can not work on unaligned memory. assert(!STI->hasFastUnalignedAccess()); assert(MBBI->getOperand(2).getOffset() % 8 == 0); MBBI->getOperand(2).setOffset(MBBI->getOperand(2).getOffset() + 4); - BuildMI(MBB, MBBI, DL, TII->get(RISCV::SW)) - .addReg(Hi, getKillRegState(MBBI->getOperand(0).isKill())) - .add(MBBI->getOperand(1)) - .add(MBBI->getOperand(2)); + auto MIBHi = BuildMI(MBB, MBBI, DL, TII->get(RISCV::SW)) + .addReg(Hi, getKillRegState(MBBI->getOperand(0).isKill())) + .add(MBBI->getOperand(1)) + .add(MBBI->getOperand(2)); + if (MMOHi) + MIBHi.setMemRefs(MMOHi); } else { assert(isInt<12>(MBBI->getOperand(2).getImm() + 4)); - BuildMI(MBB, MBBI, DL, TII->get(RISCV::SW)) - .addReg(Hi, getKillRegState(MBBI->getOperand(0).isKill())) - .add(MBBI->getOperand(1)) - .addImm(MBBI->getOperand(2).getImm() + 4); + auto MIBHi = BuildMI(MBB, MBBI, DL, TII->get(RISCV::SW)) + .addReg(Hi, getKillRegState(MBBI->getOperand(0).isKill())) + .add(MBBI->getOperand(1)) + .addImm(MBBI->getOperand(2).getImm() + 4); + if (MMOHi) + MIBHi.setMemRefs(MMOHi); } MBBI->eraseFromParent(); return true; @@ -349,36 +363,53 @@ bool RISCVExpandPseudo::expandRV32ZdinxLoad(MachineBasicBlock &MBB, Register Hi = TRI->getSubReg(MBBI->getOperand(0).getReg(), RISCV::sub_gpr_odd); + MachineMemOperand *MMOLo = nullptr; + MachineMemOperand *MMOHi = nullptr; + if (MBBI->hasOneMemOperand()) { + MachineMemOperand *OldMMO = MBBI->memoperands().front(); + MachineFunction *MF = MBB.getParent(); + MMOLo = MF->getMachineMemOperand(OldMMO, 0, 4); + MMOHi = MF->getMachineMemOperand(OldMMO, 4, 4); + } + // If the register of operand 1 is equal to the Lo register, then swap the // order of loading the Lo and Hi statements. bool IsOp1EqualToLo = Lo == MBBI->getOperand(1).getReg(); // Order: Lo, Hi if (!IsOp1EqualToLo) { - BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Lo) - .addReg(MBBI->getOperand(1).getReg()) - .add(MBBI->getOperand(2)); + auto MIBLo = BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Lo) + .addReg(MBBI->getOperand(1).getReg()) + .add(MBBI->getOperand(2)); + if (MMOLo) + MIBLo.setMemRefs(MMOLo); } if (MBBI->getOperand(2).isGlobal() || MBBI->getOperand(2).isCPI()) { auto Offset = MBBI->getOperand(2).getOffset(); assert(MBBI->getOperand(2).getOffset() % 8 == 0); MBBI->getOperand(2).setOffset(Offset + 4); - BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Hi) - .addReg(MBBI->getOperand(1).getReg()) - .add(MBBI->getOperand(2)); + auto MIBHi = BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Hi) + .addReg(MBBI->getOperand(1).getReg()) + .add(MBBI->getOperand(2)); MBBI->getOperand(2).setOffset(Offset); + if (MMOHi) + MIBHi.setMemRefs(MMOHi); } else { assert(isInt<12>(MBBI->getOperand(2).getImm() + 4)); - BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Hi) - .addReg(MBBI->getOperand(1).getReg()) - .addImm(MBBI->getOperand(2).getImm() + 4); + auto MIBHi = BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Hi) + .addReg(MBBI->getOperand(1).getReg()) + .addImm(MBBI->getOperand(2).getImm() + 4); + if (MMOHi) + MIBHi.setMemRefs(MMOHi); } // Order: Hi, Lo if (IsOp1EqualToLo) { - BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Lo) - .addReg(MBBI->getOperand(1).getReg()) - .add(MBBI->getOperand(2)); + auto MIBLo = BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Lo) + .addReg(MBBI->getOperand(1).getReg()) + .add(MBBI->getOperand(2)); + if (MMOLo) + MIBLo.setMemRefs(MMOLo); } MBBI->eraseFromParent(); diff --git a/llvm/test/CodeGen/RISCV/zdinx-large-spill.mir b/llvm/test/CodeGen/RISCV/zdinx-large-spill.mir index a2a722a7f7282..8596a65c378c6 100644 --- a/llvm/test/CodeGen/RISCV/zdinx-large-spill.mir +++ b/llvm/test/CodeGen/RISCV/zdinx-large-spill.mir @@ -14,28 +14,28 @@ ; CHECK-NEXT: .cfi_def_cfa_offset 2064 ; CHECK-NEXT: lui t0, 1 ; CHECK-NEXT: add t0, sp, t0 - ; CHECK-NEXT: sw a0, -2040(t0) - ; CHECK-NEXT: sw a1, -2036(t0) + ; CHECK-NEXT: sw a0, -2040(t0) # 4-byte Folded Spill + ; CHECK-NEXT: sw a1, -2036(t0) # 4-byte Folded Spill ; CHECK-NEXT: lui a0, 1 ; CHECK-NEXT: add a0, sp, a0 - ; CHECK-NEXT: sw a2, -2048(a0) - ; CHECK-NEXT: sw a3, -2044(a0) - ; CHECK-NEXT: sw a4, 2040(sp) - ; CHECK-NEXT: sw a5, 2044(sp) - ; CHECK-NEXT: sw a6, 2032(sp) - ; CHECK-NEXT: sw a7, 2036(sp) + ; CHECK-NEXT: sw a2, -2048(a0) # 4-byte Folded Spill + ; CHECK-NEXT: sw a3, -2044(a0) # 4-byte Folded Spill + ; CHECK-NEXT: sw a4, 2040(sp) # 4-byte Folded Spill + ; CHECK-NEXT: sw a5, 2044(sp) # 4-byte Folded Spill + ; CHECK-NEXT: sw a6, 2032(sp) # 4-byte Folded Spill + ; CHECK-NEXT: sw a7, 2036(sp) # 4-byte Folded Spill ; CHECK-NEXT: lui a0, 1 ; CHECK-NEXT: add a0, sp, a0 - ; CHECK-NEXT: lw a1, -2036(a0) - ; CHECK-NEXT: lw a0, -2040(a0) + ; CHECK-NEXT: lw a1, -2036(a0) # 4-byte Folded Reload + ; CHECK-NEXT: lw a0, -2040(a0) # 4-byte Folded Reload ; CHECK-NEXT: lui a0, 1 ; CHECK-NEXT: add a0, sp, a0 - ; CHECK-NEXT: lw a2, -2048(a0) - ; CHECK-NEXT: lw a3, -2044(a0) - ; CHECK-NEXT: lw a4, 2040(sp) - ; CHECK-NEXT: lw a5, 2044(sp) - ; CHECK-NEXT: lw a6, 2032(sp) - ; CHECK-NEXT: lw a7, 2036(sp) + ; CHECK-NEXT: lw a2, -2048(a0) # 4-byte Folded Reload + ; CHECK-NEXT: lw a3, -2044(a0) # 4-byte Folded Reload + ; CHECK-NEXT: lw a4, 2040(sp) # 4-byte Folded Reload + ; CHECK-NEXT: lw a5, 2044(sp) # 4-byte Folded Reload + ; CHECK-NEXT: lw a6, 2032(sp) # 4-byte Folded Reload + ; CHECK-NEXT: lw a7, 2036(sp) # 4-byte Folded Reload ; CHECK-NEXT: addi sp, sp, 2032 ; CHECK-NEXT: addi sp, sp, 32 ; CHECK-NEXT: ret From 394b417f04e84effed07f331fd08da4e68be28db Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 20 Mar 2024 13:27:46 -0700 Subject: [PATCH 2/3] fixup! Assume the mem operand is always there. It's a bug if its not so let's just assume it to simplify the code. --- .../Target/RISCV/RISCVExpandPseudoInsts.cpp | 45 +++++++------------ 1 file changed, 17 insertions(+), 28 deletions(-) diff --git a/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp b/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp index 080a37c9a05ed..b62e7a5192c94 100644 --- a/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp +++ b/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp @@ -317,14 +317,12 @@ bool RISCVExpandPseudo::expandRV32ZdinxStore(MachineBasicBlock &MBB, .addReg(MBBI->getOperand(1).getReg()) .add(MBBI->getOperand(2)); - MachineMemOperand *MMOHi = nullptr; - if (MBBI->hasOneMemOperand()) { - MachineMemOperand *OldMMO = MBBI->memoperands().front(); - MachineFunction *MF = MBB.getParent(); - MachineMemOperand *MMOLo = MF->getMachineMemOperand(OldMMO, 0, 4); - MMOHi = MF->getMachineMemOperand(OldMMO, 4, 4); - MIBLo.setMemRefs(MMOLo); - } + assert(MBBI->hasOneMemOperand() && "Expected mem operand"); + MachineMemOperand *OldMMO = MBBI->memoperands().front(); + MachineFunction *MF = MBB.getParent(); + MachineMemOperand *MMOLo = MF->getMachineMemOperand(OldMMO, 0, 4); + MachineMemOperand *MMOHi = MF->getMachineMemOperand(OldMMO, 4, 4); + MIBLo.setMemRefs(MMOLo); if (MBBI->getOperand(2).isGlobal() || MBBI->getOperand(2).isCPI()) { // FIXME: Zdinx RV32 can not work on unaligned memory. @@ -336,16 +334,14 @@ bool RISCVExpandPseudo::expandRV32ZdinxStore(MachineBasicBlock &MBB, .addReg(Hi, getKillRegState(MBBI->getOperand(0).isKill())) .add(MBBI->getOperand(1)) .add(MBBI->getOperand(2)); - if (MMOHi) - MIBHi.setMemRefs(MMOHi); + MIBHi.setMemRefs(MMOHi); } else { assert(isInt<12>(MBBI->getOperand(2).getImm() + 4)); auto MIBHi = BuildMI(MBB, MBBI, DL, TII->get(RISCV::SW)) .addReg(Hi, getKillRegState(MBBI->getOperand(0).isKill())) .add(MBBI->getOperand(1)) .addImm(MBBI->getOperand(2).getImm() + 4); - if (MMOHi) - MIBHi.setMemRefs(MMOHi); + MIBHi.setMemRefs(MMOHi); } MBBI->eraseFromParent(); return true; @@ -363,14 +359,11 @@ bool RISCVExpandPseudo::expandRV32ZdinxLoad(MachineBasicBlock &MBB, Register Hi = TRI->getSubReg(MBBI->getOperand(0).getReg(), RISCV::sub_gpr_odd); - MachineMemOperand *MMOLo = nullptr; - MachineMemOperand *MMOHi = nullptr; - if (MBBI->hasOneMemOperand()) { - MachineMemOperand *OldMMO = MBBI->memoperands().front(); - MachineFunction *MF = MBB.getParent(); - MMOLo = MF->getMachineMemOperand(OldMMO, 0, 4); - MMOHi = MF->getMachineMemOperand(OldMMO, 4, 4); - } + assert(MBBI->hasOneMemOperand() && "Expected mem operand"); + MachineMemOperand *OldMMO = MBBI->memoperands().front(); + MachineFunction *MF = MBB.getParent(); + MachineMemOperand *MMOLo = MF->getMachineMemOperand(OldMMO, 0, 4); + MachineMemOperand *MMOHi = MF->getMachineMemOperand(OldMMO, 4, 4); // If the register of operand 1 is equal to the Lo register, then swap the // order of loading the Lo and Hi statements. @@ -380,8 +373,7 @@ bool RISCVExpandPseudo::expandRV32ZdinxLoad(MachineBasicBlock &MBB, auto MIBLo = BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Lo) .addReg(MBBI->getOperand(1).getReg()) .add(MBBI->getOperand(2)); - if (MMOLo) - MIBLo.setMemRefs(MMOLo); + MIBLo.setMemRefs(MMOLo); } if (MBBI->getOperand(2).isGlobal() || MBBI->getOperand(2).isCPI()) { @@ -392,15 +384,13 @@ bool RISCVExpandPseudo::expandRV32ZdinxLoad(MachineBasicBlock &MBB, .addReg(MBBI->getOperand(1).getReg()) .add(MBBI->getOperand(2)); MBBI->getOperand(2).setOffset(Offset); - if (MMOHi) - MIBHi.setMemRefs(MMOHi); + MIBHi.setMemRefs(MMOHi); } else { assert(isInt<12>(MBBI->getOperand(2).getImm() + 4)); auto MIBHi = BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Hi) .addReg(MBBI->getOperand(1).getReg()) .addImm(MBBI->getOperand(2).getImm() + 4); - if (MMOHi) - MIBHi.setMemRefs(MMOHi); + MIBHi.setMemRefs(MMOHi); } // Order: Hi, Lo @@ -408,8 +398,7 @@ bool RISCVExpandPseudo::expandRV32ZdinxLoad(MachineBasicBlock &MBB, auto MIBLo = BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Lo) .addReg(MBBI->getOperand(1).getReg()) .add(MBBI->getOperand(2)); - if (MMOLo) - MIBLo.setMemRefs(MMOLo); + MIBLo.setMemRefs(MMOLo); } MBBI->eraseFromParent(); From 30cdcdef0bb5665bc05e3d9360c8776c5242fc90 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Thu, 21 Mar 2024 09:52:29 -0700 Subject: [PATCH 3/3] fixup! Add setMemRefs to the end of the existing BuildMI calls. --- .../Target/RISCV/RISCVExpandPseudoInsts.cpp | 63 ++++++++++--------- 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp b/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp index b62e7a5192c94..173995f05b51c 100644 --- a/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp +++ b/llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp @@ -312,17 +312,18 @@ bool RISCVExpandPseudo::expandRV32ZdinxStore(MachineBasicBlock &MBB, TRI->getSubReg(MBBI->getOperand(0).getReg(), RISCV::sub_gpr_even); Register Hi = TRI->getSubReg(MBBI->getOperand(0).getReg(), RISCV::sub_gpr_odd); - auto MIBLo = BuildMI(MBB, MBBI, DL, TII->get(RISCV::SW)) - .addReg(Lo, getKillRegState(MBBI->getOperand(0).isKill())) - .addReg(MBBI->getOperand(1).getReg()) - .add(MBBI->getOperand(2)); assert(MBBI->hasOneMemOperand() && "Expected mem operand"); MachineMemOperand *OldMMO = MBBI->memoperands().front(); MachineFunction *MF = MBB.getParent(); MachineMemOperand *MMOLo = MF->getMachineMemOperand(OldMMO, 0, 4); MachineMemOperand *MMOHi = MF->getMachineMemOperand(OldMMO, 4, 4); - MIBLo.setMemRefs(MMOLo); + + BuildMI(MBB, MBBI, DL, TII->get(RISCV::SW)) + .addReg(Lo, getKillRegState(MBBI->getOperand(0).isKill())) + .addReg(MBBI->getOperand(1).getReg()) + .add(MBBI->getOperand(2)) + .setMemRefs(MMOLo); if (MBBI->getOperand(2).isGlobal() || MBBI->getOperand(2).isCPI()) { // FIXME: Zdinx RV32 can not work on unaligned memory. @@ -330,18 +331,18 @@ bool RISCVExpandPseudo::expandRV32ZdinxStore(MachineBasicBlock &MBB, assert(MBBI->getOperand(2).getOffset() % 8 == 0); MBBI->getOperand(2).setOffset(MBBI->getOperand(2).getOffset() + 4); - auto MIBHi = BuildMI(MBB, MBBI, DL, TII->get(RISCV::SW)) - .addReg(Hi, getKillRegState(MBBI->getOperand(0).isKill())) - .add(MBBI->getOperand(1)) - .add(MBBI->getOperand(2)); - MIBHi.setMemRefs(MMOHi); + BuildMI(MBB, MBBI, DL, TII->get(RISCV::SW)) + .addReg(Hi, getKillRegState(MBBI->getOperand(0).isKill())) + .add(MBBI->getOperand(1)) + .add(MBBI->getOperand(2)) + .setMemRefs(MMOHi); } else { assert(isInt<12>(MBBI->getOperand(2).getImm() + 4)); - auto MIBHi = BuildMI(MBB, MBBI, DL, TII->get(RISCV::SW)) - .addReg(Hi, getKillRegState(MBBI->getOperand(0).isKill())) - .add(MBBI->getOperand(1)) - .addImm(MBBI->getOperand(2).getImm() + 4); - MIBHi.setMemRefs(MMOHi); + BuildMI(MBB, MBBI, DL, TII->get(RISCV::SW)) + .addReg(Hi, getKillRegState(MBBI->getOperand(0).isKill())) + .add(MBBI->getOperand(1)) + .addImm(MBBI->getOperand(2).getImm() + 4) + .setMemRefs(MMOHi); } MBBI->eraseFromParent(); return true; @@ -370,35 +371,35 @@ bool RISCVExpandPseudo::expandRV32ZdinxLoad(MachineBasicBlock &MBB, bool IsOp1EqualToLo = Lo == MBBI->getOperand(1).getReg(); // Order: Lo, Hi if (!IsOp1EqualToLo) { - auto MIBLo = BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Lo) - .addReg(MBBI->getOperand(1).getReg()) - .add(MBBI->getOperand(2)); - MIBLo.setMemRefs(MMOLo); + BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Lo) + .addReg(MBBI->getOperand(1).getReg()) + .add(MBBI->getOperand(2)) + .setMemRefs(MMOLo); } if (MBBI->getOperand(2).isGlobal() || MBBI->getOperand(2).isCPI()) { auto Offset = MBBI->getOperand(2).getOffset(); assert(MBBI->getOperand(2).getOffset() % 8 == 0); MBBI->getOperand(2).setOffset(Offset + 4); - auto MIBHi = BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Hi) - .addReg(MBBI->getOperand(1).getReg()) - .add(MBBI->getOperand(2)); + BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Hi) + .addReg(MBBI->getOperand(1).getReg()) + .add(MBBI->getOperand(2)) + .setMemRefs(MMOHi); MBBI->getOperand(2).setOffset(Offset); - MIBHi.setMemRefs(MMOHi); } else { assert(isInt<12>(MBBI->getOperand(2).getImm() + 4)); - auto MIBHi = BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Hi) - .addReg(MBBI->getOperand(1).getReg()) - .addImm(MBBI->getOperand(2).getImm() + 4); - MIBHi.setMemRefs(MMOHi); + BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Hi) + .addReg(MBBI->getOperand(1).getReg()) + .addImm(MBBI->getOperand(2).getImm() + 4) + .setMemRefs(MMOHi); } // Order: Hi, Lo if (IsOp1EqualToLo) { - auto MIBLo = BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Lo) - .addReg(MBBI->getOperand(1).getReg()) - .add(MBBI->getOperand(2)); - MIBLo.setMemRefs(MMOLo); + BuildMI(MBB, MBBI, DL, TII->get(RISCV::LW), Lo) + .addReg(MBBI->getOperand(1).getReg()) + .add(MBBI->getOperand(2)) + .setMemRefs(MMOLo); } MBBI->eraseFromParent();