[PATCH] D30558: Fix value numbers in successor blocks if liveout number has changed

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec created this revision.
Herald added a subscriber: tpr.

The problem can occur in presence of subregs. If we are swapping two
instructions defining different subregs of the same register we will
get a new liveout from a block. Liveins in successor blocks need to
be updated to record the new value number.

The review does not have a dedicated test case since that is very
difficult to force scheduler to reorder two certain instructions, but
when I submit https://reviews.llvm.org/D30557 it will break the test CodeGen/AMDGPU/br_cc.f16.ll
without this patch.

To illustrate the problem exposed by that test: before scheduling we had:

  %vreg12 [48r,96r:0)[96r,112r:1)[112r,400r:2)[464B,560r:2)  0 at 48r 1 at 96r 2 at 112r L00000001 [48r,208r:0)  0 at 48r L00000002 [48r,208r:0)  0 at 48r L00000004 [112r,384r:0)[464B,544r:0)  0 at 112r L000000
   
  0B      BB#0: derived from LLVM BB %entry
              Live Ins: %SGPR0_SGPR1
  16B             %vreg1<def> = COPY %SGPR0_SGPR1; SGPR_64:%vreg1
  32B             %vreg20:sub0_sub1<def,read-undef> = S_LOAD_DWORDX2_IMM %vreg1, 9, 0; mem:LD8[undef(addrspace=2)](nontemporal)(dereferenceable)(invariant) SReg_128:%vreg20 SGPR_64:%vreg1
  48B             %vreg12:sub0_sub1<def,read-undef> = S_LOAD_DWORDX2_IMM %vreg1, 11, 0; mem:LD8[undef(addrspace=2)](nontemporal)(dereferenceable)(invariant) SReg_128:%vreg12 SGPR_64:%vreg1
  96B             %vreg12:sub3<def> = S_MOV_B32 61440; SReg_128:%vreg12
  112B            %vreg12:sub2<def> = S_MOV_B32   -1; SReg_128:%vreg12
  …
   
  464B    BB#2: derived from LLVM BB %two
              Predecessors according to CFG: BB#0
  544B            %vreg20:sub2<def> = COPY %vreg12:sub2; SReg_128:%vreg20,%vreg12
  560B            %vreg20:sub3<def> = COPY %vreg12:sub3; SReg_128:%vreg20,%vreg12

%vreg12 is liveout from BB#0, segment with value #2: 112r,400r:2. Livein into #BB2 has value #2: [464B,560r:2)

After the scheduling:

  %vreg12 [24r,28r:1)[28r,48r:2)[48r,400r:0)[464B,584r:2)  0 at 48r 1 at 24r 2 at 28r L00000001 [48r,208r:0)  0 at 48r L00000002 [48r,208r:0)  0 at 48r L00000004 [28r,384r:0)[464B,580r:0)  0 at 28r L00000008 [24r,400r:0)[464B,584r:0)  0 at 24r
   
  0B      BB#0: derived from LLVM BB %entry
              Live Ins: %SGPR0_SGPR1
  16B             %vreg1<def> = COPY %SGPR0_SGPR1; SGPR_64:%vreg1
  24B             %vreg12:sub3<def,read-undef> = S_MOV_B32 61440; SReg_128:%vreg12
  28B             %vreg12:sub2<def> = S_MOV_B32 -1; SReg_128:%vreg12
  48B             %vreg12:sub0_sub1<def> = S_LOAD_DWORDX2_IMM %vreg1, 11, 0; mem:LD8[undef(addrspace=2)](nontemporal)(dereferenceable)(invariant) SReg_128:%vreg12 SGPR_64:%vreg1
  …
  464B    BB#2: derived from LLVM BB %two
              Predecessors according to CFG: BB#0
  576B            %vreg21<def> = V_CVT_F16_F32_e32 %vreg0, %EXEC<imp-use>; VGPR_32:%vreg21,%vreg0
  580B            %vreg20:sub2<def> = COPY %vreg12:sub2; SReg_128:%vreg20,%vreg12
  584B            %vreg20:sub3<def> = COPY %vreg12:sub3; SReg_128:%vreg20,%vreg12

We have switched %vreg12 subreg definitions, so the new liveout from BB#0 is [48r,400r:0), value number 0. BB#2's livein is not updated: [464B,584r:2).
At this point liveout != livein and LIS is broken. Verifier will report the error.


Repository:
  rL LLVM

https://reviews.llvm.org/D30558

Files:
  lib/CodeGen/LiveIntervalAnalysis.cpp


Index: lib/CodeGen/LiveIntervalAnalysis.cpp
===================================================================
--- lib/CodeGen/LiveIntervalAnalysis.cpp
+++ lib/CodeGen/LiveIntervalAnalysis.cpp
@@ -982,10 +982,38 @@
       handleMoveDown(LR);
     else
       handleMoveUp(LR, Reg, LaneMask);
+
+    // Fix liveins to other blocks which may need to be changed to new liveouts.
+    updateSuccessors(LR, LIS.getSlotIndexes()->getMBBFromIndex(NewIdx));
+
     DEBUG(dbgs() << "        -->\t" << LR << '\n');
     LR.verify();
   }
 
+  // Update value numbers in successor blocks in case if we have a new liveout
+  // replacing an old one. I.e. if we have swapped two instructions defining
+  // subregisters of a same register. Last one becomes a new liveout.
+  // Only update Valno if supplied.
+  void updateSuccessors(LiveRange &LR, const MachineBasicBlock *MBB,
+                        VNInfo *Valno = nullptr) {
+    VNInfo *Out = LR.getVNInfoBefore(LIS.getMBBEndIdx(MBB));
+    if (!Out || (Valno && Out != Valno))
+      return;
+    // This is a liveout.
+    // Find live-ins to other blocks which should connect to this live-out.
+    for (const auto &Succ : MBB->successors()) {
+      // Now set new value number in a successor.
+      auto *InSeg = LR.getSegmentContaining(LIS.getMBBStartIdx(Succ));
+      if (InSeg && !InSeg->valno->isPHIDef() && InSeg->valno != Out) {
+        InSeg->valno = Out;
+        // Iterate next successors. No need to maintain visited list because
+        // when we update all possible segments loop will terminate.
+        if (InSeg->contains(LIS.getMBBEndIdx(Succ).getPrevSlot()))
+          updateSuccessors(LR, Succ, Out);
+      }
+    }
+  }
+
   /// Update LR to reflect an instruction has been moved downwards from OldIdx
   /// to NewIdx (OldIdx < NewIdx).
   void handleMoveDown(LiveRange &LR) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D30558.90414.patch
Type: text/x-patch
Size: 1855 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170303/ac376076/attachment.bin>