Handle the case of live 16-bit subregisters in X86FixupBWInsts

Differential Revision: https://reviews.llvm.org/D40524

Change-Id: Ie3a405b28503ceae999f5f3ba07a68fa733a2400

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@321674 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Target/X86/X86FixupBWInsts.cpp

@@ -166,49 +166,75 @@ bool FixupBWInstPass::runOnMachineFunction(MachineFunction &MF) {
   return true;
 }
 
-/// Check if register \p Reg is live after the \p MI.
-///
-/// \p LiveRegs should be in a state describing liveness information in
-/// that exact place as this function tries to precise analysis made
-/// by \p LiveRegs by exploiting the information about particular
-/// instruction \p MI. \p MI is expected to be one of the MOVs handled
-/// by the x86FixupBWInsts pass.
-/// Note: similar to LivePhysRegs::contains this would state that
-/// super-register is not used if only some part of it is used.
-///
-/// X86 backend does not have subregister liveness tracking enabled,
-/// so liveness information might be overly conservative. However, for
-/// some specific instructions (this pass only cares about MOVs) we can
-/// produce more precise results by analysing that MOV's operands.
-///
-/// Indeed, if super-register is not live before the mov it means that it
-/// was originally <read-undef> and so we are free to modify these
-/// undef upper bits. That may happen in case where the use is in another MBB
-/// and the vreg/physreg corresponding to the move has higher width than
-/// necessary (e.g. due to register coalescing with a "truncate" copy).
-/// So, it handles pattern like this:
-///
-///   %bb.2: derived from LLVM BB %if.then
-///   Live Ins: %rdi
-///   Predecessors according to CFG: %bb.0
-///   %ax = MOV16rm killed %rdi, 1, %noreg, 0, %noreg, implicit-def %eax;
-///   mem:LD2[%p]
-///                                             No implicit %eax
-///   Successors according to CFG: %bb.3(?%)
+/// \brief Check if after \p OrigMI the only portion of super register
+/// of the destination register of \p OrigMI that is alive is that
+/// destination register.
 ///
-///   %bb.3: derived from LLVM BB %if.end
-///   Live Ins: %eax                            Only %ax is actually live
-///   Predecessors according to CFG: %bb.2 %bb.1
-///   %ax = KILL %ax, implicit killed %eax
-///   RET 0, %ax
-static bool isLive(const MachineInstr &MI,
-                   const LivePhysRegs &LiveRegs,
-                   const TargetRegisterInfo *TRI,
-                   unsigned Reg) {
-  if (!LiveRegs.contains(Reg))
+/// If so, return that super register in \p SuperDestReg.
+bool FixupBWInstPass::getSuperRegDestIfDead(MachineInstr *OrigMI,
+                                            unsigned &SuperDestReg) const {
+  auto *TRI = &TII->getRegisterInfo();
+
+  unsigned OrigDestReg = OrigMI->getOperand(0).getReg();
+  SuperDestReg = getX86SubSuperRegister(OrigDestReg, 32);
+
+  const auto SubRegIdx = TRI->getSubRegIndex(SuperDestReg, OrigDestReg);
+
+  // Make sure that the sub-register that this instruction has as its
+  // destination is the lowest order sub-register of the super-register.
+  // If it isn't, then the register isn't really dead even if the
+  // super-register is considered dead.
+  if (SubRegIdx == X86::sub_8bit_hi)
     return false;
 
-  unsigned Opc = MI.getOpcode(); (void)Opc;
+  // If neither the destination-super register nor any applicable subregisters
+  // are live after this instruction, then the super register is safe to use.
+  if (!LiveRegs.contains(SuperDestReg)) {
+    // If the original destination register was not the low 8-bit subregister
+    // then the super register check is sufficient.
+    if (SubRegIdx != X86::sub_8bit)
+      return true;
+    // If the original destination register was the low 8-bit subregister and
+    // we also need to check the 16-bit subregister and the high 8-bit
+    // subregister.
+    if (!LiveRegs.contains(getX86SubSuperRegister(OrigDestReg, 16)) &&
+        !LiveRegs.contains(getX86SubSuperRegister(SuperDestReg, 8,
+                                                  /*High=*/true)))
+      return true;
+    // Otherwise, we have a little more checking to do.
+  }
+
+  // If we get here, the super-register destination (or some part of it) is
+  // marked as live after the original instruction.
+  //
+  // The X86 backend does not have subregister liveness tracking enabled,
+  // so liveness information might be overly conservative. Specifically, the
+  // super register might be marked as live because it is implicitly defined
+  // by the instruction we are examining.
+  //
+  // However, for some specific instructions (this pass only cares about MOVs)
+  // we can produce more precise results by analysing that MOV's operands.
+  //
+  // Indeed, if super-register is not live before the mov it means that it
+  // was originally <read-undef> and so we are free to modify these
+  // undef upper bits. That may happen in case where the use is in another MBB
+  // and the vreg/physreg corresponding to the move has higher width than
+  // necessary (e.g. due to register coalescing with a "truncate" copy).
+  // So, we would like to handle patterns like this:
+  //
+  //   %bb.2: derived from LLVM BB %if.then
+  //   Live Ins: %rdi
+  //   Predecessors according to CFG: %bb.0
+  //   %ax<def> = MOV16rm killed %rdi, 1, %noreg, 0, %noreg, implicit-def %eax
+  //                                 ; No implicit %eax
+  //   Successors according to CFG: %bb.3(?%)
+  //
+  //   %bb.3: derived from LLVM BB %if.end
+  //   Live Ins: %eax                            Only %ax is actually live
+  //   Predecessors according to CFG: %bb.2 %bb.1
+  //   %ax = KILL %ax, implicit killed %eax
+  //   RET 0, %ax
+  unsigned Opc = OrigMI->getOpcode(); (void)Opc;
   // These are the opcodes currently handled by the pass, if something
   // else will be added we need to ensure that new opcode has the same
   // properties.
@@ -217,65 +243,28 @@ static bool isLive(const MachineInstr &MI,
          "Unexpected opcode.");
 
   bool IsDefined = false;
-  for (auto &MO: MI.implicit_operands()) {
+  for (auto &MO: OrigMI->implicit_operands()) {
     if (!MO.isReg())
       continue;
 
     assert((MO.isDef() || MO.isUse()) && "Expected Def or Use only!");
 
-    for (MCSuperRegIterator Supers(Reg, TRI, true); Supers.isValid(); ++Supers) {
+    for (MCSuperRegIterator Supers(OrigDestReg, TRI, true); Supers.isValid();
+         ++Supers) {
       if (*Supers == MO.getReg()) {
         if (MO.isDef())
           IsDefined = true;
         else
-          return true; // SuperReg Imp-used' -> live before the MI
+          return false; // SuperReg Imp-used' -> live before the MI
       }
     }
   }
   // Reg is not Imp-def'ed -> it's live both before/after the instruction.
   if (!IsDefined)
-    return true;
+    return false;
 
   // Otherwise, the Reg is not live before the MI and the MOV can't
   // make it really live, so it's in fact dead even after the MI.
-  return false;
-}
-
-/// \brief Check if after \p OrigMI the only portion of super register
-/// of the destination register of \p OrigMI that is alive is that
-/// destination register.
-///
-/// If so, return that super register in \p SuperDestReg.
-bool FixupBWInstPass::getSuperRegDestIfDead(MachineInstr *OrigMI,
-                                            unsigned &SuperDestReg) const {
-  auto *TRI = &TII->getRegisterInfo();
-
-  unsigned OrigDestReg = OrigMI->getOperand(0).getReg();
-  SuperDestReg = getX86SubSuperRegister(OrigDestReg, 32);
-
-  const auto SubRegIdx = TRI->getSubRegIndex(SuperDestReg, OrigDestReg);
-
-  // Make sure that the sub-register that this instruction has as its
-  // destination is the lowest order sub-register of the super-register.
-  // If it isn't, then the register isn't really dead even if the
-  // super-register is considered dead.
-  if (SubRegIdx == X86::sub_8bit_hi)
-    return false;
-
-  if (isLive(*OrigMI, LiveRegs, TRI, SuperDestReg))
-    return false;
-
-  if (SubRegIdx == X86::sub_8bit) {
-    // 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.
-    unsigned UpperByteReg =
-        getX86SubSuperRegister(SuperDestReg, 8, /*High=*/true);
-
-    if (isLive(*OrigMI, LiveRegs, TRI, UpperByteReg))
-      return false;
-  }
-
   return true;
 }
 

test/CodeGen/X86/fixup-bw-inst.mir

@@ -26,6 +26,12 @@
     ret i16 %i.0
   }
 
+  define i16 @test4() {
+  entry:
+    %t1 = zext i1 undef to i16
+    %t2 = or i16 undef, %t1
+    ret i16 %t2
+  }
 ...
 ---
 # CHECK-LABEL: name: test1
@@ -149,3 +155,47 @@ body:             |
     RETQ %ax
 
 ...
+---
+# CHECK-LABEL: name: test4
+name:            test4
+alignment:       4
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+registers:
+liveins:
+  - { reg: '%r9d' }
+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:
+# This code copies r10b into r9b and then uses r9w. We would like to promote
+# the copy to a 32-bit copy, but because r9w is used this is not acceptable.
+body:             |
+  bb.0.entry:
+    successors:
+    liveins: %r9d
+
+    %r9b = MOV8rr undef %r10b, implicit-def %r9d, implicit killed %r9d, implicit-def %eflags
+    ; CHECK-NOT: MOV32rr
+    %ax = OR16rr undef %ax, %r9w, implicit-def %eflags
+    RETQ %ax
+...