diff --git a/llvm/include/llvm/CodeGen/LiveIntervals.h b/llvm/include/llvm/CodeGen/LiveIntervals.h --- a/llvm/include/llvm/CodeGen/LiveIntervals.h +++ b/llvm/include/llvm/CodeGen/LiveIntervals.h @@ -139,6 +139,10 @@ return LI; } + LiveInterval &getOrCreateEmptyInterval(Register Reg) { + return hasInterval(Reg) ? getInterval(Reg) : createEmptyInterval(Reg); + } + /// Interval removal. void removeInterval(Register Reg) { delete VirtRegIntervals[Reg]; diff --git a/llvm/lib/CodeGen/LiveIntervals.cpp b/llvm/lib/CodeGen/LiveIntervals.cpp --- a/llvm/lib/CodeGen/LiveIntervals.cpp +++ b/llvm/lib/CodeGen/LiveIntervals.cpp @@ -862,7 +862,7 @@ LiveRange::Segment LiveIntervals::addSegmentToEndOfBlock(Register Reg, MachineInstr &startInst) { - LiveInterval &Interval = createEmptyInterval(Reg); + LiveInterval &Interval = getOrCreateEmptyInterval(Reg); VNInfo *VN = Interval.getNextValue( SlotIndex(getInstructionIndex(startInst).getRegSlot()), getVNInfoAllocator()); diff --git a/llvm/lib/CodeGen/MachineBasicBlock.cpp b/llvm/lib/CodeGen/MachineBasicBlock.cpp --- a/llvm/lib/CodeGen/MachineBasicBlock.cpp +++ b/llvm/lib/CodeGen/MachineBasicBlock.cpp @@ -1166,32 +1166,29 @@ ReplaceUsesOfBlockWith(Succ, NMBB); - // If updateTerminator() removes instructions, we need to remove them from - // SlotIndexes. - SmallVector Terminators; - if (Indexes) { - for (MachineInstr &MI : - llvm::make_range(getFirstInstrTerminator(), instr_end())) - Terminators.push_back(&MI); - } + if (!ChangedIndirectJump) { + // If updateTerminator() removes instructions they need to be removed from + // SlotIndexes, but we do not know which will be removed, so we have to + // remove all pre-emptively as their pointers will no longer be valid. + SmallVector Terminators; + if (Indexes) { + for (MachineInstr &MI : + llvm::make_range(getFirstInstrTerminator(), instr_end())) + Indexes->removeMachineInstrFromMaps(MI); + } - // Since we replaced all uses of Succ with NMBB, that should also be treated - // as the fallthrough successor - if (Succ == PrevFallthrough) - PrevFallthrough = NMBB; + // Since we replaced all uses of Succ with NMBB, that should also be treated + // as the fallthrough successor + if (Succ == PrevFallthrough) + PrevFallthrough = NMBB; - if (!ChangedIndirectJump) updateTerminator(PrevFallthrough); - if (Indexes) { - SmallVector NewTerminators; - for (MachineInstr &MI : - llvm::make_range(getFirstInstrTerminator(), instr_end())) - NewTerminators.push_back(&MI); - - for (MachineInstr *Terminator : Terminators) { - if (!is_contained(NewTerminators, Terminator)) - Indexes->removeMachineInstrFromMaps(*Terminator); + // Reinsert terminators into maps. + if (Indexes) { + for (MachineInstr &MI : + llvm::make_range(getFirstInstrTerminator(), instr_end())) + Indexes->insertMachineInstrInMaps(MI); } } @@ -1275,6 +1272,8 @@ assert(VNI && "PHI sources should be live out of their predecessors."); LI.addSegment(LiveInterval::Segment(StartIndex, EndIndex, VNI)); + for (auto &SR : LI.subranges()) + SR.addSegment(LiveInterval::Segment(StartIndex, EndIndex, VNI)); } } } @@ -1294,8 +1293,18 @@ VNInfo *VNI = LI.getVNInfoAt(PrevIndex); assert(VNI && "LiveInterval should have VNInfo where it is live."); LI.addSegment(LiveInterval::Segment(StartIndex, EndIndex, VNI)); + // Update subranges with live values + for (auto &SR : LI.subranges()) { + VNInfo *VNI = SR.getVNInfoAt(PrevIndex); + if (VNI) + SR.addSegment(LiveInterval::Segment(StartIndex, EndIndex, VNI)); + } } else if (!isLiveOut && !isLastMBB) { LI.removeSegment(StartIndex, EndIndex); + for (auto &SR : LI.subranges()) { + if (SR.overlaps(StartIndex, EndIndex)) + SR.removeSegment(StartIndex, EndIndex); + } } } diff --git a/llvm/lib/CodeGen/PHIElimination.cpp b/llvm/lib/CodeGen/PHIElimination.cpp --- a/llvm/lib/CodeGen/PHIElimination.cpp +++ b/llvm/lib/CodeGen/PHIElimination.cpp @@ -136,6 +136,7 @@ void PHIElimination::getAnalysisUsage(AnalysisUsage &AU) const { AU.addUsedIfAvailable(); + AU.addUsedIfAvailable(); AU.addPreserved(); AU.addPreserved(); AU.addPreserved(); @@ -392,7 +393,7 @@ if (IncomingReg) { // Add the region from the beginning of MBB to the copy instruction to // IncomingReg's live interval. - LiveInterval &IncomingLI = LIS->createEmptyInterval(IncomingReg); + LiveInterval &IncomingLI = LIS->getOrCreateEmptyInterval(IncomingReg); VNInfo *IncomingVNI = IncomingLI.getVNInfoAt(MBBStartIndex); if (!IncomingVNI) IncomingVNI = IncomingLI.getNextValue(MBBStartIndex, @@ -403,24 +404,49 @@ } 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 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. + VNInfo *VNI = LR->getVNInfoAt(DestSegment->start); + assert(VNI && "value should be defined for known segment"); + LR->addSegment(LiveInterval::Segment( + NewStart, DestSegment->start, VNI)); + } else if (DestSegment->start < NewStart) { + // Otherwise, remove the region from the beginning of MBB to the copy + // instruction from DestReg's live interval. + assert(DestSegment->start >= MBBStartIndex); + assert(DestSegment->end >= DestCopyIndex.getRegSlot()); + LR->removeSegment(DestSegment->start, NewStart); + } + VNInfo *DestVNI = LR->getVNInfoAt(NewStart); assert(DestVNI && "PHI destination should be live at its definition."); - DestVNI->def = DestCopyIndex.getRegSlot(); + DestVNI->def = NewStart; } } @@ -615,6 +641,10 @@ SlotIndex LastUseIndex = LIS->getInstructionIndex(*KillInst); SrcLI.removeSegment(LastUseIndex.getRegSlot(), LIS->getMBBEndIdx(&opBlock)); + for (auto &SR : SrcLI.subranges()) { + SR.removeSegment(LastUseIndex.getRegSlot(), + LIS->getMBBEndIdx(&opBlock)); + } } } } diff --git a/llvm/test/CodeGen/AMDGPU/split-mbb-lis-subrange.mir b/llvm/test/CodeGen/AMDGPU/split-mbb-lis-subrange.mir --- a/llvm/test/CodeGen/AMDGPU/split-mbb-lis-subrange.mir +++ b/llvm/test/CodeGen/AMDGPU/split-mbb-lis-subrange.mir @@ -1,7 +1,7 @@ # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 2 -# RUN: llc -march=amdgcn -mcpu=gfx1100 -verify-machineinstrs -run-pass liveintervals -o - %s | FileCheck -check-prefixes=GCN %s +# RUN: llc -march=amdgcn -mcpu=gfx1100 -verify-machineinstrs -run-pass liveintervals,phi-node-elimination -o - %s | FileCheck -check-prefixes=GCN %s -# This test simply checks that liveintervals pass verification. +# This checks liveintervals pass verification and phi-node-elimination correctly preserves them. --- name: split_critical_edge_subranges @@ -9,7 +9,7 @@ body: | ; GCN-LABEL: name: split_critical_edge_subranges ; GCN: bb.0: - ; GCN-NEXT: successors: %bb.3(0x40000000), %bb.1(0x40000000) + ; GCN-NEXT: successors: %bb.5(0x40000000), %bb.1(0x40000000) ; GCN-NEXT: {{ $}} ; GCN-NEXT: %coord:vreg_64 = IMPLICIT_DEF ; GCN-NEXT: %desc:sgpr_256 = IMPLICIT_DEF @@ -20,14 +20,22 @@ ; GCN-NEXT: %s0a:vgpr_32 = COPY %load.sub0 ; GCN-NEXT: %s0b:vgpr_32 = COPY %load.sub1 ; GCN-NEXT: S_CMP_EQ_U32 %c0, %c1, implicit-def $scc - ; GCN-NEXT: S_CBRANCH_SCC1 %bb.3, implicit $scc - ; GCN-NEXT: S_BRANCH %bb.1 + ; GCN-NEXT: S_CBRANCH_SCC0 %bb.1, implicit $scc + ; GCN-NEXT: {{ $}} + ; GCN-NEXT: bb.5: + ; GCN-NEXT: successors: %bb.3(0x80000000) + ; GCN-NEXT: {{ $}} + ; GCN-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY %s0a + ; GCN-NEXT: [[COPY1:%[0-9]+]]:vgpr_32 = COPY %s0b + ; GCN-NEXT: S_BRANCH %bb.3 ; GCN-NEXT: {{ $}} ; GCN-NEXT: bb.1: ; GCN-NEXT: successors: %bb.3(0x80000000) ; GCN-NEXT: {{ $}} ; GCN-NEXT: %s0c:vgpr_32 = V_ADD_F32_e64 0, %s0a, 0, %const, 0, 0, implicit $mode, implicit $exec ; GCN-NEXT: %s0d:vgpr_32 = V_ADD_F32_e64 0, %s0b, 0, %const, 0, 0, implicit $mode, implicit $exec + ; GCN-NEXT: [[COPY2:%[0-9]+]]:vgpr_32 = COPY %s0c + ; GCN-NEXT: [[COPY3:%[0-9]+]]:vgpr_32 = COPY %s0d ; GCN-NEXT: S_BRANCH %bb.3 ; GCN-NEXT: {{ $}} ; GCN-NEXT: bb.2: @@ -37,8 +45,8 @@ ; GCN-NEXT: bb.3: ; GCN-NEXT: successors: %bb.4(0x80000000) ; GCN-NEXT: {{ $}} - ; GCN-NEXT: %phi0:vgpr_32 = PHI %s0a, %bb.0, %s0c, %bb.1 - ; GCN-NEXT: %phi1:vgpr_32 = PHI %s0b, %bb.0, %s0d, %bb.1 + ; GCN-NEXT: %phi1:vgpr_32 = COPY [[COPY3]] + ; GCN-NEXT: %phi0:vgpr_32 = COPY [[COPY2]] ; GCN-NEXT: S_BRANCH %bb.4 ; GCN-NEXT: {{ $}} ; GCN-NEXT: bb.4: