[PATCH] D42533: [X86FixupBWInsts] Fix miscompilation if sibling sub-register is live.

[Andrei Elovikov via Phabricator via llvm-commits]

a.elovikov updated this revision to Diff 131474.
a.elovikov added a comment.

Indeed, on X86 checkRegistersAlias is just isSuperOrSubRegisterEq (although the former would be the right choice if that was not so :)).
Comment was added - thanks for the exact text!


Repository:
  rL LLVM

https://reviews.llvm.org/D42533

Files:
  lib/Target/X86/X86FixupBWInsts.cpp
  test/CodeGen/X86/fixup-bw-inst.mir


Index: test/CodeGen/X86/fixup-bw-inst.mir
===================================================================
--- test/CodeGen/X86/fixup-bw-inst.mir
+++ test/CodeGen/X86/fixup-bw-inst.mir
@@ -32,6 +32,8 @@
     %t2 = or i16 undef, %t1
     ret i16 %t2
   }
+
+  define void @test5() {ret void}
 ...
 ---
 # CHECK-LABEL: name: test1
@@ -199,3 +201,46 @@
     %ax = OR16rr undef %ax, %r9w, implicit-def %eflags
     RETQ %ax
 ...
+
+---
+# CHECK-LABEL: name: test5
+name:            test5
+alignment:       4
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+registers:
+liveins:
+  - { reg: '%ch', reg: '%bl' }
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    0
+  adjustsStack:    false
+  hasCalls:        false
+  stackProtector:  ''
+  maxCallFrameSize: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:
+stack:
+constants:
+body:             |
+  bb.0:
+    successors:
+    liveins: %ch, %bl
+
+    %cl = MOV8rr %bl, implicit-def %cx, implicit killed %ch, implicit-def %eflags
+    ; CHECK: %cl = MOV8rr %bl, implicit-def %cx, implicit killed %ch, implicit-def %eflags
+
+    RETQ %cx
+...
Index: lib/Target/X86/X86FixupBWInsts.cpp
===================================================================
--- lib/Target/X86/X86FixupBWInsts.cpp
+++ lib/Target/X86/X86FixupBWInsts.cpp
@@ -249,15 +249,16 @@
 
     assert((MO.isDef() || MO.isUse()) && "Expected Def or Use only!");
 
-    for (MCSuperRegIterator Supers(OrigDestReg, TRI, true); Supers.isValid();
-         ++Supers) {
-      if (*Supers == MO.getReg()) {
-        if (MO.isDef())
-          IsDefined = true;
-        else
-          return false; // SuperReg Imp-used' -> live before the MI
-      }
-    }
+    if (MO.isDef() && TRI->isSuperRegisterEq(OrigDestReg, MO.getReg()))
+        IsDefined = true;
+
+    // If MO is a use of any part of the destination register but is not equal
+    // to OrigDestReg or one of its subregisters, we cannot use SuperDestReg.
+    // For example, if OrigDestReg is %al then an implicit use of %ah, %ax,
+    // %eax, or %rax will prevent us from using the %eax register.
+    if (MO.isUse() && !TRI->isSubRegisterEq(OrigDestReg, MO.getReg()) &&
+        TRI->isSuperOrSubRegisterEq(SuperDestReg, MO.getReg()))
+      return false;
   }
   // Reg is not Imp-def'ed -> it's live both before/after the instruction.
   if (!IsDefined)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D42533.131474.patch
Type: text/x-patch
Size: 2658 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180125/536b518e/attachment.bin>