X86InstrInfo::copyPhysReg: workaround reg liveness

[jfb]

Summary

computeRegisterLiveness and analyzePhysReg are currently getting
confused about liveness in some cases, breaking copyPhysReg's
calculation of whether AX is dead in some cases. Work around this issue
temporarily by assuming that AX is always live.

See detail in: https://llvm.org/bugs/show_bug.cgi?id=25033#c7
And associated bugs PR24535 PR25033 PR24991 PR24992 PR25201.

This workaround makes the code correct but slightly inefficient, but it
seems to confuse the machine instr verifier which now things EAX was
undefined in some cases where it's being conservatively saved /
restored.

[majnemer]

LGTM

[jfb]

• Fix temporary MI verif issue caused by D15157.

llvm/trunk/lib/Target/X86/X86InstrInfo.cpp

if ((FromEFLAGS || ToEFLAGS) && (is32 || is64)) {
     // Notice that we have to adjust the stack if we don't want to clobber the
     // first frame index. See X86FrameLowering.cpp - clobbersTheStack.
 
     int Mov = is64 ? X86::MOV64rr : X86::MOV32rr;
     int Push = is64 ? X86::PUSH64r : X86::PUSH32r;
     int Pop = is64 ? X86::POP64r : X86::POP32r;
     int AX = is64 ? X86::RAX : X86::EAX;
 
-    bool AXDead = (Reg == AX) ||
-                  (MachineBasicBlock::LQR_Dead ==
-                   MBB.computeRegisterLiveness(&getRegisterInfo(), AX, MI));
+    bool AXDead = (Reg == AX);
+    // FIXME: The above could figure out that AX is dead in more cases with:
+    //          || (MachineBasicBlock::LQR_Dead ==
+    //            MBB.computeRegisterLiveness(&getRegisterInfo(), AX, MI));
+    //
+    //        Unfortunately this is slightly broken, see PR24535 and the likely
+    //        related PR25033 PR24991 PR24992 PR25201. These issues seem to
+    //        showcase sub-register / super-register confusion: a previous kill
+    //        of AH but no kill of AL leads computeRegisterLiveness to
+    //        erroneously conclude that AX is dead.
+    //
+    //        Once fixed, also update cmpxchg-clobber-flags.ll and
+    //        peephole-na-phys-copy-folding.ll.
 
     if (!AXDead)
       BuildMI(MBB, MI, DL, get(Push)).addReg(AX, getKillRegState(true));
     if (FromEFLAGS) {
       BuildMI(MBB, MI, DL, get(X86::SETOr), X86::AL);
       BuildMI(MBB, MI, DL, get(X86::LAHF));
       BuildMI(MBB, MI, DL, get(Mov), Reg).addReg(AX);
     }

llvm/trunk/test/CodeGen/X86/cmpxchg-clobber-flags.ll

-; RUN: llc -verify-machineinstrs -mtriple=i386-linux-gnu %s -o - | FileCheck %s -check-prefix=i386
-; RUN: llc -verify-machineinstrs -mtriple=i386-linux-gnu -pre-RA-sched=fast %s -o - | FileCheck %s -check-prefix=i386f
-; RUN: llc -verify-machineinstrs -mtriple=x86_64-linux-gnu %s -o - | FileCheck %s -check-prefix=x8664
-; RUN: llc -verify-machineinstrs -mtriple=x86_64-linux-gnu -pre-RA-sched=fast %s -o - | FileCheck %s -check-prefix=x8664
+; RUN: llc -mtriple=i386-linux-gnu %s -o - | FileCheck %s -check-prefix=i386
+; RUN: llc -mtriple=i386-linux-gnu -pre-RA-sched=fast %s -o - | FileCheck %s -check-prefix=i386f
+; RUN: llc -mtriple=x86_64-linux-gnu %s -o - | FileCheck %s -check-prefix=x8664
+; RUN: llc -mtriple=x86_64-linux-gnu -pre-RA-sched=fast %s -o - | FileCheck %s -check-prefix=x8664
+
+; FIXME: X86InstrInfo::copyPhysReg had code which figured out whether AX was
+;        live or not to avoid save / restore when it's not needed. See FIXME in
+;        that function for more details on which the code is currently
+;        disabled. The extra push/pop are marked below and can be removed once
+;        the issue is fixed.
+;        -verify-machineinstrs should also be added back in the RUN lines above.
 
 declare i32 @foo()
 declare i32 @bar(i64)
 
 define i64 @test_intervening_call(i64* %foo, i64 %bar, i64 %baz) {
 ; i386-LABEL: test_intervening_call:
 ; i386: cmpxchg8b
 ; i386-NEXT: pushl %eax
 ; i386-NEXT: seto %al
 ; i386-NEXT: lahf
 ; i386-NEXT: movl %eax, [[FLAGS:%.*]]
 ; i386-NEXT: popl %eax
 ; i386-NEXT: movl %edx, 4(%esp)
 ; i386-NEXT: movl %eax, (%esp)
 ; i386-NEXT: calll bar
+; ** FIXME Next line isn't actually necessary. **
+; i386-NEXT: pushl %eax
 ; i386-NEXT: movl [[FLAGS]], %eax
 ; i386-NEXT: addb $127, %al
 ; i386-NEXT: sahf
+; ** FIXME Next line isn't actually necessary. **
+; i386-NEXT: popl %eax
 ; i386-NEXT: jne
 
 ; i386f-LABEL: test_intervening_call:
 ; i386f: cmpxchg8b
 ; i386f-NEXT: movl %eax, (%esp)
 ; i386f-NEXT: movl %edx, 4(%esp)
+; ** FIXME Next line isn't actually necessary. **
+; i386f-NEXT: pushl %eax
 ; i386f-NEXT: seto %al
 ; i386f-NEXT: lahf
 ; i386f-NEXT: movl %eax, [[FLAGS:%.*]]
+; ** FIXME Next line isn't actually necessary. **
+; i386f-NEXT: popl %eax
 ; i386f-NEXT: calll bar
+; ** FIXME Next line isn't actually necessary. **
+; i386f-NEXT: pushl %eax
 ; i386f-NEXT: movl [[FLAGS]], %eax
 ; i386f-NEXT: addb $127, %al
 ; i386f-NEXT: sahf
+; ** FIXME Next line isn't actually necessary. **
+; i386f-NEXT: popl %eax
 ; i386f-NEXT: jne
 
 ; x8664-LABEL: test_intervening_call:
 ; x8664: cmpxchgq
 ; x8664: pushq %rax
 ; x8664-NEXT: seto %al
 ; x8664-NEXT: lahf
 ; x8664-NEXT: movq %rax, [[FLAGS:%.*]]
 ; x8664-NEXT: popq %rax
 ; x8664-NEXT: movq %rax, %rdi
 ; x8664-NEXT: callq bar
+; ** FIXME Next line isn't actually necessary. **
+; x8664-NEXT: pushq %rax
 ; x8664-NEXT: movq [[FLAGS]], %rax
 ; x8664-NEXT: addb $127, %al
 ; x8664-NEXT: sahf
+; ** FIXME Next line isn't actually necessary. **
+; x8664-NEXT: popq %rax
 ; x8664-NEXT: jne
 
   %cx = cmpxchg i64* %foo, i64 %bar, i64 %baz seq_cst seq_cst
   %v = extractvalue { i64, i1 } %cx, 0
   %p = extractvalue { i64, i1 } %cx, 1
   call i32 @bar(i64 %v)
   br i1 %p, label %t, label %f
 
cond.end:
   ret i32 %cond
 }
 
 ; This one is an interesting case because CMOV doesn't have a chain
 ; operand. Naive attempts to limit cmpxchg EFLAGS use are likely to fail here.
 define i32 @test_feed_cmov(i32* %addr, i32 %desired, i32 %new) {
 ; i386-LABEL: test_feed_cmov:
 ; i386: cmpxchgl
+; ** FIXME Next line isn't actually necessary. **
+; i386-NEXT: pushl %eax
 ; i386-NEXT: seto %al
 ; i386-NEXT: lahf
 ; i386-NEXT: movl %eax, [[FLAGS:%.*]]
+; ** FIXME Next line isn't actually necessary. **
+; i386-NEXT: popl %eax
 ; i386-NEXT: calll foo
 ; i386-NEXT: pushl %eax
 ; i386-NEXT: movl [[FLAGS]], %eax
 ; i386-NEXT: addb $127, %al
 ; i386-NEXT: sahf
 ; i386-NEXT: popl %eax
 
 ; i386f-LABEL: test_feed_cmov:
 ; i386f: cmpxchgl
+; ** FIXME Next line isn't actually necessary. **
+; i386f-NEXT: pushl %eax
 ; i386f-NEXT: seto %al
 ; i386f-NEXT: lahf
 ; i386f-NEXT: movl %eax, [[FLAGS:%.*]]
+; ** FIXME Next line isn't actually necessary. **
+; i386f-NEXT: popl %eax
 ; i386f-NEXT: calll foo
 ; i386f-NEXT: pushl %eax
 ; i386f-NEXT: movl [[FLAGS]], %eax
 ; i386f-NEXT: addb $127, %al
 ; i386f-NEXT: sahf
 ; i386f-NEXT: popl %eax
 
 ; x8664-LABEL: test_feed_cmov:
 ; x8664: cmpxchgl
+; ** FIXME Next line isn't actually necessary. **
+; x8664: pushq %rax
 ; x8664: seto %al
 ; x8664-NEXT: lahf
 ; x8664-NEXT: movq %rax, [[FLAGS:%.*]]
+; ** FIXME Next line isn't actually necessary. **
+; x8664-NEXT: popq %rax
 ; x8664-NEXT: callq foo
 ; x8664-NEXT: pushq %rax
 ; x8664-NEXT: movq [[FLAGS]], %rax
 ; x8664-NEXT: addb $127, %al
 ; x8664-NEXT: sahf
 ; x8664-NEXT: popq %rax
 
   %res = cmpxchg i32* %addr, i32 %desired, i32 %new seq_cst seq_cst
   %success = extractvalue { i32, i1 } %res, 1
 
   %rhs = call i32 @foo()
 
   %ret = select i1 %success, i32 %new, i32 %rhs
   ret i32 %ret
 }

llvm/trunk/test/CodeGen/X86/peephole-na-phys-copy-folding.ll

-; RUN: llc -verify-machineinstrs -mtriple=i386-linux-gnu %s -o - | FileCheck %s
-; RUN: llc -verify-machineinstrs -mtriple=x86_64-linux-gnu %s -o - | FileCheck %s
+; RUN: llc -mtriple=i386-linux-gnu %s -o - | FileCheck %s
+; RUN: llc -mtriple=x86_64-linux-gnu %s -o - | FileCheck %s
+
+; FIXME Add -verify-machineinstrs back when PR24535 is fixed.
 
 ; The peephole optimizer can elide some physical register copies such as
 ; EFLAGS. Make sure the flags are used directly, instead of needlessly using
 ; lahf, when possible.
 
 @L = external global i32
 @M = external global i8
 declare i32 @bar(i64)
t:
   ret i64 42
 
 f:
   ret i64 0
 }
 
 ; CHECK-LABEL: test_two_live_flags:
 ; CHECK:       cmpxchg
-; CHECK-NEXT:  seto %al
+; CHECK:       seto %al
 ; CHECK-NEXT:  lahf
 ; Save result of the first cmpxchg into D.
 ; CHECK-NEXT:  mov{{[lq]}} %[[AX:[er]ax]], %[[D:[re]d[xi]]]
 ; CHECK:       cmpxchg
 ; CHECK-NEXT:  sete %al
 ; Save result of the second cmpxchg onto the stack.
 ; CHECK-NEXT:  push{{[lq]}} %[[AX]]
 ; Restore result of the first cmpxchg from D, put it back in EFLAGS.