[PATCH] D37559: [X86FixupBWInsts] More presise register liveness if no <imp-use> on MOVs.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86FixupBWInsts.cpp:169
 
+/// 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<def> = MOV16rm %RDI<kill>, 1, %noreg, 0, %noreg, %EAX<imp-def>; mem:LD2[%p]
+///                                             No %EAX<imp-use>
+///   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<def> = KILL %AX, %EAX<imp-use,kill>
+///   RET 0, %AX
+static bool isLive(const MachineInstr &MI,
+                   const LivePhysRegs &LiveRegs,
+                   const TargetRegisterInfo *TRI,
+                   unsigned Reg) {
+  if (!LiveRegs.contains(Reg))
+    return false;
+
+  unsigned Opc = MI.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.
+  assert((Opc == X86::MOV8rm || Opc == X86::MOV16rm || Opc == X86::MOV8rr ||
+          Opc == X86::MOV16rr) &&
+         "Unexpected opcode.");
+
+  bool IsDefined = false;
+  for (auto &MO: MI.implicit_operands()) {
+    if (!MO.isReg() || (!MO.isDef() && !MO.isUse()))
+      continue;
+
+    for (MCSuperRegIterator Supers(Reg, TRI, true); Supers.isValid(); ++Supers) {
+      if (*Supers == MO.getReg()) {
+        if (MO.isDef())
+          IsDefined = true;
+        else
+          return true; // 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;
+
+  // 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;
+}
+
 // TODO: This method of analysis can miss some legal cases, because the
 // super-register could be live into the address expression for a memory
----------------
Not directly related to this patch, but is this TODO still accurate? When it was written this algorithm was completely different. Right now we're using the live regs after the instruction we're considering so we shouldn't be seeing live regs from the address calc right?


https://reviews.llvm.org/D37559