[llvm] [PHIElimination] Handle subranges in LiveInterval updates (PR #69429)

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 23 17:08:01 PDT 2023


================
@@ -400,24 +400,50 @@ void PHIElimination::LowerPHINode(MachineBasicBlock &MBB,
     }
 
     LiveInterval &DestLI = LIS->getInterval(DestReg);
-    assert(!DestLI.empty() && "PHIs should have nonempty LiveIntervals.");
-    if (DestLI.endIndex().isDead()) {
-      // A dead PHI's live range begins and ends at the start of the MBB, but
-      // the lowered copy, which will still be dead, needs to begin and end at
-      // the copy instruction.
-      VNInfo *OrigDestVNI = DestLI.getVNInfoAt(MBBStartIndex);
-      assert(OrigDestVNI && "PHI destination should be live at block entry.");
-      DestLI.removeSegment(MBBStartIndex, MBBStartIndex.getDeadSlot());
-      DestLI.createDeadDef(DestCopyIndex.getRegSlot(),
-                           LIS->getVNInfoAllocator());
-      DestLI.removeValNo(OrigDestVNI);
-    } else {
-      // Otherwise, remove the region from the beginning of MBB to the copy
-      // instruction from DestReg's live interval.
-      DestLI.removeSegment(MBBStartIndex, DestCopyIndex.getRegSlot());
-      VNInfo *DestVNI = DestLI.getVNInfoAt(DestCopyIndex.getRegSlot());
+    assert(!DestLI.empty() && "PHIs should have non-empty LiveIntervals.");
+
+    SlotIndex NewStart = DestCopyIndex.getRegSlot();
+
+    SmallVector<LiveRange *> ToUpdate;
+    ToUpdate.push_back(&DestLI);
+    for (auto &SR : DestLI.subranges())
+      ToUpdate.push_back(&SR);
+
+    for (auto LR : ToUpdate) {
+      auto DestSegment = LR->find(MBBStartIndex);
+      assert(DestSegment != LR->end() &&
+             "PHI destination must be live in block");
+
+      if (LR->endIndex().isDead()) {
+        // A dead PHI's live range begins and ends at the start of the MBB, but
+        // the lowered copy, which will still be dead, needs to begin and end at
+        // the copy instruction.
+        VNInfo *OrigDestVNI = LR->getVNInfoAt(DestSegment->start);
+        assert(OrigDestVNI && "PHI destination should be live at block entry.");
+        LR->removeSegment(DestSegment->start, DestSegment->start.getDeadSlot());
+        LR->createDeadDef(NewStart, LIS->getVNInfoAllocator());
+        LR->removeValNo(OrigDestVNI);
+        continue;
+      }
+
+      if (DestSegment->start > NewStart) {
+        // With a single PHI removed from block the index of the copy may be
+        // lower than the original PHI. Extend live range backward to cover
+        // the copy.
----------------
MatzeB wrote:

I find this comment highly confusing, and have a hard time seeing when this condition would ever be true... There was a PHI definition `DestReg` here, no? That should mean that every segment here should have been defined at `MBBStartIndex` (where PHI definitions take effect), no? (Except maybe if a subrange was undefined, but then we would have already entered the `isDead()` case above...)

https://github.com/llvm/llvm-project/pull/69429


More information about the llvm-commits mailing list