Skip to content

Commit f106b3f

Browse files
vitalybukavitalybuka
committed
Revert "[PHIElimination] Handle subranges in LiveInterval updates"
Leaks memory. This reverts commit 3bff611.
1 parent eb81493 commit f106b3f

File tree

5 files changed

+26
-80
lines changed

5 files changed

+26
-80
lines changed

‎llvm/include/llvm/CodeGen/LiveIntervals.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,6 @@ class VirtRegMap;
139139
return LI;
140140
}
141141

142-
LiveInterval &getOrCreateEmptyInterval(Register Reg) {
143-
return hasInterval(Reg) ? getInterval(Reg) : createEmptyInterval(Reg);
144-
}
145-
146142
/// Interval removal.
147143
void removeInterval(Register Reg) {
148144
delete VirtRegIntervals[Reg];

‎llvm/lib/CodeGen/LiveIntervals.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -862,7 +862,7 @@ float LiveIntervals::getSpillWeight(bool isDef, bool isUse,
862862

863863
LiveRange::Segment
864864
LiveIntervals::addSegmentToEndOfBlock(Register Reg, MachineInstr &startInst) {
865-
LiveInterval &Interval = getOrCreateEmptyInterval(Reg);
865+
LiveInterval &Interval = createEmptyInterval(Reg);
866866
VNInfo *VN = Interval.getNextValue(
867867
SlotIndex(getInstructionIndex(startInst).getRegSlot()),
868868
getVNInfoAllocator());

‎llvm/lib/CodeGen/MachineBasicBlock.cpp‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1275,8 +1275,6 @@ MachineBasicBlock *MachineBasicBlock::SplitCriticalEdge(
12751275
assert(VNI &&
12761276
"PHI sources should be live out of their predecessors.");
12771277
LI.addSegment(LiveInterval::Segment(StartIndex, EndIndex, VNI));
1278-
for (auto &SR : LI.subranges())
1279-
SR.addSegment(LiveInterval::Segment(StartIndex, EndIndex, VNI));
12801278
}
12811279
}
12821280
}
@@ -1296,18 +1294,8 @@ MachineBasicBlock *MachineBasicBlock::SplitCriticalEdge(
12961294
VNInfo *VNI = LI.getVNInfoAt(PrevIndex);
12971295
assert(VNI && "LiveInterval should have VNInfo where it is live.");
12981296
LI.addSegment(LiveInterval::Segment(StartIndex, EndIndex, VNI));
1299-
// Update subranges with live values
1300-
for (auto &SR : LI.subranges()) {
1301-
VNInfo *VNI = SR.getVNInfoAt(PrevIndex);
1302-
if (VNI)
1303-
SR.addSegment(LiveInterval::Segment(StartIndex, EndIndex, VNI));
1304-
}
13051297
} else if (!isLiveOut && !isLastMBB) {
13061298
LI.removeSegment(StartIndex, EndIndex);
1307-
for (auto &SR : LI.subranges()) {
1308-
if (SR.overlaps(StartIndex, EndIndex))
1309-
SR.removeSegment(StartIndex, EndIndex);
1310-
}
13111299
}
13121300
}
13131301

‎llvm/lib/CodeGen/PHIElimination.cpp‎

Lines changed: 18 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ INITIALIZE_PASS_END(PHIElimination, DEBUG_TYPE,
136136

137137
void PHIElimination::getAnalysisUsage(AnalysisUsage &AU) const {
138138
AU.addUsedIfAvailable<LiveVariables>();
139-
AU.addUsedIfAvailable<LiveIntervals>();
140139
AU.addPreserved<LiveVariables>();
141140
AU.addPreserved<SlotIndexes>();
142141
AU.addPreserved<LiveIntervals>();
@@ -393,7 +392,7 @@ void PHIElimination::LowerPHINode(MachineBasicBlock &MBB,
393392
if (IncomingReg) {
394393
// Add the region from the beginning of MBB to the copy instruction to
395394
// IncomingReg's live interval.
396-
LiveInterval &IncomingLI = LIS->getOrCreateEmptyInterval(IncomingReg);
395+
LiveInterval &IncomingLI = LIS->createEmptyInterval(IncomingReg);
397396
VNInfo *IncomingVNI = IncomingLI.getVNInfoAt(MBBStartIndex);
398397
if (!IncomingVNI)
399398
IncomingVNI = IncomingLI.getNextValue(MBBStartIndex,
@@ -404,49 +403,24 @@ void PHIElimination::LowerPHINode(MachineBasicBlock &MBB,
404403
}
405404

406405
LiveInterval &DestLI = LIS->getInterval(DestReg);
407-
assert(!DestLI.empty() && "PHIs should have non-empty LiveIntervals.");
408-
409-
SlotIndex NewStart = DestCopyIndex.getRegSlot();
410-
411-
SmallVector<LiveRange*> ToUpdate;
412-
ToUpdate.push_back(&DestLI);
413-
for (auto &SR : DestLI.subranges())
414-
ToUpdate.push_back(&SR);
415-
416-
for (auto LR : ToUpdate) {
417-
auto DestSegment = LR->find(MBBStartIndex);
418-
assert(DestSegment != LR->end() && "PHI destination must be live in block");
419-
420-
if (LR->endIndex().isDead()) {
421-
// A dead PHI's live range begins and ends at the start of the MBB, but
422-
// the lowered copy, which will still be dead, needs to begin and end at
423-
// the copy instruction.
424-
VNInfo *OrigDestVNI = LR->getVNInfoAt(DestSegment->start);
425-
assert(OrigDestVNI && "PHI destination should be live at block entry.");
426-
LR->removeSegment(DestSegment->start, DestSegment->start.getDeadSlot());
427-
LR->createDeadDef(NewStart, LIS->getVNInfoAllocator());
428-
LR->removeValNo(OrigDestVNI);
429-
continue;
430-
}
431-
432-
if (DestSegment->start > NewStart) {
433-
// With a single PHI removed from block the index of the copy may be
434-
// lower than the original PHI. Extend live range backward to cover
435-
// the copy.
436-
VNInfo *VNI = LR->getVNInfoAt(DestSegment->start);
437-
assert(VNI && "value should be defined for known segment");
438-
LR->addSegment(LiveInterval::Segment(
439-
NewStart, DestSegment->start, VNI));
440-
} else if (DestSegment->start < NewStart) {
441-
// Otherwise, remove the region from the beginning of MBB to the copy
442-
// instruction from DestReg's live interval.
443-
assert(DestSegment->start >= MBBStartIndex);
444-
assert(DestSegment->end >= DestCopyIndex.getRegSlot());
445-
LR->removeSegment(DestSegment->start, NewStart);
446-
}
447-
VNInfo *DestVNI = LR->getVNInfoAt(NewStart);
406+
assert(!DestLI.empty() && "PHIs should have nonempty LiveIntervals.");
407+
if (DestLI.endIndex().isDead()) {
408+
// A dead PHI's live range begins and ends at the start of the MBB, but
409+
// the lowered copy, which will still be dead, needs to begin and end at
410+
// the copy instruction.
411+
VNInfo *OrigDestVNI = DestLI.getVNInfoAt(MBBStartIndex);
412+
assert(OrigDestVNI && "PHI destination should be live at block entry.");
413+
DestLI.removeSegment(MBBStartIndex, MBBStartIndex.getDeadSlot());
414+
DestLI.createDeadDef(DestCopyIndex.getRegSlot(),
415+
LIS->getVNInfoAllocator());
416+
DestLI.removeValNo(OrigDestVNI);
417+
} else {
418+
// Otherwise, remove the region from the beginning of MBB to the copy
419+
// instruction from DestReg's live interval.
420+
DestLI.removeSegment(MBBStartIndex, DestCopyIndex.getRegSlot());
421+
VNInfo *DestVNI = DestLI.getVNInfoAt(DestCopyIndex.getRegSlot());
448422
assert(DestVNI && "PHI destination should be live at its definition.");
449-
DestVNI->def = NewStart;
423+
DestVNI->def = DestCopyIndex.getRegSlot();
450424
}
451425
}
452426

@@ -641,10 +615,6 @@ void PHIElimination::LowerPHINode(MachineBasicBlock &MBB,
641615
SlotIndex LastUseIndex = LIS->getInstructionIndex(*KillInst);
642616
SrcLI.removeSegment(LastUseIndex.getRegSlot(),
643617
LIS->getMBBEndIdx(&opBlock));
644-
for (auto &SR : SrcLI.subranges()) {
645-
SR.removeSegment(LastUseIndex.getRegSlot(),
646-
LIS->getMBBEndIdx(&opBlock));
647-
}
648618
}
649619
}
650620
}

‎llvm/test/CodeGen/AMDGPU/split-mbb-lis-subrange.mir‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 2
2-
# RUN: llc -march=amdgcn -mcpu=gfx1100 -verify-machineinstrs -run-pass liveintervals,phi-node-elimination -o - %s | FileCheck -check-prefixes=GCN %s
2+
# RUN: llc -march=amdgcn -mcpu=gfx1100 -verify-machineinstrs -run-pass liveintervals -o - %s | FileCheck -check-prefixes=GCN %s
33

4-
# This checks liveintervals pass verification and phi-node-elimination correctly preserves them.
4+
# This test simply checks that liveintervals pass verification.
55

66
---
77
name: split_critical_edge_subranges
88
tracksRegLiveness: true
99
body: |
1010
; GCN-LABEL: name: split_critical_edge_subranges
1111
; GCN: bb.0:
12-
; GCN-NEXT: successors: %bb.5(0x40000000), %bb.1(0x40000000)
12+
; GCN-NEXT: successors: %bb.3(0x40000000), %bb.1(0x40000000)
1313
; GCN-NEXT: {{ $}}
1414
; GCN-NEXT: %coord:vreg_64 = IMPLICIT_DEF
1515
; GCN-NEXT: %desc:sgpr_256 = IMPLICIT_DEF
@@ -20,22 +20,14 @@ body: |
2020
; GCN-NEXT: %s0a:vgpr_32 = COPY %load.sub0
2121
; GCN-NEXT: %s0b:vgpr_32 = COPY %load.sub1
2222
; GCN-NEXT: S_CMP_EQ_U32 %c0, %c1, implicit-def $scc
23-
; GCN-NEXT: S_CBRANCH_SCC0 %bb.1, implicit $scc
24-
; GCN-NEXT: {{ $}}
25-
; GCN-NEXT: bb.5:
26-
; GCN-NEXT: successors: %bb.3(0x80000000)
27-
; GCN-NEXT: {{ $}}
28-
; GCN-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY %s0a
29-
; GCN-NEXT: [[COPY1:%[0-9]+]]:vgpr_32 = COPY %s0b
30-
; GCN-NEXT: S_BRANCH %bb.3
23+
; GCN-NEXT: S_CBRANCH_SCC1 %bb.3, implicit $scc
24+
; GCN-NEXT: S_BRANCH %bb.1
3125
; GCN-NEXT: {{ $}}
3226
; GCN-NEXT: bb.1:
3327
; GCN-NEXT: successors: %bb.3(0x80000000)
3428
; GCN-NEXT: {{ $}}
3529
; GCN-NEXT: %s0c:vgpr_32 = V_ADD_F32_e64 0, %s0a, 0, %const, 0, 0, implicit $mode, implicit $exec
3630
; GCN-NEXT: %s0d:vgpr_32 = V_ADD_F32_e64 0, %s0b, 0, %const, 0, 0, implicit $mode, implicit $exec
37-
; GCN-NEXT: [[COPY2:%[0-9]+]]:vgpr_32 = COPY %s0c
38-
; GCN-NEXT: [[COPY3:%[0-9]+]]:vgpr_32 = COPY %s0d
3931
; GCN-NEXT: S_BRANCH %bb.3
4032
; GCN-NEXT: {{ $}}
4133
; GCN-NEXT: bb.2:
@@ -45,8 +37,8 @@ body: |
4537
; GCN-NEXT: bb.3:
4638
; GCN-NEXT: successors: %bb.4(0x80000000)
4739
; GCN-NEXT: {{ $}}
48-
; GCN-NEXT: %phi1:vgpr_32 = COPY [[COPY3]]
49-
; GCN-NEXT: %phi0:vgpr_32 = COPY [[COPY2]]
40+
; GCN-NEXT: %phi0:vgpr_32 = PHI %s0a, %bb.0, %s0c, %bb.1
41+
; GCN-NEXT: %phi1:vgpr_32 = PHI %s0b, %bb.0, %s0d, %bb.1
5042
; GCN-NEXT: S_BRANCH %bb.4
5143
; GCN-NEXT: {{ $}}
5244
; GCN-NEXT: bb.4:

0 commit comments

Comments
 (0)