From 3ce74d65c182b78443550f141874db7dcc773a92 Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Tue, 8 Oct 2024 13:53:37 +0100 Subject: [PATCH 01/10] [CodeGen] Correctly handle non-standard cases in RemoveLoadsIntoFakeUses In the RemoveLoadsIntoFakeUses pass, we try to remove loads that are only used by fake uses, as well as the fake use in question. There are two existing errors with the pass however: it incorrectly examines every operand of each FAKE_USE, when only the first is relevant (extra operands should just be "killed" regs), and it ignores cases where the FAKE_USE register is not an exact match for the loaded register, which is incorrect as regalloc may choose to load a wider value than was required pre-regalloc. This patch fixes both of these cases. --- llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp | 60 ++++---- .../CodeGen/X86/fake-use-remove-loads.mir | 129 ++++++++++++++++++ 2 files changed, 165 insertions(+), 24 deletions(-) create mode 100644 llvm/test/CodeGen/X86/fake-use-remove-loads.mir diff --git a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp index ef7a58670c3ac..0670e1b887128 100644 --- a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp +++ b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp @@ -94,11 +94,13 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { for (MachineInstr &MI : make_early_inc_range(reverse(*MBB))) { if (MI.isFakeUse()) { - for (const MachineOperand &MO : MI.operands()) { - // Track the Fake Uses that use this register so that we can delete - // them if we delete the corresponding load. - if (MO.isReg()) - RegFakeUses[MO.getReg()].push_back(&MI); + const MachineOperand &FakeUseOp = MI.getOperand(0); + // Track the Fake Uses that use this register so that we can delete + // them if we delete the corresponding load. + if (FakeUseOp.isReg()) { + assert(FakeUseOp.getReg().isPhysical() && + "VReg seen in function with NoVRegs set?"); + RegFakeUses[FakeUseOp.getReg()].push_back(&MI); } // Do not record FAKE_USE uses in LivePhysRegs so that we can recognize // otherwise-unused loads. @@ -113,27 +115,34 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { // Don't delete live physreg defs, or any reserved register defs. if (!LivePhysRegs.available(Reg) || MRI->isReserved(Reg)) continue; - // There should be an exact match between the loaded register and the - // FAKE_USE use. If not, this is a load that is unused by anything? It - // should probably be deleted, but that's outside of this pass' scope. - if (RegFakeUses.contains(Reg)) { - LLVM_DEBUG(dbgs() << "RemoveLoadsIntoFakeUses: DELETING: " << MI); - // It is possible that some DBG_VALUE instructions refer to this - // instruction. They will be deleted in the live debug variable - // analysis. - MI.eraseFromParent(); - AnyChanges = true; - ++NumLoadsDeleted; - // Each FAKE_USE now appears to be a fake use of the previous value - // of the loaded register; delete them to avoid incorrectly - // interpreting them as such. - for (MachineInstr *FakeUse : RegFakeUses[Reg]) { + // There should typically be an exact match between the loaded register + // and the FAKE_USE, but sometimes regalloc will choose to load a larger + // value than is needed. Therefore, as long as the load isn't used by + // anything except at least one FAKE_USE, we will delete it. If it isn't + // used by any fake uses, it should still be safe to delete but we + // choose to ignore it so that this pass has no side effects unrelated + // to fake uses. + bool HasFakeUse = false; + for (MCPhysReg SubReg : TRI->subregs_inclusive(Reg)) { + if (!RegFakeUses.contains(SubReg)) + continue; + HasFakeUse = true; + for (MachineInstr *FakeUse : RegFakeUses[SubReg]) { LLVM_DEBUG(dbgs() << "RemoveLoadsIntoFakeUses: DELETING: " << *FakeUse); FakeUse->eraseFromParent(); } - NumFakeUsesDeleted += RegFakeUses[Reg].size(); - RegFakeUses[Reg].clear(); + NumFakeUsesDeleted += RegFakeUses[SubReg].size(); + RegFakeUses[SubReg].clear(); + } + if (HasFakeUse) { + LLVM_DEBUG(dbgs() << "RemoveLoadsIntoFakeUses: DELETING: " << MI); + // Since this load only exists to restore a spilled register and we + // haven't, run LiveDebugValues yet, there shouldn't be any DBG_VALUEs + // for this load; otherwise, deleting this would be incorrect. + MI.eraseFromParent(); + AnyChanges = true; + ++NumLoadsDeleted; } continue; } @@ -147,8 +156,11 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { Register Reg = MO.getReg(); assert(Reg.isPhysical() && "VReg seen in function with NoVRegs set?"); - for (MCRegUnit Unit : TRI->regunits(Reg)) - RegFakeUses.erase(Unit); + // We clear RegFakeUses for this register and all subregisters, + // because any such FAKE_USE encountered prior applies only to this + // instruction. + for (MCPhysReg SubReg : TRI->subregs_inclusive(Reg)) + RegFakeUses.erase(SubReg); } } } diff --git a/llvm/test/CodeGen/X86/fake-use-remove-loads.mir b/llvm/test/CodeGen/X86/fake-use-remove-loads.mir new file mode 100644 index 0000000000000..a8edbed5b28fe --- /dev/null +++ b/llvm/test/CodeGen/X86/fake-use-remove-loads.mir @@ -0,0 +1,129 @@ +# Ensure that loads into FAKE_USEs are correctly removed by the +# remove-loads-into-fake-uses pass. +# RUN: llc -run-pass remove-loads-into-fake-uses -mtriple=x86_64-unknown-linux -debug-only=remove-loads-into-fake-uses 2>&1 -o - %s | FileCheck %s --implicit-check-not=DELETING +# REQUIRES: asserts +# +## We ensure that the load into the FAKE_USE is removed, along with the FAKE_USE +## itself, even when the FAKE_USE is for a subregister of the move, and that we +## correctly handle situations where FAKE_USE has additional `killed` operands +## added by other passes. + +# CHECK: DELETING: FAKE_USE renamable $eax +# CHECK: DELETING: renamable $rax = MOV64rm $rbp + +## Also verify that the store to the stack slot still exists. + +# CHECK-LABEL: bb.5: +# CHECK: MOV64mi32 $rbp, 1, $noreg, -48, $noreg, 0 :: (store (s64) into %stack.0) +# CHECK-LABEL: bb.6: + + +--- | + define void @_ZN1g1jEv(ptr %this, i1 %cmp36, ptr %ref.tmp5) { + entry: + ret void + } + +... +--- +name: _ZN1g1jEv +alignment: 16 +tracksRegLiveness: true +noPhis: true +noVRegs: true +hasFakeUses: true +tracksDebugUserValues: true +liveins: + - { reg: '$rdi', virtual-reg: '' } + - { reg: '$esi', virtual-reg: '' } + - { reg: '$rdx', virtual-reg: '' } +frameInfo: + isCalleeSavedInfoValid: true +stack: + - { id: 0, name: '', type: spill-slot, offset: -64, size: 8, alignment: 8, + stack-id: default, callee-saved-register: '', callee-saved-restored: true, + debug-info-variable: '', debug-info-expression: '', debug-info-location: '' } +body: | + bb.0: + successors: %bb.2(0x80000000) + liveins: $esi, $rdi, $rdx, $r15, $r14, $r13, $r12, $rbx + + frame-setup PUSH64r killed $rbp, implicit-def $rsp, implicit $rsp + frame-setup CFI_INSTRUCTION def_cfa_offset 16 + frame-setup CFI_INSTRUCTION offset $rbp, -16 + $rbp = frame-setup MOV64rr $rsp + frame-setup CFI_INSTRUCTION def_cfa_register $rbp + frame-setup PUSH64r killed $r15, implicit-def $rsp, implicit $rsp + frame-setup PUSH64r killed $r14, implicit-def $rsp, implicit $rsp + frame-setup PUSH64r killed $r13, implicit-def $rsp, implicit $rsp + frame-setup PUSH64r killed $r12, implicit-def $rsp, implicit $rsp + frame-setup PUSH64r killed $rbx, implicit-def $rsp, implicit $rsp + frame-setup PUSH64r undef $rax, implicit-def $rsp, implicit $rsp + CFI_INSTRUCTION offset $rbx, -56 + CFI_INSTRUCTION offset $r12, -48 + CFI_INSTRUCTION offset $r13, -40 + CFI_INSTRUCTION offset $r14, -32 + CFI_INSTRUCTION offset $r15, -24 + $rbx = MOV64rr $rdx + $r14d = MOV32rr $esi + $r15 = MOV64rr $rdi + renamable $r12d = XOR32rr undef $r12d, undef $r12d, implicit-def dead $eflags, implicit-def $r12 + renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $rax + JMP_1 %bb.2 + + bb.1: + successors: %bb.2(0x783e0f0f), %bb.6(0x07c1f0f1) + liveins: $rbx, $r12, $r15, $r13d, $r14d + + renamable $rax = MOV64rm $rbp, 1, $noreg, -48, $noreg :: (load (s64) from %stack.0) + FAKE_USE renamable $eax, implicit killed $rax + renamable $eax = MOV32ri 1, implicit-def $rax + TEST32rr killed renamable $r13d, renamable $r13d, implicit-def $eflags + JCC_1 %bb.6, 9, implicit $eflags + + bb.2: + successors: %bb.3(0x80000000) + liveins: $rax, $rbx, $r12, $r15, $r14d + + MOV64mr $rbp, 1, $noreg, -48, $noreg, killed renamable $rax :: (store (s64) into %stack.0) + renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags + + bb.3: + successors: %bb.4(0x04000000), %bb.3(0x7c000000) + liveins: $eax, $rbx, $r12, $r15, $r14d + + $r13d = MOV32rr killed $eax + $rdi = MOV64rr $r15 + CALL64r renamable $r12, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp + dead renamable $eax = MOV32rm renamable $rbx, 1, $noreg, 0, $noreg :: (volatile load (s32) from %ir.ref.tmp5) + renamable $eax = MOV32ri 1 + TEST8ri renamable $r14b, 1, implicit-def $eflags + JCC_1 %bb.3, 4, implicit $eflags + + bb.4: + successors: %bb.5(0x40000000), %bb.1(0x40000000) + liveins: $eflags, $rbx, $r12, $r15, $r13d, $r14d + + JCC_1 %bb.1, 4, implicit $eflags + + bb.5: + successors: %bb.1(0x80000000) + liveins: $rbx, $r12, $r15, $r13d, $r14d + + renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $rax + MOV64mi32 $rbp, 1, $noreg, -48, $noreg, 0 :: (store (s64) into %stack.0) + CALL64r killed renamable $rax, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def dead $eax + JMP_1 %bb.1 + + bb.6: + $rsp = frame-destroy ADD64ri32 $rsp, 8, implicit-def dead $eflags + $rbx = frame-destroy POP64r implicit-def $rsp, implicit $rsp + $r12 = frame-destroy POP64r implicit-def $rsp, implicit $rsp + $r13 = frame-destroy POP64r implicit-def $rsp, implicit $rsp + $r14 = frame-destroy POP64r implicit-def $rsp, implicit $rsp + $r15 = frame-destroy POP64r implicit-def $rsp, implicit $rsp + $rbp = frame-destroy POP64r implicit-def $rsp, implicit $rsp + frame-destroy CFI_INSTRUCTION def_cfa $rsp, 8 + RET64 + +... From fd2c94e716678033ba1fb0e361f692e74eea0b2b Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Tue, 8 Oct 2024 17:45:39 +0100 Subject: [PATCH 02/10] Remove assertions, handle 0-op FAKE_USES, use RegUnits --- llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp | 49 +++++++++---------- .../CodeGen/X86/fake-use-remove-loads.mir | 2 +- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp index 0670e1b887128..b56a07010188d 100644 --- a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp +++ b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp @@ -86,22 +86,23 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { const TargetInstrInfo *TII = ST.getInstrInfo(); const TargetRegisterInfo *TRI = ST.getRegisterInfo(); - SmallDenseMap> RegFakeUses; + SmallDenseMap> RegFakeUses; LivePhysRegs.init(*TRI); SmallVector Statepoints; for (MachineBasicBlock *MBB : post_order(&MF)) { + RegFakeUses.clear(); LivePhysRegs.addLiveOuts(*MBB); for (MachineInstr &MI : make_early_inc_range(reverse(*MBB))) { if (MI.isFakeUse()) { + if (MI.getNumOperands() == 0 || !MI.getOperand(0).isReg()) + continue; const MachineOperand &FakeUseOp = MI.getOperand(0); - // Track the Fake Uses that use this register so that we can delete - // them if we delete the corresponding load. - if (FakeUseOp.isReg()) { - assert(FakeUseOp.getReg().isPhysical() && - "VReg seen in function with NoVRegs set?"); - RegFakeUses[FakeUseOp.getReg()].push_back(&MI); - } + // Track the Fake Uses that use these register units so that we can + // delete them if we delete the corresponding load. + if (FakeUseOp.isReg()) + for (MCRegUnit Unit : TRI->regunits(FakeUseOp.getReg())) + RegFakeUses[Unit].push_back(&MI); // Do not record FAKE_USE uses in LivePhysRegs so that we can recognize // otherwise-unused loads. continue; @@ -111,7 +112,6 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { // reload of a spilled register. if (MI.getRestoreSize(TII)) { Register Reg = MI.getOperand(0).getReg(); - assert(Reg.isPhysical() && "VReg seen in function with NoVRegs set?"); // Don't delete live physreg defs, or any reserved register defs. if (!LivePhysRegs.available(Reg) || MRI->isReserved(Reg)) continue; @@ -122,20 +122,15 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { // used by any fake uses, it should still be safe to delete but we // choose to ignore it so that this pass has no side effects unrelated // to fake uses. - bool HasFakeUse = false; - for (MCPhysReg SubReg : TRI->subregs_inclusive(Reg)) { - if (!RegFakeUses.contains(SubReg)) + SmallDenseSet FakeUsesToDelete; + for (MCRegUnit Unit : TRI->regunits(Reg)) { + if (!RegFakeUses.contains(Unit)) continue; - HasFakeUse = true; - for (MachineInstr *FakeUse : RegFakeUses[SubReg]) { - LLVM_DEBUG(dbgs() - << "RemoveLoadsIntoFakeUses: DELETING: " << *FakeUse); - FakeUse->eraseFromParent(); - } - NumFakeUsesDeleted += RegFakeUses[SubReg].size(); - RegFakeUses[SubReg].clear(); + for (MachineInstr *FakeUse : RegFakeUses[Unit]) + FakeUsesToDelete.insert(FakeUse); + RegFakeUses.erase(Unit); } - if (HasFakeUse) { + if (!FakeUsesToDelete.empty()) { LLVM_DEBUG(dbgs() << "RemoveLoadsIntoFakeUses: DELETING: " << MI); // Since this load only exists to restore a spilled register and we // haven't, run LiveDebugValues yet, there shouldn't be any DBG_VALUEs @@ -143,6 +138,12 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { MI.eraseFromParent(); AnyChanges = true; ++NumLoadsDeleted; + for (MachineInstr *FakeUse : FakeUsesToDelete) { + LLVM_DEBUG(dbgs() + << "RemoveLoadsIntoFakeUses: DELETING: " << *FakeUse); + FakeUse->eraseFromParent(); + } + NumFakeUsesDeleted += FakeUsesToDelete.size(); } continue; } @@ -154,13 +155,11 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { for (const MachineOperand &MO : MI.operands()) { if (MO.isReg() && MO.isDef()) { Register Reg = MO.getReg(); - assert(Reg.isPhysical() && - "VReg seen in function with NoVRegs set?"); // We clear RegFakeUses for this register and all subregisters, // because any such FAKE_USE encountered prior applies only to this // instruction. - for (MCPhysReg SubReg : TRI->subregs_inclusive(Reg)) - RegFakeUses.erase(SubReg); + for (MCRegUnit Unit : TRI->regunits(Reg)) + RegFakeUses.erase(Unit); } } } diff --git a/llvm/test/CodeGen/X86/fake-use-remove-loads.mir b/llvm/test/CodeGen/X86/fake-use-remove-loads.mir index a8edbed5b28fe..e001bbec637ac 100644 --- a/llvm/test/CodeGen/X86/fake-use-remove-loads.mir +++ b/llvm/test/CodeGen/X86/fake-use-remove-loads.mir @@ -8,8 +8,8 @@ ## correctly handle situations where FAKE_USE has additional `killed` operands ## added by other passes. -# CHECK: DELETING: FAKE_USE renamable $eax # CHECK: DELETING: renamable $rax = MOV64rm $rbp +# CHECK: DELETING: FAKE_USE renamable $eax ## Also verify that the store to the stack slot still exists. From 0a8b5a680532b285372267a8264c580c1148870a Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Mon, 14 Oct 2024 11:21:52 +0100 Subject: [PATCH 03/10] Use a SmallVector of fake uses instead of a SmallDenseMap --- llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp | 32 +++--- .../CodeGen/X86/fake-use-remove-loads.mir | 99 +++++-------------- 2 files changed, 38 insertions(+), 93 deletions(-) diff --git a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp index b56a07010188d..74ad2fe3858ea 100644 --- a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp +++ b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp @@ -86,7 +86,7 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { const TargetInstrInfo *TII = ST.getInstrInfo(); const TargetRegisterInfo *TRI = ST.getRegisterInfo(); - SmallDenseMap> RegFakeUses; + SmallVector RegFakeUses; LivePhysRegs.init(*TRI); SmallVector Statepoints; for (MachineBasicBlock *MBB : post_order(&MF)) { @@ -101,8 +101,7 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { // Track the Fake Uses that use these register units so that we can // delete them if we delete the corresponding load. if (FakeUseOp.isReg()) - for (MCRegUnit Unit : TRI->regunits(FakeUseOp.getReg())) - RegFakeUses[Unit].push_back(&MI); + RegFakeUses.push_back(&MI); // Do not record FAKE_USE uses in LivePhysRegs so that we can recognize // otherwise-unused loads. continue; @@ -123,12 +122,12 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { // choose to ignore it so that this pass has no side effects unrelated // to fake uses. SmallDenseSet FakeUsesToDelete; - for (MCRegUnit Unit : TRI->regunits(Reg)) { - if (!RegFakeUses.contains(Unit)) - continue; - for (MachineInstr *FakeUse : RegFakeUses[Unit]) + SmallVector RemainingFakeUses; + for (MachineInstr *&FakeUse : reverse(RegFakeUses)) { + if (TRI->regsOverlap(Reg, FakeUse->getOperand(0).getReg())) { FakeUsesToDelete.insert(FakeUse); - RegFakeUses.erase(Unit); + RegFakeUses.erase(&FakeUse); + } } if (!FakeUsesToDelete.empty()) { LLVM_DEBUG(dbgs() << "RemoveLoadsIntoFakeUses: DELETING: " << MI); @@ -153,14 +152,15 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { // that register. if (!RegFakeUses.empty()) { for (const MachineOperand &MO : MI.operands()) { - if (MO.isReg() && MO.isDef()) { - Register Reg = MO.getReg(); - // We clear RegFakeUses for this register and all subregisters, - // because any such FAKE_USE encountered prior applies only to this - // instruction. - for (MCRegUnit Unit : TRI->regunits(Reg)) - RegFakeUses.erase(Unit); - } + if (!MO.isReg()) + continue; + Register Reg = MO.getReg(); + // We clear RegFakeUses for this register and all subregisters, + // because any such FAKE_USE encountered prior is no longer relevant + // for later encountered loads. + for (MachineInstr *&FakeUse : reverse(RegFakeUses)) + if (!TRI->regsOverlap(Reg, FakeUse->getOperand(0).getReg())) + RegFakeUses.erase(&FakeUse); } } LivePhysRegs.stepBackward(MI); diff --git a/llvm/test/CodeGen/X86/fake-use-remove-loads.mir b/llvm/test/CodeGen/X86/fake-use-remove-loads.mir index e001bbec637ac..d82434d1b72cf 100644 --- a/llvm/test/CodeGen/X86/fake-use-remove-loads.mir +++ b/llvm/test/CodeGen/X86/fake-use-remove-loads.mir @@ -13,18 +13,17 @@ ## Also verify that the store to the stack slot still exists. -# CHECK-LABEL: bb.5: -# CHECK: MOV64mi32 $rbp, 1, $noreg, -48, $noreg, 0 :: (store (s64) into %stack.0) -# CHECK-LABEL: bb.6: +# CHECK-LABEL: bb.0: +# CHECK: MOV64mr $rbp, 1, $noreg, -48, $noreg, killed renamable $rax :: (store (s64) into %stack.0) +## Finally, verify that when the register has a use between the restore and the +## FAKE_USE, we do not delete the load or fake use. + +# CHECK: renamable $r11 = MOV64rm $rbp, 1, $noreg, -40, $noreg :: (load (s64) from %stack.1) +# CHECK: renamable $r12d = XOR32rr $r12d, $r11d, implicit-def dead $eflags +# CHECK: FAKE_USE killed renamable $r11d ---- | - define void @_ZN1g1jEv(ptr %this, i1 %cmp36, ptr %ref.tmp5) { - entry: - ret void - } -... --- name: _ZN1g1jEv alignment: 16 @@ -40,90 +39,36 @@ liveins: frameInfo: isCalleeSavedInfoValid: true stack: - - { id: 0, name: '', type: spill-slot, offset: -64, size: 8, alignment: 8, + - { id: 0, name: '', type: spill-slot, offset: -8, size: 8, alignment: 8, + stack-id: default, callee-saved-register: '', callee-saved-restored: true, + debug-info-variable: '', debug-info-expression: '', debug-info-location: '' } + - { id: 1, name: '', type: spill-slot, offset: -16, size: 8, alignment: 8, stack-id: default, callee-saved-register: '', callee-saved-restored: true, debug-info-variable: '', debug-info-expression: '', debug-info-location: '' } body: | bb.0: - successors: %bb.2(0x80000000) - liveins: $esi, $rdi, $rdx, $r15, $r14, $r13, $r12, $rbx - - frame-setup PUSH64r killed $rbp, implicit-def $rsp, implicit $rsp - frame-setup CFI_INSTRUCTION def_cfa_offset 16 - frame-setup CFI_INSTRUCTION offset $rbp, -16 - $rbp = frame-setup MOV64rr $rsp - frame-setup CFI_INSTRUCTION def_cfa_register $rbp - frame-setup PUSH64r killed $r15, implicit-def $rsp, implicit $rsp - frame-setup PUSH64r killed $r14, implicit-def $rsp, implicit $rsp - frame-setup PUSH64r killed $r13, implicit-def $rsp, implicit $rsp - frame-setup PUSH64r killed $r12, implicit-def $rsp, implicit $rsp - frame-setup PUSH64r killed $rbx, implicit-def $rsp, implicit $rsp - frame-setup PUSH64r undef $rax, implicit-def $rsp, implicit $rsp - CFI_INSTRUCTION offset $rbx, -56 - CFI_INSTRUCTION offset $r12, -48 - CFI_INSTRUCTION offset $r13, -40 - CFI_INSTRUCTION offset $r14, -32 - CFI_INSTRUCTION offset $r15, -24 + liveins: $esi, $rdi, $rdx, $r15, $r14, $r13, $r12, $r11, $rbx + $rbx = MOV64rr $rdx $r14d = MOV32rr $esi $r15 = MOV64rr $rdi renamable $r12d = XOR32rr undef $r12d, undef $r12d, implicit-def dead $eflags, implicit-def $r12 renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $rax - JMP_1 %bb.2 - - bb.1: - successors: %bb.2(0x783e0f0f), %bb.6(0x07c1f0f1) - liveins: $rbx, $r12, $r15, $r13d, $r14d - - renamable $rax = MOV64rm $rbp, 1, $noreg, -48, $noreg :: (load (s64) from %stack.0) - FAKE_USE renamable $eax, implicit killed $rax - renamable $eax = MOV32ri 1, implicit-def $rax - TEST32rr killed renamable $r13d, renamable $r13d, implicit-def $eflags - JCC_1 %bb.6, 9, implicit $eflags - - bb.2: - successors: %bb.3(0x80000000) - liveins: $rax, $rbx, $r12, $r15, $r14d - MOV64mr $rbp, 1, $noreg, -48, $noreg, killed renamable $rax :: (store (s64) into %stack.0) + MOV64mr $rbp, 1, $noreg, -40, $noreg, killed renamable $r11 :: (store (s64) into %stack.1) renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags - - bb.3: - successors: %bb.4(0x04000000), %bb.3(0x7c000000) - liveins: $eax, $rbx, $r12, $r15, $r14d - $r13d = MOV32rr killed $eax $rdi = MOV64rr $r15 CALL64r renamable $r12, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp - dead renamable $eax = MOV32rm renamable $rbx, 1, $noreg, 0, $noreg :: (volatile load (s32) from %ir.ref.tmp5) + dead renamable $eax = MOV32rm renamable $rbx, 1, $noreg, 0, $noreg renamable $eax = MOV32ri 1 TEST8ri renamable $r14b, 1, implicit-def $eflags - JCC_1 %bb.3, 4, implicit $eflags - - bb.4: - successors: %bb.5(0x40000000), %bb.1(0x40000000) - liveins: $eflags, $rbx, $r12, $r15, $r13d, $r14d - - JCC_1 %bb.1, 4, implicit $eflags - - bb.5: - successors: %bb.1(0x80000000) - liveins: $rbx, $r12, $r15, $r13d, $r14d - - renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $rax - MOV64mi32 $rbp, 1, $noreg, -48, $noreg, 0 :: (store (s64) into %stack.0) - CALL64r killed renamable $rax, csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp, implicit-def dead $eax - JMP_1 %bb.1 - - bb.6: - $rsp = frame-destroy ADD64ri32 $rsp, 8, implicit-def dead $eflags - $rbx = frame-destroy POP64r implicit-def $rsp, implicit $rsp - $r12 = frame-destroy POP64r implicit-def $rsp, implicit $rsp - $r13 = frame-destroy POP64r implicit-def $rsp, implicit $rsp - $r14 = frame-destroy POP64r implicit-def $rsp, implicit $rsp - $r15 = frame-destroy POP64r implicit-def $rsp, implicit $rsp - $rbp = frame-destroy POP64r implicit-def $rsp, implicit $rsp - frame-destroy CFI_INSTRUCTION def_cfa $rsp, 8 + renamable $rax = MOV64rm $rbp, 1, $noreg, -48, $noreg :: (load (s64) from %stack.0) + FAKE_USE renamable $eax, implicit killed $rax + renamable $r11 = MOV64rm $rbp, 1, $noreg, -40, $noreg :: (load (s64) from %stack.1) + renamable $r12d = XOR32rr $r12d, $r11d, implicit-def dead $eflags + FAKE_USE killed renamable $r11d + TEST32rr killed renamable $r13d, renamable $r13d, implicit-def $eflags RET64 ... From c9d1b6e2ef0c81becbe2c33a70bd91e0dc54162a Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Wed, 16 Oct 2024 00:41:24 +0100 Subject: [PATCH 04/10] Use 'readsRegister' --- llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp index 74ad2fe3858ea..03bc8b33745ef 100644 --- a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp +++ b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp @@ -124,7 +124,7 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { SmallDenseSet FakeUsesToDelete; SmallVector RemainingFakeUses; for (MachineInstr *&FakeUse : reverse(RegFakeUses)) { - if (TRI->regsOverlap(Reg, FakeUse->getOperand(0).getReg())) { + if (FakeUse->readsRegister(Reg, TRI)) { FakeUsesToDelete.insert(FakeUse); RegFakeUses.erase(&FakeUse); } @@ -159,7 +159,7 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { // because any such FAKE_USE encountered prior is no longer relevant // for later encountered loads. for (MachineInstr *&FakeUse : reverse(RegFakeUses)) - if (!TRI->regsOverlap(Reg, FakeUse->getOperand(0).getReg())) + if (FakeUse->readsRegister(Reg, TRI)) RegFakeUses.erase(&FakeUse); } } From 52b95c707b5a48d53a506080e608da5879a58e09 Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Wed, 4 Dec 2024 17:46:23 +0000 Subject: [PATCH 05/10] Update test to check full output --- .../CodeGen/X86/fake-use-remove-loads.mir | 67 ++++++++++++------- 1 file changed, 43 insertions(+), 24 deletions(-) diff --git a/llvm/test/CodeGen/X86/fake-use-remove-loads.mir b/llvm/test/CodeGen/X86/fake-use-remove-loads.mir index d82434d1b72cf..4283552a20740 100644 --- a/llvm/test/CodeGen/X86/fake-use-remove-loads.mir +++ b/llvm/test/CodeGen/X86/fake-use-remove-loads.mir @@ -1,27 +1,17 @@ +# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5 # Ensure that loads into FAKE_USEs are correctly removed by the # remove-loads-into-fake-uses pass. -# RUN: llc -run-pass remove-loads-into-fake-uses -mtriple=x86_64-unknown-linux -debug-only=remove-loads-into-fake-uses 2>&1 -o - %s | FileCheck %s --implicit-check-not=DELETING +# RUN: llc -run-pass remove-loads-into-fake-uses -mtriple=x86_64-unknown-linux -debug-only=remove-loads-into-fake-uses 2>&1 -o - %s | FileCheck %s # REQUIRES: asserts # -## We ensure that the load into the FAKE_USE is removed, along with the FAKE_USE -## itself, even when the FAKE_USE is for a subregister of the move, and that we -## correctly handle situations where FAKE_USE has additional `killed` operands -## added by other passes. - -# CHECK: DELETING: renamable $rax = MOV64rm $rbp -# CHECK: DELETING: FAKE_USE renamable $eax - -## Also verify that the store to the stack slot still exists. - -# CHECK-LABEL: bb.0: -# CHECK: MOV64mr $rbp, 1, $noreg, -48, $noreg, killed renamable $rax :: (store (s64) into %stack.0) - -## Finally, verify that when the register has a use between the restore and the -## FAKE_USE, we do not delete the load or fake use. - -# CHECK: renamable $r11 = MOV64rm $rbp, 1, $noreg, -40, $noreg :: (load (s64) from %stack.1) -# CHECK: renamable $r12d = XOR32rr $r12d, $r11d, implicit-def dead $eflags -# CHECK: FAKE_USE killed renamable $r11d +## We verify that: +## - The load into the FAKE_USE is removed, along with the FAKE_USE itself, +## even when the FAKE_USE is for a subregister of the move. +## - We correctly handle situations where FAKE_USE has additional `killed` +## operands added by other passes. +## - The store to the stack slot still exists. +## - When the register has a use between the restore and the FAKE_USE, we do +## not delete the load or fake use. --- @@ -39,16 +29,45 @@ liveins: frameInfo: isCalleeSavedInfoValid: true stack: - - { id: 0, name: '', type: spill-slot, offset: -8, size: 8, alignment: 8, - stack-id: default, callee-saved-register: '', callee-saved-restored: true, + - { id: 0, name: '', type: spill-slot, offset: -8, size: 8, alignment: 8, + stack-id: default, callee-saved-register: '', callee-saved-restored: true, debug-info-variable: '', debug-info-expression: '', debug-info-location: '' } - - { id: 1, name: '', type: spill-slot, offset: -16, size: 8, alignment: 8, - stack-id: default, callee-saved-register: '', callee-saved-restored: true, + - { id: 1, name: '', type: spill-slot, offset: -16, size: 8, alignment: 8, + stack-id: default, callee-saved-register: '', callee-saved-restored: true, debug-info-variable: '', debug-info-expression: '', debug-info-location: '' } body: | bb.0: liveins: $esi, $rdi, $rdx, $r15, $r14, $r13, $r12, $r11, $rbx + ; CHECK-LABEL: name: _ZN1g1jEv + ; CHECK: liveins: $esi, $rdi, $rdx, $r15, $r14, $r13, $r12, $r11, $rbx + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: $rbx = MOV64rr $rdx + ; CHECK-NEXT: $r14d = MOV32rr $esi + ; CHECK-NEXT: $r15 = MOV64rr $rdi + ; CHECK-NEXT: renamable $r12d = XOR32rr undef $r12d, undef $r12d, implicit-def dead $eflags, implicit-def $r12 + ; CHECK-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $rax + + ;; The store to the stack slot is still present. + ; CHECK-NEXT: MOV64mr $rbp, 1, $noreg, -48, $noreg, killed renamable $rax :: (store (s64) into %stack.0) + + ; CHECK-NEXT: MOV64mr $rbp, 1, $noreg, -40, $noreg, killed renamable $r11 :: (store (s64) into %stack.1) + ; CHECK-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags + ; CHECK-NEXT: $r13d = MOV32rr killed $eax + ; CHECK-NEXT: $rdi = MOV64rr $r15 + ; CHECK-NEXT: CALL64r renamable $r12, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp + ; CHECK-NEXT: dead renamable $eax = MOV32rm renamable $rbx, 1, $noreg, 0, $noreg + ; CHECK-NEXT: renamable $eax = MOV32ri 1 + ; CHECK-NEXT: TEST8ri renamable $r14b, 1, implicit-def $eflags + + ;; First FAKE_USE and its corresponding load are removed; second FAKE_USE of + ;; a restored value that is also used is preserved. + ; CHECK-NEXT: renamable $r11 = MOV64rm $rbp, 1, $noreg, -40, $noreg :: (load (s64) from %stack.1) + ; CHECK-NEXT: renamable $r12d = XOR32rr $r12d, $r11d, implicit-def dead $eflags + ; CHECK-NEXT: FAKE_USE killed renamable $r11d + + ; CHECK-NEXT: TEST32rr killed renamable $r13d, renamable $r13d, implicit-def $eflags + ; CHECK-NEXT: RET64 $rbx = MOV64rr $rdx $r14d = MOV32rr $esi $r15 = MOV64rr $rdi From 6aadd23fb160255e25a8c0e7a466cd6ffd068788 Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Thu, 5 Dec 2024 09:37:08 +0000 Subject: [PATCH 06/10] Turn off pass when using varloc-based LDV --- llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp index 03bc8b33745ef..df7aa9f03d6ea 100644 --- a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp +++ b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp @@ -32,6 +32,9 @@ #include "llvm/IR/Function.h" #include "llvm/InitializePasses.h" #include "llvm/Support/Debug.h" +#include "llvm/Target/TargetMachine.h" + +#include "LiveDebugValues/LiveDebugValues.h" using namespace llvm; @@ -74,6 +77,10 @@ INITIALIZE_PASS_END(RemoveLoadsIntoFakeUses, DEBUG_TYPE, "Remove Loads Into Fake Uses", false, false) bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { + // Skip this pass if we would use VarLoc-based LDV, as there may be DBG_VALUE + // instructions of the restored values that would become invalid. + if (debuginfoShouldUseDebugInstrRef(MF.getTarget().getTargetTriple())) + return false; // Only run this for functions that have fake uses. if (!MF.hasFakeUses() || skipFunction(MF.getFunction())) return false; From a5fd7ee3b84bd3f6b277731bd76203a7ba34734e Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Fri, 13 Dec 2024 10:26:17 +0000 Subject: [PATCH 07/10] Remove unnecessary isReg check --- llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp index df7aa9f03d6ea..03f0a7243d00b 100644 --- a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp +++ b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp @@ -104,11 +104,9 @@ bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { if (MI.isFakeUse()) { if (MI.getNumOperands() == 0 || !MI.getOperand(0).isReg()) continue; - const MachineOperand &FakeUseOp = MI.getOperand(0); // Track the Fake Uses that use these register units so that we can // delete them if we delete the corresponding load. - if (FakeUseOp.isReg()) - RegFakeUses.push_back(&MI); + RegFakeUses.push_back(&MI); // Do not record FAKE_USE uses in LivePhysRegs so that we can recognize // otherwise-unused loads. continue; From 0d4a75b411c26531aa84d37ed39a0e4f5eebc784 Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Fri, 24 Jan 2025 18:14:46 +0000 Subject: [PATCH 08/10] Fix check for instr-ref enabled, update test to check disabled case --- llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp | 2 +- .../CodeGen/X86/fake-use-remove-loads.mir | 80 +++++++++++++------ 2 files changed, 57 insertions(+), 25 deletions(-) diff --git a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp index 03f0a7243d00b..1bed82cff5ac3 100644 --- a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp +++ b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp @@ -79,7 +79,7 @@ INITIALIZE_PASS_END(RemoveLoadsIntoFakeUses, DEBUG_TYPE, bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { // Skip this pass if we would use VarLoc-based LDV, as there may be DBG_VALUE // instructions of the restored values that would become invalid. - if (debuginfoShouldUseDebugInstrRef(MF.getTarget().getTargetTriple())) + if (!MF.shouldUseDebugInstrRef()) return false; // Only run this for functions that have fake uses. if (!MF.hasFakeUses() || skipFunction(MF.getFunction())) diff --git a/llvm/test/CodeGen/X86/fake-use-remove-loads.mir b/llvm/test/CodeGen/X86/fake-use-remove-loads.mir index 4283552a20740..fa61bfd60bfca 100644 --- a/llvm/test/CodeGen/X86/fake-use-remove-loads.mir +++ b/llvm/test/CodeGen/X86/fake-use-remove-loads.mir @@ -1,7 +1,8 @@ # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5 # Ensure that loads into FAKE_USEs are correctly removed by the # remove-loads-into-fake-uses pass. -# RUN: llc -run-pass remove-loads-into-fake-uses -mtriple=x86_64-unknown-linux -debug-only=remove-loads-into-fake-uses 2>&1 -o - %s | FileCheck %s +# RUN: llc -run-pass remove-loads-into-fake-uses -mtriple=x86_64-unknown-linux -debug-only=remove-loads-into-fake-uses 2>&1 -o - %s | FileCheck %s --check-prefix=ENABLED +# RUN: llc -run-pass remove-loads-into-fake-uses -mtriple=x86_64-unknown-linux -debug-only=remove-loads-into-fake-uses -experimental-debug-variable-locations=false 2>&1 -o - %s | FileCheck %s --check-prefixes=DISABLED # REQUIRES: asserts # ## We verify that: @@ -39,35 +40,66 @@ body: | bb.0: liveins: $esi, $rdi, $rdx, $r15, $r14, $r13, $r12, $r11, $rbx - ; CHECK-LABEL: name: _ZN1g1jEv - ; CHECK: liveins: $esi, $rdi, $rdx, $r15, $r14, $r13, $r12, $r11, $rbx - ; CHECK-NEXT: {{ $}} - ; CHECK-NEXT: $rbx = MOV64rr $rdx - ; CHECK-NEXT: $r14d = MOV32rr $esi - ; CHECK-NEXT: $r15 = MOV64rr $rdi - ; CHECK-NEXT: renamable $r12d = XOR32rr undef $r12d, undef $r12d, implicit-def dead $eflags, implicit-def $r12 - ; CHECK-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $rax + + + + + ; ENABLED-LABEL: name: _ZN1g1jEv + ; ENABLED: liveins: $esi, $rdi, $rdx, $r15, $r14, $r13, $r12, $r11, $rbx + ; ENABLED-NEXT: {{ $}} + ; ENABLED-NEXT: $rbx = MOV64rr $rdx + ; ENABLED-NEXT: $r14d = MOV32rr $esi + ; ENABLED-NEXT: $r15 = MOV64rr $rdi + ; ENABLED-NEXT: renamable $r12d = XOR32rr undef $r12d, undef $r12d, implicit-def dead $eflags, implicit-def $r12 + ; ENABLED-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $rax ;; The store to the stack slot is still present. - ; CHECK-NEXT: MOV64mr $rbp, 1, $noreg, -48, $noreg, killed renamable $rax :: (store (s64) into %stack.0) - - ; CHECK-NEXT: MOV64mr $rbp, 1, $noreg, -40, $noreg, killed renamable $r11 :: (store (s64) into %stack.1) - ; CHECK-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags - ; CHECK-NEXT: $r13d = MOV32rr killed $eax - ; CHECK-NEXT: $rdi = MOV64rr $r15 - ; CHECK-NEXT: CALL64r renamable $r12, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp - ; CHECK-NEXT: dead renamable $eax = MOV32rm renamable $rbx, 1, $noreg, 0, $noreg - ; CHECK-NEXT: renamable $eax = MOV32ri 1 - ; CHECK-NEXT: TEST8ri renamable $r14b, 1, implicit-def $eflags + ; ENABLED-NEXT: MOV64mr $rbp, 1, $noreg, -48, $noreg, killed renamable $rax :: (store (s64) into %stack.0) + + ; ENABLED-NEXT: MOV64mr $rbp, 1, $noreg, -40, $noreg, killed renamable $r11 :: (store (s64) into %stack.1) + ; ENABLED-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags + ; ENABLED-NEXT: $r13d = MOV32rr killed $eax + ; ENABLED-NEXT: $rdi = MOV64rr $r15 + ; ENABLED-NEXT: CALL64r renamable $r12, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp + ; ENABLED-NEXT: dead renamable $eax = MOV32rm renamable $rbx, 1, $noreg, 0, $noreg + ; ENABLED-NEXT: renamable $eax = MOV32ri 1 + ; ENABLED-NEXT: TEST8ri renamable $r14b, 1, implicit-def $eflags ;; First FAKE_USE and its corresponding load are removed; second FAKE_USE of ;; a restored value that is also used is preserved. - ; CHECK-NEXT: renamable $r11 = MOV64rm $rbp, 1, $noreg, -40, $noreg :: (load (s64) from %stack.1) - ; CHECK-NEXT: renamable $r12d = XOR32rr $r12d, $r11d, implicit-def dead $eflags - ; CHECK-NEXT: FAKE_USE killed renamable $r11d + ; ENABLED-NEXT: renamable $r11 = MOV64rm $rbp, 1, $noreg, -40, $noreg :: (load (s64) from %stack.1) + ; ENABLED-NEXT: renamable $r12d = XOR32rr $r12d, $r11d, implicit-def dead $eflags + ; ENABLED-NEXT: FAKE_USE killed renamable $r11d + + ; ENABLED-NEXT: TEST32rr killed renamable $r13d, renamable $r13d, implicit-def $eflags + ; ENABLED-NEXT: RET64 + ; + ; DISABLED-LABEL: name: _ZN1g1jEv + ; DISABLED: liveins: $esi, $rdi, $rdx, $r15, $r14, $r13, $r12, $r11, $rbx + ; DISABLED-NEXT: {{ $}} + ; DISABLED-NEXT: $rbx = MOV64rr $rdx + ; DISABLED-NEXT: $r14d = MOV32rr $esi + ; DISABLED-NEXT: $r15 = MOV64rr $rdi + ; DISABLED-NEXT: renamable $r12d = XOR32rr undef $r12d, undef $r12d, implicit-def dead $eflags, implicit-def $r12 + ; DISABLED-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $rax + ; DISABLED-NEXT: MOV64mr $rbp, 1, $noreg, -48, $noreg, killed renamable $rax :: (store (s64) into %stack.0) + ; DISABLED-NEXT: MOV64mr $rbp, 1, $noreg, -40, $noreg, killed renamable $r11 :: (store (s64) into %stack.1) + ; DISABLED-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags + ; DISABLED-NEXT: $r13d = MOV32rr killed $eax + ; DISABLED-NEXT: $rdi = MOV64rr $r15 + ; DISABLED-NEXT: CALL64r renamable $r12, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp + ; DISABLED-NEXT: dead renamable $eax = MOV32rm renamable $rbx, 1, $noreg, 0, $noreg + ; DISABLED-NEXT: renamable $eax = MOV32ri 1 + ; DISABLED-NEXT: TEST8ri renamable $r14b, 1, implicit-def $eflags - ; CHECK-NEXT: TEST32rr killed renamable $r13d, renamable $r13d, implicit-def $eflags - ; CHECK-NEXT: RET64 + ;; Verify that when instr-ref is disabled, we do not remove fake uses. + ; DISABLED-NEXT: renamable $rax = MOV64rm $rbp, 1, $noreg, -48, $noreg :: (load (s64) from %stack.0) + ; DISABLED-NEXT: FAKE_USE renamable $eax, implicit killed $rax + ; DISABLED-NEXT: renamable $r11 = MOV64rm $rbp, 1, $noreg, -40, $noreg :: (load (s64) from %stack.1) + ; DISABLED-NEXT: renamable $r12d = XOR32rr $r12d, $r11d, implicit-def dead $eflags + ; DISABLED-NEXT: FAKE_USE killed renamable $r11d + ; DISABLED-NEXT: TEST32rr killed renamable $r13d, renamable $r13d, implicit-def $eflags + ; DISABLED-NEXT: RET64 $rbx = MOV64rr $rdx $r14d = MOV32rr $esi $r15 = MOV64rr $rdi From 026fc06c4c42798d9f4a4c5fe5285e083a7e906e Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Tue, 28 Jan 2025 12:37:43 +0000 Subject: [PATCH 09/10] Use instr-ref bit instead of high-level function --- llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp | 4 +--- llvm/test/CodeGen/X86/fake-use-remove-loads.mir | 8 +++++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp index 1bed82cff5ac3..384a049acfe34 100644 --- a/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp +++ b/llvm/lib/CodeGen/RemoveLoadsIntoFakeUses.cpp @@ -34,8 +34,6 @@ #include "llvm/Support/Debug.h" #include "llvm/Target/TargetMachine.h" -#include "LiveDebugValues/LiveDebugValues.h" - using namespace llvm; #define DEBUG_TYPE "remove-loads-into-fake-uses" @@ -79,7 +77,7 @@ INITIALIZE_PASS_END(RemoveLoadsIntoFakeUses, DEBUG_TYPE, bool RemoveLoadsIntoFakeUses::runOnMachineFunction(MachineFunction &MF) { // Skip this pass if we would use VarLoc-based LDV, as there may be DBG_VALUE // instructions of the restored values that would become invalid. - if (!MF.shouldUseDebugInstrRef()) + if (!MF.useDebugInstrRef()) return false; // Only run this for functions that have fake uses. if (!MF.hasFakeUses() || skipFunction(MF.getFunction())) diff --git a/llvm/test/CodeGen/X86/fake-use-remove-loads.mir b/llvm/test/CodeGen/X86/fake-use-remove-loads.mir index fa61bfd60bfca..27151de01d4a4 100644 --- a/llvm/test/CodeGen/X86/fake-use-remove-loads.mir +++ b/llvm/test/CodeGen/X86/fake-use-remove-loads.mir @@ -1,8 +1,9 @@ # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5 # Ensure that loads into FAKE_USEs are correctly removed by the -# remove-loads-into-fake-uses pass. -# RUN: llc -run-pass remove-loads-into-fake-uses -mtriple=x86_64-unknown-linux -debug-only=remove-loads-into-fake-uses 2>&1 -o - %s | FileCheck %s --check-prefix=ENABLED -# RUN: llc -run-pass remove-loads-into-fake-uses -mtriple=x86_64-unknown-linux -debug-only=remove-loads-into-fake-uses -experimental-debug-variable-locations=false 2>&1 -o - %s | FileCheck %s --check-prefixes=DISABLED +# remove-loads-into-fake-uses pass, and that if the function does not use +# instruction referencing then no changes are made. +# RUN: sed 's/USE_INSTR_REF/true/' %s | llc - -x mir -run-pass remove-loads-into-fake-uses -mtriple=x86_64-unknown-linux -debug-only=remove-loads-into-fake-uses 2>&1 -o - | FileCheck %s --check-prefix=ENABLED +# RUN: sed 's/USE_INSTR_REF/false/' %s | llc - -x mir -run-pass remove-loads-into-fake-uses -mtriple=x86_64-unknown-linux -debug-only=remove-loads-into-fake-uses 2>&1 -o - | FileCheck %s --check-prefixes=DISABLED # REQUIRES: asserts # ## We verify that: @@ -22,6 +23,7 @@ tracksRegLiveness: true noPhis: true noVRegs: true hasFakeUses: true +debugInstrRef: USE_INSTR_REF tracksDebugUserValues: true liveins: - { reg: '$rdi', virtual-reg: '' } From 92c0631ce61b4ed98d454fdd6bc2b440c05c831d Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Tue, 28 Jan 2025 12:52:39 +0000 Subject: [PATCH 10/10] Update test to be more update_test_checks-friendly --- .../CodeGen/X86/fake-use-remove-loads.mir | 154 +++++++++++------- 1 file changed, 99 insertions(+), 55 deletions(-) diff --git a/llvm/test/CodeGen/X86/fake-use-remove-loads.mir b/llvm/test/CodeGen/X86/fake-use-remove-loads.mir index 27151de01d4a4..3f67f03c9a63d 100644 --- a/llvm/test/CodeGen/X86/fake-use-remove-loads.mir +++ b/llvm/test/CodeGen/X86/fake-use-remove-loads.mir @@ -2,8 +2,7 @@ # Ensure that loads into FAKE_USEs are correctly removed by the # remove-loads-into-fake-uses pass, and that if the function does not use # instruction referencing then no changes are made. -# RUN: sed 's/USE_INSTR_REF/true/' %s | llc - -x mir -run-pass remove-loads-into-fake-uses -mtriple=x86_64-unknown-linux -debug-only=remove-loads-into-fake-uses 2>&1 -o - | FileCheck %s --check-prefix=ENABLED -# RUN: sed 's/USE_INSTR_REF/false/' %s | llc - -x mir -run-pass remove-loads-into-fake-uses -mtriple=x86_64-unknown-linux -debug-only=remove-loads-into-fake-uses 2>&1 -o - | FileCheck %s --check-prefixes=DISABLED +# RUN: llc %s -run-pass remove-loads-into-fake-uses -mtriple=x86_64-unknown-linux -debug-only=remove-loads-into-fake-uses 2>&1 -o - | FileCheck %s # REQUIRES: asserts # ## We verify that: @@ -17,14 +16,14 @@ --- -name: _ZN1g1jEv +name: enabled alignment: 16 tracksRegLiveness: true noPhis: true noVRegs: true hasFakeUses: true -debugInstrRef: USE_INSTR_REF tracksDebugUserValues: true +debugInstrRef: true liveins: - { reg: '$rdi', virtual-reg: '' } - { reg: '$esi', virtual-reg: '' } @@ -42,66 +41,111 @@ body: | bb.0: liveins: $esi, $rdi, $rdx, $r15, $r14, $r13, $r12, $r11, $rbx - - - - - ; ENABLED-LABEL: name: _ZN1g1jEv - ; ENABLED: liveins: $esi, $rdi, $rdx, $r15, $r14, $r13, $r12, $r11, $rbx - ; ENABLED-NEXT: {{ $}} - ; ENABLED-NEXT: $rbx = MOV64rr $rdx - ; ENABLED-NEXT: $r14d = MOV32rr $esi - ; ENABLED-NEXT: $r15 = MOV64rr $rdi - ; ENABLED-NEXT: renamable $r12d = XOR32rr undef $r12d, undef $r12d, implicit-def dead $eflags, implicit-def $r12 - ; ENABLED-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $rax + ; CHECK-LABEL: name: enabled + ; CHECK: liveins: $esi, $rdi, $rdx, $r15, $r14, $r13, $r12, $r11, $rbx + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: $rbx = MOV64rr $rdx + ; CHECK-NEXT: $r14d = MOV32rr $esi + ; CHECK-NEXT: $r15 = MOV64rr $rdi + ; CHECK-NEXT: renamable $r12d = XOR32rr undef $r12d, undef $r12d, implicit-def dead $eflags, implicit-def $r12 + ; CHECK-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $rax ;; The store to the stack slot is still present. - ; ENABLED-NEXT: MOV64mr $rbp, 1, $noreg, -48, $noreg, killed renamable $rax :: (store (s64) into %stack.0) + ; CHECK-NEXT: MOV64mr $rbp, 1, $noreg, -48, $noreg, killed renamable $rax :: (store (s64) into %stack.0) - ; ENABLED-NEXT: MOV64mr $rbp, 1, $noreg, -40, $noreg, killed renamable $r11 :: (store (s64) into %stack.1) - ; ENABLED-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags - ; ENABLED-NEXT: $r13d = MOV32rr killed $eax - ; ENABLED-NEXT: $rdi = MOV64rr $r15 - ; ENABLED-NEXT: CALL64r renamable $r12, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp - ; ENABLED-NEXT: dead renamable $eax = MOV32rm renamable $rbx, 1, $noreg, 0, $noreg - ; ENABLED-NEXT: renamable $eax = MOV32ri 1 - ; ENABLED-NEXT: TEST8ri renamable $r14b, 1, implicit-def $eflags + ; CHECK-NEXT: MOV64mr $rbp, 1, $noreg, -40, $noreg, killed renamable $r11 :: (store (s64) into %stack.1) + ; CHECK-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags + ; CHECK-NEXT: $r13d = MOV32rr killed $eax + ; CHECK-NEXT: $rdi = MOV64rr $r15 + ; CHECK-NEXT: CALL64r renamable $r12, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp + ; CHECK-NEXT: dead renamable $eax = MOV32rm renamable $rbx, 1, $noreg, 0, $noreg + ; CHECK-NEXT: renamable $eax = MOV32ri 1 + ; CHECK-NEXT: TEST8ri renamable $r14b, 1, implicit-def $eflags ;; First FAKE_USE and its corresponding load are removed; second FAKE_USE of ;; a restored value that is also used is preserved. - ; ENABLED-NEXT: renamable $r11 = MOV64rm $rbp, 1, $noreg, -40, $noreg :: (load (s64) from %stack.1) - ; ENABLED-NEXT: renamable $r12d = XOR32rr $r12d, $r11d, implicit-def dead $eflags - ; ENABLED-NEXT: FAKE_USE killed renamable $r11d + ; CHECK-NEXT: renamable $r11 = MOV64rm $rbp, 1, $noreg, -40, $noreg :: (load (s64) from %stack.1) + ; CHECK-NEXT: renamable $r12d = XOR32rr $r12d, $r11d, implicit-def dead $eflags + ; CHECK-NEXT: FAKE_USE killed renamable $r11d + + ; CHECK-NEXT: TEST32rr killed renamable $r13d, renamable $r13d, implicit-def $eflags + ; CHECK-NEXT: RET64 + + $rbx = MOV64rr $rdx + $r14d = MOV32rr $esi + $r15 = MOV64rr $rdi + renamable $r12d = XOR32rr undef $r12d, undef $r12d, implicit-def dead $eflags, implicit-def $r12 + renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $rax + MOV64mr $rbp, 1, $noreg, -48, $noreg, killed renamable $rax :: (store (s64) into %stack.0) + MOV64mr $rbp, 1, $noreg, -40, $noreg, killed renamable $r11 :: (store (s64) into %stack.1) + renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags + $r13d = MOV32rr killed $eax + $rdi = MOV64rr $r15 + CALL64r renamable $r12, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp + dead renamable $eax = MOV32rm renamable $rbx, 1, $noreg, 0, $noreg + renamable $eax = MOV32ri 1 + TEST8ri renamable $r14b, 1, implicit-def $eflags + renamable $rax = MOV64rm $rbp, 1, $noreg, -48, $noreg :: (load (s64) from %stack.0) + FAKE_USE renamable $eax, implicit killed $rax + renamable $r11 = MOV64rm $rbp, 1, $noreg, -40, $noreg :: (load (s64) from %stack.1) + renamable $r12d = XOR32rr $r12d, $r11d, implicit-def dead $eflags + FAKE_USE killed renamable $r11d + TEST32rr killed renamable $r13d, renamable $r13d, implicit-def $eflags + RET64 + +... +--- +name: disabled +alignment: 16 +tracksRegLiveness: true +noPhis: true +noVRegs: true +hasFakeUses: true +tracksDebugUserValues: true +debugInstrRef: false +liveins: + - { reg: '$rdi', virtual-reg: '' } + - { reg: '$esi', virtual-reg: '' } + - { reg: '$rdx', virtual-reg: '' } +frameInfo: + isCalleeSavedInfoValid: true +stack: + - { id: 0, name: '', type: spill-slot, offset: -8, size: 8, alignment: 8, + stack-id: default, callee-saved-register: '', callee-saved-restored: true, + debug-info-variable: '', debug-info-expression: '', debug-info-location: '' } + - { id: 1, name: '', type: spill-slot, offset: -16, size: 8, alignment: 8, + stack-id: default, callee-saved-register: '', callee-saved-restored: true, + debug-info-variable: '', debug-info-expression: '', debug-info-location: '' } +body: | + bb.0: + liveins: $esi, $rdi, $rdx, $r15, $r14, $r13, $r12, $r11, $rbx - ; ENABLED-NEXT: TEST32rr killed renamable $r13d, renamable $r13d, implicit-def $eflags - ; ENABLED-NEXT: RET64 - ; - ; DISABLED-LABEL: name: _ZN1g1jEv - ; DISABLED: liveins: $esi, $rdi, $rdx, $r15, $r14, $r13, $r12, $r11, $rbx - ; DISABLED-NEXT: {{ $}} - ; DISABLED-NEXT: $rbx = MOV64rr $rdx - ; DISABLED-NEXT: $r14d = MOV32rr $esi - ; DISABLED-NEXT: $r15 = MOV64rr $rdi - ; DISABLED-NEXT: renamable $r12d = XOR32rr undef $r12d, undef $r12d, implicit-def dead $eflags, implicit-def $r12 - ; DISABLED-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $rax - ; DISABLED-NEXT: MOV64mr $rbp, 1, $noreg, -48, $noreg, killed renamable $rax :: (store (s64) into %stack.0) - ; DISABLED-NEXT: MOV64mr $rbp, 1, $noreg, -40, $noreg, killed renamable $r11 :: (store (s64) into %stack.1) - ; DISABLED-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags - ; DISABLED-NEXT: $r13d = MOV32rr killed $eax - ; DISABLED-NEXT: $rdi = MOV64rr $r15 - ; DISABLED-NEXT: CALL64r renamable $r12, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp - ; DISABLED-NEXT: dead renamable $eax = MOV32rm renamable $rbx, 1, $noreg, 0, $noreg - ; DISABLED-NEXT: renamable $eax = MOV32ri 1 - ; DISABLED-NEXT: TEST8ri renamable $r14b, 1, implicit-def $eflags + ; CHECK-LABEL: name: disabled + ; CHECK: liveins: $esi, $rdi, $rdx, $r15, $r14, $r13, $r12, $r11, $rbx + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: $rbx = MOV64rr $rdx + ; CHECK-NEXT: $r14d = MOV32rr $esi + ; CHECK-NEXT: $r15 = MOV64rr $rdi + ; CHECK-NEXT: renamable $r12d = XOR32rr undef $r12d, undef $r12d, implicit-def dead $eflags, implicit-def $r12 + ; CHECK-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags, implicit-def $rax + ; CHECK-NEXT: MOV64mr $rbp, 1, $noreg, -48, $noreg, killed renamable $rax :: (store (s64) into %stack.0) + ; CHECK-NEXT: MOV64mr $rbp, 1, $noreg, -40, $noreg, killed renamable $r11 :: (store (s64) into %stack.1) + ; CHECK-NEXT: renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags + ; CHECK-NEXT: $r13d = MOV32rr killed $eax + ; CHECK-NEXT: $rdi = MOV64rr $r15 + ; CHECK-NEXT: CALL64r renamable $r12, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp + ; CHECK-NEXT: dead renamable $eax = MOV32rm renamable $rbx, 1, $noreg, 0, $noreg + ; CHECK-NEXT: renamable $eax = MOV32ri 1 + ; CHECK-NEXT: TEST8ri renamable $r14b, 1, implicit-def $eflags ;; Verify that when instr-ref is disabled, we do not remove fake uses. - ; DISABLED-NEXT: renamable $rax = MOV64rm $rbp, 1, $noreg, -48, $noreg :: (load (s64) from %stack.0) - ; DISABLED-NEXT: FAKE_USE renamable $eax, implicit killed $rax - ; DISABLED-NEXT: renamable $r11 = MOV64rm $rbp, 1, $noreg, -40, $noreg :: (load (s64) from %stack.1) - ; DISABLED-NEXT: renamable $r12d = XOR32rr $r12d, $r11d, implicit-def dead $eflags - ; DISABLED-NEXT: FAKE_USE killed renamable $r11d - ; DISABLED-NEXT: TEST32rr killed renamable $r13d, renamable $r13d, implicit-def $eflags - ; DISABLED-NEXT: RET64 + ; CHECK-NEXT: renamable $rax = MOV64rm $rbp, 1, $noreg, -48, $noreg :: (load (s64) from %stack.0) + ; CHECK-NEXT: FAKE_USE renamable $eax, implicit killed $rax + ; CHECK-NEXT: renamable $r11 = MOV64rm $rbp, 1, $noreg, -40, $noreg :: (load (s64) from %stack.1) + ; CHECK-NEXT: renamable $r12d = XOR32rr $r12d, $r11d, implicit-def dead $eflags + ; CHECK-NEXT: FAKE_USE killed renamable $r11d + ; CHECK-NEXT: TEST32rr killed renamable $r13d, renamable $r13d, implicit-def $eflags + ; CHECK-NEXT: RET64 $rbx = MOV64rr $rdx $r14d = MOV32rr $esi $r15 = MOV64rr $rdi