rL338481-cherry-pick-really-subtle-miscompile
authorLLVM Packaging Team <pkg-llvm-team@lists.alioth.debian.org>
Mon, 20 Jan 2020 09:26:04 +0000 (09:26 +0000)
committerGianfranco Costamagna <locutusofborg@debian.org>
Mon, 20 Jan 2020 09:26:04 +0000 (09:26 +0000)
===================================================================

Gbp-Pq: Name rL338481-cherry-pick-really-subtle-miscompile.diff

lib/Target/X86/X86FlagsCopyLowering.cpp
test/CodeGen/X86/flags-copy-lowering.mir

index a6fccd13474078ad47f6a5101fe608f43649f946..41c8d6ee9a82e942828b7073a7152d255f000b68 100644 (file)
@@ -608,9 +608,12 @@ X86FlagsCopyLoweringPass::collectCondsInRegs(MachineBasicBlock &MBB,
   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.
index 3d8a4ed3c734e7fcda065f06be8662ca429a30a4..b9fb9a7446e693d113edc36c11496126e3e2f668 100644 (file)
     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
+
+...