[llvm] r269115 - [X86] Properly check that EAX is dead when copying EFLAGS.

[Quentin Colombet via llvm-commits]

Author: qcolombet
Date: Tue May 10 15:49:46 2016
New Revision: 269115

URL: http://llvm.org/viewvc/llvm-project?rev=269115&view=rev
Log:
[X86] Properly check that EAX is dead when copying EFLAGS.

This fixes a bug introduced in r267623, where we got smarter and avoided to save
EAX before using it. However, we failed to check if any of the subregister of
EAX were alive and thus, missed cases where we have to save EAX before using it.

The problem may happen on every X86/i386/... platform.

This fixes llvm.org/PR27624

Added:
    llvm/trunk/test/CodeGen/X86/eflags-copy-expansion.mir
Modified:
    llvm/trunk/lib/Target/X86/X86InstrInfo.cpp

Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.cpp?rev=269115&r1=269114&r2=269115&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86InstrInfo.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86InstrInfo.cpp Tue May 10 15:49:46 2016
@@ -4527,8 +4527,9 @@ void X86InstrInfo::copyPhysReg(MachineBa
     // first frame index.
     // See X86ISelLowering.cpp - X86::hasCopyImplyingStackAdjustment.
 
+    const TargetRegisterInfo *TRI = &getRegisterInfo();
     MachineBasicBlock::LivenessQueryResult LQR =
-        MBB.computeRegisterLiveness(&getRegisterInfo(), AX, MI);
+        MBB.computeRegisterLiveness(TRI, AX, MI);
     // We do not want to save and restore AX if we do not have to.
     // Moreover, if we do so whereas AX is dead, we would need to set
     // an undef flag on the use of AX, otherwise the verifier will
@@ -4536,15 +4537,19 @@ void X86InstrInfo::copyPhysReg(MachineBa
     // We do not want to change the behavior of the machine verifier
     // as this is usually wrong to read an undef value.
     if (MachineBasicBlock::LQR_Unknown == LQR) {
-      LivePhysRegs LPR(&getRegisterInfo());
+      LivePhysRegs LPR(TRI);
       LPR.addLiveOuts(MBB);
       MachineBasicBlock::iterator I = MBB.end();
       while (I != MI) {
         --I;
         LPR.stepBackward(*I);
       }
-      LQR = LPR.contains(AX) ? MachineBasicBlock::LQR_Live
-                             : MachineBasicBlock::LQR_Dead;
+      // AX contains the top most register in the aliasing hierarchy.
+      // It may not be live, but one of its aliases may be.
+      for (MCRegAliasIterator AI(AX, TRI, true);
+           AI.isValid() && LQR != MachineBasicBlock::LQR_Live; ++AI)
+        LQR = LPR.contains(*AI) ? MachineBasicBlock::LQR_Live
+                                : MachineBasicBlock::LQR_Dead;
     }
     bool AXDead = (Reg == AX) || (MachineBasicBlock::LQR_Dead == LQR);
     if (!AXDead)

Added: llvm/trunk/test/CodeGen/X86/eflags-copy-expansion.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/eflags-copy-expansion.mir?rev=269115&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/eflags-copy-expansion.mir (added)
+++ llvm/trunk/test/CodeGen/X86/eflags-copy-expansion.mir Tue May 10 15:49:46 2016
@@ -0,0 +1,67 @@
+# RUN: llc -run-pass postrapseudos -mtriple=i386-apple-macosx -o /dev/null %s 2>&1 | FileCheck %s
+
+# Verify that we correctly save and restore eax when copying eflags,
+# even when only a smaller alias of eax is used. We used to check only
+# eax and not its aliases.
+# PR27624.
+
+--- |
+  target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+
+  define void @foo() {
+  entry:
+    br label %false
+  false:
+    ret void
+  }
+
+...
+
+---
+name:            foo
+allVRegsAllocated: true
+isSSA:           false
+tracksRegLiveness: true
+liveins:
+  - { reg: '%edi' }
+body:             |
+  bb.0.entry:
+    liveins: %edi
+    successors: %bb.1.false
+    NOOP implicit-def %al
+
+    ; The bug was triggered only when LivePhysReg is used, which
+    ; happens only when the heuristic for the liveness computation
+    ; failed. The liveness computation heuristic looks at 10 instructions
+    ; before and after the copy. Make sure we do not reach the definition of
+    ; AL in 10 instructions, otherwise the heuristic will see that it is live.
+    NOOP
+    NOOP
+    NOOP
+    NOOP
+    NOOP
+    NOOP
+    NOOP
+    NOOP
+    NOOP
+    NOOP
+    NOOP
+    NOOP
+    NOOP
+    ; Save AL.
+    ; CHECK: PUSH32r killed %eax
+
+    ; Copy EDI into EFLAGS
+    ; CHECK-NEXT: %eax = MOV32rr %edi
+    ; CHECK-NEXT: %al = ADD8ri %al, 127, implicit-def %eflags
+    ; CHECK-NEXT: SAHF implicit-def %eflags, implicit %ah
+    %eflags = COPY %edi
+
+    ; Restore AL.
+    ; CHECK-NEXT: %eax = POP32r
+  bb.1.false:
+    liveins: %al
+    NOOP implicit %al
+    RETQ
+
+...