[X86] Resolve FIXME: Copy kill flag for eflags

[AreaZR]

We now mark the last eflags usage as kill if the def was also kill.

[llvmbot]

@llvm/pr-subscribers-llvm-regalloc

@llvm/pr-subscribers-backend-x86

Author: AtariDreams (AtariDreams)

Changes

We now mark the last eflags usage as kill if the def was also kill.

---

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

1 Files Affected:

• (modified) llvm/lib/Target/X86/X86FlagsCopyLowering.cpp (+7-2)

diff --git a/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp b/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp
index af25b34fbab995..e9830f086668a0 100644
--- a/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp
+++ b/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp
@@ -683,6 +683,8 @@ bool X86FlagsCopyLoweringPass::runOnMachineFunction(MachineFunction &MF) {
     // Now rewrite the jumps that use the flags. These we handle specially
     // because if there are multiple jumps in a single basic block we'll have
     // to do surgery on the CFG.
+    bool CopyDefIsKill = CopyDefI.killsRegister(X86::EFLAGS);
+    MachineOperand *LastEflagsUse = nullptr;
     MachineBasicBlock *LastJmpMBB = nullptr;
     for (MachineInstr *JmpI : JmpIs) {
       // Past the first jump within a basic block we need to split the blocks
@@ -693,10 +695,13 @@ bool X86FlagsCopyLoweringPass::runOnMachineFunction(MachineFunction &MF) {
         LastJmpMBB = JmpI->getParent();
 
       rewriteCondJmp(*TestMBB, TestPos, TestLoc, *JmpI, CondRegs);
+      if (JmpI->readsRegister(X86::EFLAGS))
+        LastEflagsUse = JmpI->findRegisterUseOperand(X86::EFLAGS);
     }
 
-    // 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.
+    // After the loop that processes jumps:
+    if (LastEflagsUse && CopyDefIsKill)
+      LastEflagsUse->setIsKill(true);
   }
 
 #ifndef NDEBUG

[github-actions]

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

[topperc]

Can you write an MIR test for this?

[github-actions]

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

[RKSimon]

Do you think we should we consider altering the findRegisterDefOperandIdx/findRegisterDefOperand argument order and require the TRI pointer to help avoid this in the future?

[AreaZR]

Do you think we should we consider altering the findRegisterDefOperandIdx/findRegisterDefOperand argument order and require the TRI pointer to help avoid this in the future?

I do think this actually.

[RKSimon]

@AtariDreams any luck with adding test coverage?

[RKSimon]

Still needs test coverage - and the patch title/description needs updating to actually describe what that patch does and is for