[PATCH] D82916: LIS: fix handleMove to properly extend main range

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec created this revision.
rampitec added reviewers: arsenm, kparzysz, MatzeB, tpr.
Herald added subscribers: hiraditya, wdng.
Herald added a project: LLVM.
rampitec added a comment.

If there are better ideas I would be happy to hear!


rampitec added a comment.

I also believe that will happen extremely rare, otherwise we would have already hit this problem. So the main overhead is really covers() checks.


handleMoveDown or handleMoveUp cannot properly repair a main
range of a LiveInterval since they only get LiveRange. There
is a problem if certain use has moved few segments away and
there is a hole in the main range in between of these two
locations. We may get a SubRange with a very extended Segment
spanning several Segments of the main range and also spanning
that hole. If that happens then we end up with the main range
not covering its SubRange which is an error.

It might be possible to attempt fixing the main range in place
just between of the old and new index by extending all of its
Segments in between, but it is unclear this logic will be
faster than just straight constructMainRangeFromSubranges,
which itself is pretty cheap since it only contains interval
logic. That will also require shrinkToUses() call after which
is probably even more expensive.

In the test second move is from 64B to 92B for the sub1.
Subrange is correctly fixed:

L000000000000000C [16r,32B:0)[32B,92r:1)  0 at 16r 1 at 32B-phi

But the main range has a hole in between 80d and 88r after
updateRange():

%1 [16r,32B:0)[32B,80r:4)[80r,80d:3)[88r,96r:1)[96r,160B:2)

Since source position is 64B this segment is not even considered
by the updateRange().


https://reviews.llvm.org/D82916

Files:
  llvm/lib/CodeGen/LiveIntervals.cpp
  llvm/unittests/MI/LiveIntervalTest.cpp


Index: llvm/unittests/MI/LiveIntervalTest.cpp
===================================================================
--- llvm/unittests/MI/LiveIntervalTest.cpp
+++ llvm/unittests/MI/LiveIntervalTest.cpp
@@ -499,6 +499,26 @@
   });
 }
 
+TEST(LiveIntervalTest, TestMoveSubRegUseAcrossMainRangeHole) {
+  liveIntervalTest(R"MIR(
+    %1:sgpr_128 = IMPLICIT_DEF
+  bb.1:
+    %2:sgpr_32 = COPY %1.sub2
+    %3:sgpr_32 = COPY %1.sub1
+    %1.sub2 = COPY %2
+    undef %1.sub0 = IMPLICIT_DEF
+    %1.sub2 = IMPLICIT_DEF
+    S_CBRANCH_SCC1 %bb.1, implicit undef $scc
+    S_BRANCH %bb.2
+  bb.2:
+)MIR", [](MachineFunction &MF, LiveIntervals &LIS) {
+    MachineInstr &MI = getMI(MF, 3, /*BlockNum=*/1);
+    MI.getOperand(0).setIsUndef(false);
+    testHandleMove(MF, LIS, 4, 3, 1);
+    testHandleMove(MF, LIS, 1, 4, 1);
+  });
+}
+
 TEST(LiveIntervalTest, BundleUse) {
   liveIntervalTest(R"MIR(
     %0 = IMPLICIT_DEF
Index: llvm/lib/CodeGen/LiveIntervals.cpp
===================================================================
--- llvm/lib/CodeGen/LiveIntervals.cpp
+++ llvm/lib/CodeGen/LiveIntervals.cpp
@@ -1011,6 +1011,14 @@
           }
         }
         updateRange(LI, Reg, LaneBitmask::getNone());
+        for (LiveInterval::SubRange &S : LI.subranges()) {
+          if (LI.covers(S))
+            continue;
+          LI.clear();
+          LIS.constructMainRangeFromSubranges(LI);
+          break;
+        }
+
         continue;
       }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D82916.274624.patch
Type: text/x-patch
Size: 1452 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200630/231e8ade/attachment.bin>