Mark the last use of EFLAGS before the copy's def as a kill if the copy's def operand is itself a kill.

[AZero13]

No description provided.

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: AZero13 (AZero13)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/135726.diff

3 Files Affected:

• (modified) llvm/lib/Target/X86/X86FlagsCopyLowering.cpp (+25-3)
• (modified) llvm/test/CodeGen/X86/apx/flags-copy-lowering.mir (+1-1)
• (modified) llvm/test/CodeGen/X86/flags-copy-lowering.mir (+1-1)

diff --git a/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp b/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp index ca953d6008b27..4e1b81324afc7 100644 --- a/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp +++ b/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp @@ -418,6 +418,31 @@ bool X86FlagsCopyLoweringPass::runOnMachineFunction(MachineFunction &MF) { auto Cleanup = make_scope_exit([&] { // All uses of the EFLAGS copy are now rewritten, kill the copy into // eflags and if dead the copy from. + + // Mark the last use of EFLAGS before the copy's def as a kill if + // the copy's def operand is itself a kill. + if (CopyDefI.getOperand(1).isKill()) { + MachineBasicBlock *DefMBB = CopyDefI.getParent(); + MachineInstr *LastUse = nullptr; +  + // Find the last use of EFLAGS before CopyDefI + for (auto MI = std::prev(CopyDefI.getIterator());  + MI != DefMBB->begin(); --MI) { + if (MI->readsRegister(X86::EFLAGS, TRI)) { + LastUse = &*MI; + break; + } + } +  + // If we found a last use, mark it as kill + if (LastUse) { + MachineOperand *FlagUse =  + LastUse->findRegisterUseOperand(X86::EFLAGS, TRI); + if (FlagUse) + FlagUse->setIsKill(true); + } + } + CopyI->eraseFromParent(); if (MRI->use_empty(CopyDefI.getOperand(0).getReg())) CopyDefI.eraseFromParent(); @@ -687,9 +712,6 @@ bool X86FlagsCopyLoweringPass::runOnMachineFunction(MachineFunction &MF) { rewriteMI(*TestMBB, TestPos, TestLoc, *JmpI, CondRegs); } - - // FIXME: Mark the last use of EFLAGS before the copy's def as a kill if - // the copy's def operand is itself a kill. } #ifndef NDEBUG diff --git a/llvm/test/CodeGen/X86/apx/flags-copy-lowering.mir b/llvm/test/CodeGen/X86/apx/flags-copy-lowering.mir index 4002906795dc8..ab48243b0e4e1 100644 --- a/llvm/test/CodeGen/X86/apx/flags-copy-lowering.mir +++ b/llvm/test/CodeGen/X86/apx/flags-copy-lowering.mir @@ -50,7 +50,7 @@ body: | ; CHECK-NEXT: [[COPY:%[0-9]+]]:gr64 = COPY $rdi ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gr64 = COPY $rsi ; CHECK-NEXT: [[SUB64rr_ND:%[0-9]+]]:gr64 = SUB64rr_ND [[COPY]], [[COPY1]], implicit-def $eflags - ; CHECK-NEXT: [[SETCCr:%[0-9]+]]:gr8 = SETCCr 2, implicit $eflags + ; CHECK-NEXT: [[SETCCr:%[0-9]+]]:gr8 = SETCCr 2, implicit killed $eflags ; CHECK-NEXT: INLINEASM &nop, 1 /* sideeffect attdialect */, 12 /* clobber */, implicit-def dead $eflags ; CHECK-NEXT: dead [[ADD8ri_ND:%[0-9]+]]:gr8 = ADD8ri_ND [[SETCCr]], 255, implicit-def $eflags ; CHECK-NEXT: [[SBB64ri32_ND:%[0-9]+]]:gr64 = SBB64ri32_ND [[SUB64rr_ND]], 42, implicit-def $eflags, implicit killed $eflags diff --git a/llvm/test/CodeGen/X86/flags-copy-lowering.mir b/llvm/test/CodeGen/X86/flags-copy-lowering.mir index a08c63c285e15..69256cbd43337 100644 --- a/llvm/test/CodeGen/X86/flags-copy-lowering.mir +++ b/llvm/test/CodeGen/X86/flags-copy-lowering.mir @@ -277,7 +277,7 @@ body: | ; CHECK-NEXT: [[COPY:%[0-9]+]]:gr64 = COPY $rdi ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gr64 = COPY $rsi ; CHECK-NEXT: [[SUB64rr:%[0-9]+]]:gr64 = SUB64rr [[COPY]], [[COPY1]], implicit-def $eflags - ; CHECK-NEXT: [[SETCCr:%[0-9]+]]:gr8 = SETCCr 2, implicit $eflags + ; CHECK-NEXT: [[SETCCr:%[0-9]+]]:gr8 = SETCCr 2, implicit killed $eflags ; CHECK-NEXT: INLINEASM &nop, 1 /* sideeffect attdialect */, 12 /* clobber */, implicit-def dead $eflags ; CHECK-NEXT: dead [[ADD8ri:%[0-9]+]]:gr8 = ADD8ri [[SETCCr]], 255, implicit-def $eflags ; CHECK-NEXT: [[SBB64ri32_:%[0-9]+]]:gr64 = SBB64ri32 [[SUB64rr]], 42, implicit-def $eflags, implicit killed $eflags 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[topperc]

Is there a benefit to this other than removing a FIXME?

[phoebewang]

Why we need to find the last use? Can we set it killed during rewritten the instruction?