summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--llvm/lib/Target/X86/X86InstrInfo.cpp16
-rw-r--r--llvm/test/CodeGen/X86/cmpxchg-clobber-flags.ll43
-rw-r--r--llvm/test/CodeGen/X86/peephole-na-phys-copy-folding.ll8
3 files changed, 57 insertions, 10 deletions
diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp
index 12da3a9319e..e9d36f8ce2f 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.cpp
+++ b/llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -4412,9 +4412,19 @@ void X86InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
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));
diff --git a/llvm/test/CodeGen/X86/cmpxchg-clobber-flags.ll b/llvm/test/CodeGen/X86/cmpxchg-clobber-flags.ll
index c129128b5fa..791edba89c4 100644
--- a/llvm/test/CodeGen/X86/cmpxchg-clobber-flags.ll
+++ b/llvm/test/CodeGen/X86/cmpxchg-clobber-flags.ll
@@ -1,7 +1,14 @@
-; 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)
@@ -17,22 +24,34 @@ define i64 @test_intervening_call(i64* %foo, i64 %bar, i64 %baz) {
; 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:
@@ -44,9 +63,13 @@ define i64 @test_intervening_call(i64* %foo, i64 %bar, i64 %baz) {
; 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
@@ -111,9 +134,13 @@ cond.end:
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
@@ -123,9 +150,13 @@ define i32 @test_feed_cmov(i32* %addr, i32 %desired, i32 %new) {
; 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
@@ -135,9 +166,13 @@ define i32 @test_feed_cmov(i32* %addr, i32 %desired, i32 %new) {
; 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
diff --git a/llvm/test/CodeGen/X86/peephole-na-phys-copy-folding.ll b/llvm/test/CodeGen/X86/peephole-na-phys-copy-folding.ll
index 438bf8ddf4c..891a925611c 100644
--- a/llvm/test/CodeGen/X86/peephole-na-phys-copy-folding.ll
+++ b/llvm/test/CodeGen/X86/peephole-na-phys-copy-folding.ll
@@ -1,5 +1,7 @@
-; 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
@@ -137,7 +139,7 @@ f:
; 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]]]
OpenPOWER on IntegriCloud