Cherry pick an upstream issue with x86 mentionned here: https://lists.llvm.org/pipermail/llvm-dev/2018-August/125111.html "A very subtle miscompile due to a bug in EFLAGS copy lowering for X86 was fixed. Chandler suggests maintainers of out-of-tree branches using the X86 backend may want to cherry-pick this fix." https://reviews.llvm.org/rL338481

This commit is contained in:
Sylvestre Ledru 2018-09-10 16:57:56 +02:00
parent 79ca353790
commit 8337dca806
3 changed files with 160 additions and 0 deletions

6
debian/changelog vendored
View File

@ -7,6 +7,12 @@ llvm-toolchain-6.0 (1:6.0.1-7) unstable; urgency=medium
See upstream bug 38707 (Closes: #907622)
* Fix an optimization issues (Closes: #907649)
See upstream bug 38786
* Cherry pick an upstream issue with x86 mentionned here:
https://lists.llvm.org/pipermail/llvm-dev/2018-August/125111.html
"A very subtle miscompile due to a bug in EFLAGS copy lowering
for X86 was fixed. Chandler suggests maintainers of out-of-tree
branches using the X86 backend may want to cherry-pick this fix."
https://reviews.llvm.org/rL338481
-- Sylvestre Ledru <sylvestre@debian.org> Thu, 06 Sep 2018 20:51:24 +0200

View File

@ -0,0 +1,153 @@
Index: llvm-toolchain-6.0-6.0.1/test/CodeGen/X86/flags-copy-lowering.mir
===================================================================
--- llvm-toolchain-6.0-6.0.1.orig/test/CodeGen/X86/flags-copy-lowering.mir
+++ llvm-toolchain-6.0-6.0.1/test/CodeGen/X86/flags-copy-lowering.mir
@@ -72,6 +72,18 @@
call void @foo()
ret void
}
+
+ define i32 @test_existing_setcc(i64 %a, i64 %b) {
+ entry:
+ call void @foo()
+ ret i32 0
+ }
+
+ define i32 @test_existing_setcc_memory(i64 %a, i64 %b) {
+ entry:
+ call void @foo()
+ ret i32 0
+ }
...
---
name: test_branch
@@ -553,3 +565,110 @@ body: |
RET 0
...
+---
+name: test_existing_setcc
+# CHECK-LABEL: name: test_existing_setcc
+liveins:
+ - { reg: '$rdi', virtual-reg: '%0' }
+ - { reg: '$rsi', virtual-reg: '%1' }
+body: |
+ bb.0:
+ successors: %bb.1, %bb.2, %bb.3
+ liveins: $rdi, $rsi
+
+ %0:gr64 = COPY $rdi
+ %1:gr64 = COPY $rsi
+ CMP64rr %0, %1, implicit-def $eflags
+ %2:gr8 = SETAr implicit $eflags
+ %3:gr8 = SETAEr implicit $eflags
+ %4:gr64 = COPY $eflags
+ ; CHECK: CMP64rr %0, %1, implicit-def $eflags
+ ; CHECK-NEXT: %[[A_REG:[^:]*]]:gr8 = SETAr implicit $eflags
+ ; CHECK-NEXT: %[[AE_REG:[^:]*]]:gr8 = SETAEr implicit $eflags
+ ; CHECK-NOT: COPY{{( killed)?}} $eflags
+
+ ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+ CALL64pcrel32 @foo, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp, implicit-def $eax
+ ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+
+ $eflags = COPY %4
+ JA_1 %bb.1, implicit $eflags
+ JB_1 %bb.2, implicit $eflags
+ JMP_1 %bb.3
+ ; CHECK-NOT: $eflags =
+ ;
+ ; CHECK: TEST8rr %[[A_REG]], %[[A_REG]], implicit-def $eflags
+ ; CHECK-NEXT: JNE_1 %bb.1, implicit killed $eflags
+ ; CHECK-SAME: {{$[[:space:]]}}
+ ; CHECK-NEXT: bb.4:
+ ; CHECK-NEXT: successors: {{.*$}}
+ ; CHECK-SAME: {{$[[:space:]]}}
+ ; CHECK-NEXT: TEST8rr %[[AE_REG]], %[[AE_REG]], implicit-def $eflags
+ ; CHECK-NEXT: JE_1 %bb.2, implicit killed $eflags
+ ; CHECK-NEXT: JMP_1 %bb.3
+
+ bb.1:
+ %5:gr32 = MOV32ri64 42
+ $eax = COPY %5
+ RET 0, $eax
+
+ bb.2:
+ %6:gr32 = MOV32ri64 43
+ $eax = COPY %6
+ RET 0, $eax
+
+ bb.3:
+ %7:gr32 = MOV32r0 implicit-def dead $eflags
+ $eax = COPY %7
+ RET 0, $eax
+
+...
+---
+name: test_existing_setcc_memory
+# CHECK-LABEL: name: test_existing_setcc_memory
+liveins:
+ - { reg: '$rdi', virtual-reg: '%0' }
+ - { reg: '$rsi', virtual-reg: '%1' }
+body: |
+ bb.0:
+ successors: %bb.1, %bb.2
+ liveins: $rdi, $rsi
+
+ %0:gr64 = COPY $rdi
+ %1:gr64 = COPY $rsi
+ CMP64rr %0, %1, implicit-def $eflags
+ SETEm %0, 1, $noreg, -16, $noreg, implicit $eflags
+ %2:gr64 = COPY $eflags
+ ; CHECK: CMP64rr %0, %1, implicit-def $eflags
+ ; We cannot reuse this SETE because it stores the flag directly to memory,
+ ; so we have two SETEs here. FIXME: It'd be great if something could fold
+ ; these automatically. If not, maybe we want to unfold SETcc instructions
+ ; writing to memory so we can reuse them.
+ ; CHECK-NEXT: SETEm {{.*}} implicit $eflags
+ ; CHECK-NEXT: %[[E_REG:[^:]*]]:gr8 = SETEr implicit $eflags
+ ; CHECK-NOT: COPY{{( killed)?}} $eflags
+
+ ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+ CALL64pcrel32 @foo, csr_64, implicit $rsp, implicit $ssp, implicit $rdi, implicit-def $rsp, implicit-def $ssp, implicit-def $eax
+ ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+
+ $eflags = COPY %2
+ JE_1 %bb.1, implicit $eflags
+ JMP_1 %bb.2
+ ; CHECK-NOT: $eflags =
+ ;
+ ; CHECK: TEST8rr %[[E_REG]], %[[E_REG]], implicit-def $eflags
+ ; CHECK-NEXT: JNE_1 %bb.1, implicit killed $eflags
+ ; CHECK-NEXT: JMP_1 %bb.2
+
+ bb.1:
+ %3:gr32 = MOV32ri64 42
+ $eax = COPY %3
+ RET 0, $eax
+
+ bb.2:
+ %4:gr32 = MOV32ri64 43
+ $eax = COPY %4
+ RET 0, $eax
+
+...
Index: llvm-toolchain-6.0-6.0.1/lib/Target/X86/X86FlagsCopyLowering.cpp
===================================================================
--- llvm-toolchain-6.0-6.0.1.orig/lib/Target/X86/X86FlagsCopyLowering.cpp
+++ llvm-toolchain-6.0-6.0.1/lib/Target/X86/X86FlagsCopyLowering.cpp
@@ -608,9 +608,12 @@ X86FlagsCopyLoweringPass::collectCondsIn
for (MachineInstr &MI : llvm::reverse(
llvm::make_range(MBB.instr_begin(), CopyDefI.getIterator()))) {
X86::CondCode Cond = X86::getCondFromSETOpc(MI.getOpcode());
- if (Cond != X86::COND_INVALID && MI.getOperand(0).isReg() &&
- TRI->isVirtualRegister(MI.getOperand(0).getReg()))
+ if (Cond != X86::COND_INVALID && !MI.mayStore() && MI.getOperand(0).isReg() &&
+ TRI->isVirtualRegister(MI.getOperand(0).getReg())) {
+ assert(MI.getOperand(0).isDef() &&
+ "A non-storing SETcc should always define a register!");
CondRegs[Cond] = MI.getOperand(0).getReg();
+ }
// Stop scanning when we see the first definition of the EFLAGS as prior to
// this we would potentially capture the wrong flag state.

View File

@ -64,3 +64,4 @@ D51108-rust-powerpc.diff
pr38663-pgo-lto-crash.patch
D51335-alignment-issue.diff
D51639-optim-issue.diff
rL338481-cherry-pick-really-subtle-miscompile.diff