[PATCH] D40524: Handle the case of live 16-bit subregisters in X86FixupBWInsts

[Andy Kaylor via Phabricator via llvm-commits]

andrew.w.kaylor created this revision.

This fixes Bugzilla report 35240 (https://bugs.llvm.org/show_bug.cgi?id=35240).

What was happening in that case is we had an instruction the was copying R10B into R9B and we wanted to promote this to a 32-bit move. This potentially leaves the upper 24 bits of R9D as undefined. That's OK if those bits are never used, but if they are then we need to prevent the transformation.

The Machine IR before the transformation in the failing test case looks like this:

  BB#3: derived from LLVM BB %entry
      Live Ins: %EAX %EDX %ESI %R8B **%R10B %R9D**
      Predecessors according to CFG: BB#2 BB#1
    %CL<def> = MOV8rr %R8B<kill>
    %EDX<def,tied1> = SHR32rCL %EDX<kill,tied0>, %EFLAGS<imp-def,dead>, %CL<imp-use>
    MOV32mr %RSP, 1, %noreg, 24, %noreg, %EDX<kill>; mem:Volatile ST4[%k]
  **  %R9B<def> = MOV8rr %R10B<kill>, %R9D<imp-use,kill>, %R9D<imp-def>**
    %CX<def> = MOV16rm %RSP, 1, %noreg, 18, %noreg; mem:Volatile LD2[%f](dereferenceable)
  **  %CX<def,tied1> = OR16rr %CX<kill,tied0>, %R9W, %EFLAGS<imp-def,dead>**
   <snip>

As LivePhysRegs is stepping backward, it adds R9W to the live set, but when X86FixupBWInsts checks for liveness, it only checks R8D. This patch fixes that.

When a live register is added using LivePhysRegs::addReg() all of its sub-registers are also marked as live, but not super registers. In theory, we could take advantage of this behavior and only check the 16-bit sub-register for liveness (since if the 32-bit subregister is live the 16-bit subregister will be also), but the check is cheap and it seems preferable to have the code explicitly checking the conditions it requires.


https://reviews.llvm.org/D40524

Files:
  lib/Target/X86/X86FixupBWInsts.cpp


Index: lib/Target/X86/X86FixupBWInsts.cpp
===================================================================
--- lib/Target/X86/X86FixupBWInsts.cpp
+++ lib/Target/X86/X86FixupBWInsts.cpp
@@ -265,6 +265,11 @@
     return false;
 
   if (SubRegIdx == X86::sub_8bit) {
+    // We've checked that the 32-bit super register isn't live, but the
+    // 16-bit form might still be live.
+    unsigned DestReg16 = getX86SubSuperRegister(OrigDestReg, 16);
+    if (isLive(*OrigMI, LiveRegs, TRI, DestReg16))
+      return false;
     // In the case of byte registers, we also have to check that the upper
     // byte register is also dead. That is considered to be independent of
     // whether the super-register is dead.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40524.124471.patch
Type: text/x-patch
Size: 717 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171127/9ac9d6ad/attachment.bin>