[llvm-branch-commits] [llvm-branch] r311108 - Merging r310979:

Thu Aug 17 10:36:27 PDT 2017

Author: hans
Date: Thu Aug 17 10:36:27 2017
New Revision: 311108

URL: http://llvm.org/viewvc/llvm-project?rev=311108&view=rev
Log:
Merging r310979:
------------------------------------------------------------------------
r310979 | qcolombet | 2017-08-15 17:17:05 -0700 (Tue, 15 Aug 2017) | 38 lines

[VirtRegRewriter] Properly model the register liveness on undef subreg definition

Undef subreg definition means that the content of the super register
doesn't matter at this point. While that's true for virtual registers,
this may not hold when replacing them with actual physical registers.
Indeed, some part of the physical register may be coalesced with the
related virtual register and thus, the values for those parts matter and
must be live.

The fix consists in checking whether or not subregs of the physical register
being assigned to an undef subreg definition are live through that def and
insert an implicit use if they are. Doing so, will keep them alive until
that point like they should be.

E.g., let vreg14 being assigned to R0_R1 then
%vreg14:gsub_0<def,read-undef> = COPY %R0 ; <-- R1 is still live here
%vreg14:gsub_1<def> = COPY %R1

Before this changes, the rewriter would change the code into:
%R0<def> = KILL %R0, %R0_R1<imp-def> ; <-- this tells R1 is redefined
%R1<def> = KILL %R1, %R0_R1<imp-def>, %R0_R1<imp-use> ; this value of this R1
                                                      ; is believed to come
                                                      ; from the previous
                                                      ; instruction

Because of this invalid liveness, later pass could make wrong choices and in
particular clobber live register as it happened with the register scavenger in
llvm.org/PR34107

Now we would generate:
%R0<def> = KILL %R0, %R0_R1<imp-def>, %R0_R1<imp-use> ; This tells R1 needs to
                                                      ; reach this point
%R1<def> = KILL %R1, %R0_R1<imp-def>, %R0_R1<imp-use>

The bug has been here forever, it got exposed recently because the register
scavenger got smarter.

Fixes llvm.org/PR34107
------------------------------------------------------------------------

Added:
    llvm/branches/release_50/test/CodeGen/ARM/virtregrewriter-subregliveness.mir
      - copied unchanged from r310979, llvm/trunk/test/CodeGen/ARM/virtregrewriter-subregliveness.mir
Modified:
    llvm/branches/release_50/   (props changed)
    llvm/branches/release_50/lib/CodeGen/VirtRegMap.cpp

Propchange: llvm/branches/release_50/
------------------------------------------------------------------------------

--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Aug 17 10:36:27 2017
@@ -1,3 +1,3 @@
 /llvm/branches/Apple/Pertwee:110850,110961
 /llvm/branches/type-system-rewrite:133420-134817
-/llvm/trunk:155241,308483-308484,308503,308808,308813,308847,308891,308906,308950,308963,308978,308986,309044,309071,309113,309120,309122,309140,309227,309302,309321,309323,309325,309330,309343,309353,309355,309422,309481,309483,309495,309555,309561,309594,309614,309651,309744,309758,309849,309928,309930,309945,310071,310190,310240-310242,310250,310253,310267,310481,310492,310510,310534,310552,310604,310779,310784,310796,310842,310906,310926,310939,310988,310991,311068
+/llvm/trunk:155241,308483-308484,308503,308808,308813,308847,308891,308906,308950,308963,308978,308986,309044,309071,309113,309120,309122,309140,309227,309302,309321,309323,309325,309330,309343,309353,309355,309422,309481,309483,309495,309555,309561,309594,309614,309651,309744,309758,309849,309928,309930,309945,310071,310190,310240-310242,310250,310253,310267,310481,310492,310510,310534,310552,310604,310779,310784,310796,310842,310906,310926,310939,310979,310988,310991,311068

Modified: llvm/branches/release_50/lib/CodeGen/VirtRegMap.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_50/lib/CodeGen/VirtRegMap.cpp?rev=311108&r1=311107&r2=311108&view=diff
==============================================================================
--- llvm/branches/release_50/lib/CodeGen/VirtRegMap.cpp (original)
+++ llvm/branches/release_50/lib/CodeGen/VirtRegMap.cpp Thu Aug 17 10:36:27 2017
@@ -180,6 +180,7 @@ class VirtRegRewriter : public MachineFu
   void addLiveInsForSubRanges(const LiveInterval &LI, unsigned PhysReg) const;
   void handleIdentityCopy(MachineInstr &MI) const;
   void expandCopyBundle(MachineInstr &MI) const;
+  bool subRegLiveThrough(const MachineInstr &MI, unsigned SuperPhysReg) const;
 
 public:
   static char ID;
@@ -415,6 +416,32 @@ void VirtRegRewriter::expandCopyBundle(M
   }
 }
 
+/// Check whether (part of) \p SuperPhysReg is live through \p MI.
+/// \pre \p MI defines a subregister of a virtual register that
+/// has been assigned to \p SuperPhysReg.
+bool VirtRegRewriter::subRegLiveThrough(const MachineInstr &MI,
+                                        unsigned SuperPhysReg) const {
+  SlotIndex MIIndex = LIS->getInstructionIndex(MI);
+  SlotIndex BeforeMIUses = MIIndex.getBaseIndex();
+  SlotIndex AfterMIDefs = MIIndex.getBoundaryIndex();
+  for (MCRegUnitIterator Unit(SuperPhysReg, TRI); Unit.isValid(); ++Unit) {
+    const LiveRange &UnitRange = LIS->getRegUnit(*Unit);
+    // If the regunit is live both before and after MI,
+    // we assume it is live through.
+    // Generally speaking, this is not true, because something like
+    // "RU = op RU" would match that description.
+    // However, we know that we are trying to assess whether
+    // a def of a virtual reg, vreg, is live at the same time of RU.
+    // If we are in the "RU = op RU" situation, that means that vreg
+    // is defined at the same time as RU (i.e., "vreg, RU = op RU").
+    // Thus, vreg and RU interferes and vreg cannot be assigned to
+    // SuperPhysReg. Therefore, this situation cannot happen.
+    if (UnitRange.liveAt(AfterMIDefs) && UnitRange.liveAt(BeforeMIUses))
+      return true;
+  }
+  return false;
+}
+
 void VirtRegRewriter::rewrite() {
   bool NoSubRegLiveness = !MRI->subRegLivenessEnabled();
   SmallVector<unsigned, 8> SuperDeads;
@@ -452,7 +479,8 @@ void VirtRegRewriter::rewrite() {
             // A virtual register kill refers to the whole register, so we may
             // have to add <imp-use,kill> operands for the super-register.  A
             // partial redef always kills and redefines the super-register.
-            if (MO.readsReg() && (MO.isDef() || MO.isKill()))
+            if ((MO.readsReg() && (MO.isDef() || MO.isKill())) ||
+                (MO.isDef() && subRegLiveThrough(*MI, PhysReg)))
               SuperKills.push_back(PhysReg);
 
             if (MO.isDef()) {


