[llvm] r270935 - [CodeGen] Fix problem with X86 byte registers in CriticalAntiDepBreaker

[Mitch Bodart via llvm-commits]

Author: mbodart
Date: Thu May 26 18:08:52 2016
New Revision: 270935

URL: http://llvm.org/viewvc/llvm-project?rev=270935&view=rev
Log:
[CodeGen] Fix problem with X86 byte registers in CriticalAntiDepBreaker

CriticalAntiDepBreaker was not correctly tracking defs of the high X86 byte
registers, leading to incorrect use of a busy register to break an
antidependence.

Fixes pr27681, and its duplicates pr27580, pr27804.

Differential Revision: http://reviews.llvm.org/D20456

Added:
    llvm/trunk/test/CodeGen/X86/pr27681.mir
Modified:
    llvm/trunk/lib/CodeGen/CriticalAntiDepBreaker.cpp

Modified: llvm/trunk/lib/CodeGen/CriticalAntiDepBreaker.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CriticalAntiDepBreaker.cpp?rev=270935&r1=270934&r2=270935&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/CriticalAntiDepBreaker.cpp (original)
+++ llvm/trunk/lib/CodeGen/CriticalAntiDepBreaker.cpp Thu May 26 18:08:52 2016
@@ -260,22 +260,24 @@ void CriticalAntiDepBreaker::ScanInstruc
       if (Reg == 0) continue;
       if (!MO.isDef()) continue;
 
-      // If we've already marked this reg as unchangeable, carry on.
-      if (KeepRegs.test(Reg)) continue;
-      
       // Ignore two-addr defs.
       if (MI.isRegTiedToUseOperand(i))
         continue;
 
+      // If we've already marked this reg as unchangeable, don't remove
+      // it or any of its subregs from KeepRegs.
+      bool Keep = KeepRegs.test(Reg);
+
       // For the reg itself and all subregs: update the def to current;
       // reset the kill state, any restrictions, and references.
       for (MCSubRegIterator SRI(Reg, TRI, true); SRI.isValid(); ++SRI) {
         unsigned SubregReg = *SRI;
         DefIndices[SubregReg] = Count;
         KillIndices[SubregReg] = ~0u;
-        KeepRegs.reset(SubregReg);
         Classes[SubregReg] = nullptr;
         RegRefs.erase(SubregReg);
+        if (!Keep)
+          KeepRegs.reset(SubregReg);
       }
       // Conservatively mark super-registers as unusable.
       for (MCSuperRegIterator SR(Reg, TRI); SR.isValid(); ++SR)

Added: llvm/trunk/test/CodeGen/X86/pr27681.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/pr27681.mir?rev=270935&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/pr27681.mir (added)
+++ llvm/trunk/test/CodeGen/X86/pr27681.mir Thu May 26 18:08:52 2016
@@ -0,0 +1,87 @@
+# RUN: llc -mtriple=i386-unknown-linux-gnu -mcpu=slm -run-pass post-RA-sched -o /dev/null %s 2>&1 | FileCheck %s
+#
+# Verify that the critical antidependence breaker does not consider
+# a high byte register as available as a replacement register
+# in a certain context.
+--- |
+
+  define void @main() { ret void }
+
+...
+---
+# CHECK-LABEL: main
+name:            main
+allVRegsAllocated: true
+tracksRegLiveness: true
+frameInfo:
+  stackSize:       52
+fixedStack:
+  - { id: 0, type: spill-slot, offset: -20, size: 4, alignment: 4, callee-saved-register: '%esi' }
+  - { id: 1, type: spill-slot, offset: -16, size: 4, alignment: 4, callee-saved-register: '%edi' }
+  - { id: 2, type: spill-slot, offset: -12, size: 4, alignment: 4, callee-saved-register: '%ebx' }
+  - { id: 3, type: spill-slot, offset: -8, size: 4, alignment: 4, callee-saved-register: '%ebp' }
+stack:
+  - { id: 0, type: spill-slot, offset: -53, size: 1, alignment: 1 }
+  - { id: 1, type: spill-slot, offset: -48, size: 4, alignment: 4 }
+  - { id: 2, type: spill-slot, offset: -32, size: 4, alignment: 4 }
+body:             |
+  bb.0:
+    successors: %bb.1
+    liveins: %ebp, %ebx, %edi, %esi
+
+    frame-setup PUSH32r killed %ebp, implicit-def %esp, implicit %esp
+    frame-setup PUSH32r killed %ebx, implicit-def %esp, implicit %esp
+    frame-setup PUSH32r killed %edi, implicit-def %esp, implicit %esp
+    frame-setup PUSH32r killed %esi, implicit-def %esp, implicit %esp
+    %esp = frame-setup SUB32ri8 %esp, 36, implicit-def dead %eflags
+    %eax = MOV32ri 1
+    %ebp = MOV32ri 2
+    %ebx = MOV32ri 3
+    %ecx = MOV32ri 4
+    %edi = MOV32ri 5
+    %edx = MOV32ri 6
+
+  bb.1:
+    successors: %bb.3, %bb.2
+    liveins: %eax, %ebp, %ebx, %ecx, %edi, %edx
+
+    %ebp = SHR32rCL killed %ebp, implicit-def dead %eflags, implicit %cl
+    %ebp = XOR32rr killed %ebp, killed %ebx, implicit-def dead %eflags
+    TEST32rr %edx, %edx, implicit-def %eflags
+    %cl = SETNEr implicit %eflags
+    ; This %bl def is antidependent on the above use of %ebx
+    %bl = MOV8rm %esp, 1, _, 3, _ ; :: (load 1 from %stack.0)
+    %cl = OR8rr killed %cl, %bl, implicit-def dead %eflags
+    %esi = MOVZX32rr8 killed %cl
+    %esi = ADD32rr killed %esi, killed %edi, implicit-def dead %eflags
+    %ecx = MOV32rm %esp, 1, _, 24, _ ; :: (load 4 from %stack.2)
+    %edx = SAR32rCL killed %edx, implicit-def dead %eflags, implicit %cl
+    TEST32rr killed %edx, %edx, implicit-def %eflags
+    %cl = SETNEr implicit %eflags
+    ; Verify that removal of the %bl antidependence does not use %ch
+    ; as a replacement register.
+    ; CHECK: %cl = AND8rr %cl, killed %b
+    %cl = AND8rr killed %cl, killed %bl, implicit-def dead %eflags
+    CMP32ri8 %ebp, -1, implicit-def %eflags
+    %edx = MOV32ri 0
+    JE_1 %bb.3, implicit %eflags
+
+  bb.2:
+    successors: %bb.3
+    liveins: %cl, %eax, %ebp, %esi
+
+    OR32mr %esp, 1, _, 8, _, killed %eax, implicit-def %eflags ; :: (store 4 into %stack.1)
+    %dl = SETNEr implicit %eflags, implicit-def %edx
+
+  bb.3:
+    liveins: %cl, %ebp, %edx, %esi
+
+    %eax = XOR32rr undef %eax, undef %eax, implicit-def dead %eflags
+    %esp = ADD32ri8 %esp, 36, implicit-def dead %eflags
+    %esi = POP32r implicit-def %esp, implicit %esp
+    %edi = POP32r implicit-def %esp, implicit %esp
+    %ebx = POP32r implicit-def %esp, implicit %esp
+    %ebp = POP32r implicit-def %esp, implicit %esp
+    RET 0, %eax
+
+...