Skip to content

Commit 6b1db79

Browse files
committed
Revert "Reland "RegisterCoalescer: Add implicit-def of super register when coalescing SUBREG_TO_REG" (#123632)"
There's a regression with one of the bootstrap builds for x86. I'll revert this while I investigate. This reverts commit 4df6d3d.
1 parent 974f678 commit 6b1db79

24 files changed

+154
-1263
lines changed

llvm/lib/CodeGen/RegisterCoalescer.cpp

Lines changed: 16 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -306,11 +306,7 @@ namespace {
306306
/// number if it is not zero. If DstReg is a physical register and the
307307
/// existing subregister number of the def / use being updated is not zero,
308308
/// make sure to set it to the correct physical subregister.
309-
///
310-
/// If \p IsSubregToReg, we are coalescing a DstReg = SUBREG_TO_REG
311-
/// SrcReg. This introduces an implicit-def of DstReg on coalesced users.
312-
void updateRegDefsUses(Register SrcReg, Register DstReg, unsigned SubIdx,
313-
bool IsSubregToReg);
309+
void updateRegDefsUses(Register SrcReg, Register DstReg, unsigned SubIdx);
314310

315311
/// If the given machine operand reads only undefined lanes add an undef
316312
/// flag.
@@ -1434,7 +1430,6 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
14341430

14351431
// CopyMI may have implicit operands, save them so that we can transfer them
14361432
// over to the newly materialized instruction after CopyMI is removed.
1437-
LaneBitmask NewMIImplicitOpsMask;
14381433
SmallVector<MachineOperand, 4> ImplicitOps;
14391434
ImplicitOps.reserve(CopyMI->getNumOperands() -
14401435
CopyMI->getDesc().getNumOperands());
@@ -1448,9 +1443,6 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
14481443
(MO.getSubReg() == 0 && MO.getReg() == DstOperand.getReg())) &&
14491444
"unexpected implicit virtual register def");
14501445
ImplicitOps.push_back(MO);
1451-
if (MO.isDef() && MO.getReg().isVirtual() &&
1452-
MRI->shouldTrackSubRegLiveness(DstReg))
1453-
NewMIImplicitOpsMask |= MRI->getMaxLaneMaskForVReg(MO.getReg());
14541446
}
14551447
}
14561448

@@ -1493,11 +1485,14 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
14931485
} else {
14941486
assert(MO.getReg() == NewMI.getOperand(0).getReg());
14951487

1496-
// If lanemasks need to be tracked, compile the lanemask of the NewMI
1497-
// implicit def operands to avoid subranges for the super-regs from
1498-
// being removed by code later on in this function.
1499-
if (MRI->shouldTrackSubRegLiveness(MO.getReg()))
1500-
NewMIImplicitOpsMask |= MRI->getMaxLaneMaskForVReg(MO.getReg());
1488+
// We're only expecting another def of the main output, so the range
1489+
// should get updated with the regular output range.
1490+
//
1491+
// FIXME: The range updating below probably needs updating to look at
1492+
// the super register if subranges are tracked.
1493+
assert(!MRI->shouldTrackSubRegLiveness(DstReg) &&
1494+
"subrange update for implicit-def of super register may not be "
1495+
"properly handled");
15011496
}
15021497
}
15031498
}
@@ -1521,7 +1516,7 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
15211516
MRI->setRegClass(DstReg, NewRC);
15221517

15231518
// Update machine operands and add flags.
1524-
updateRegDefsUses(DstReg, DstReg, DstIdx, false);
1519+
updateRegDefsUses(DstReg, DstReg, DstIdx);
15251520
NewMI.getOperand(0).setSubReg(NewIdx);
15261521
// updateRegDefUses can add an "undef" flag to the definition, since
15271522
// it will replace DstReg with DstReg.DstIdx. If NewIdx is 0, make
@@ -1597,8 +1592,7 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
15971592
CurrIdx.getRegSlot(NewMI.getOperand(0).isEarlyClobber());
15981593
VNInfo::Allocator &Alloc = LIS->getVNInfoAllocator();
15991594
for (LiveInterval::SubRange &SR : DstInt.subranges()) {
1600-
if ((SR.LaneMask & DstMask).none() &&
1601-
(SR.LaneMask & NewMIImplicitOpsMask).none()) {
1595+
if ((SR.LaneMask & DstMask).none()) {
16021596
LLVM_DEBUG(dbgs()
16031597
<< "Removing undefined SubRange "
16041598
<< PrintLaneMask(SR.LaneMask) << " : " << SR << "\n");
@@ -1863,7 +1857,7 @@ void RegisterCoalescer::addUndefFlag(const LiveInterval &Int, SlotIndex UseIdx,
18631857
}
18641858

18651859
void RegisterCoalescer::updateRegDefsUses(Register SrcReg, Register DstReg,
1866-
unsigned SubIdx, bool IsSubregToReg) {
1860+
unsigned SubIdx) {
18671861
bool DstIsPhys = DstReg.isPhysical();
18681862
LiveInterval *DstInt = DstIsPhys ? nullptr : &LIS->getInterval(DstReg);
18691863

@@ -1883,14 +1877,6 @@ void RegisterCoalescer::updateRegDefsUses(Register SrcReg, Register DstReg,
18831877
}
18841878
}
18851879

1886-
// If DstInt already has a subrange for the unused lanes, then we shouldn't
1887-
// create duplicate subranges when we update the interval for unused lanes.
1888-
LaneBitmask DefinedLanes;
1889-
if (DstInt && MRI->shouldTrackSubRegLiveness(DstReg)) {
1890-
for (LiveInterval::SubRange &SR : DstInt->subranges())
1891-
DefinedLanes |= SR.LaneMask;
1892-
}
1893-
18941880
SmallPtrSet<MachineInstr*, 8> Visited;
18951881
for (MachineRegisterInfo::reg_instr_iterator
18961882
I = MRI->reg_instr_begin(SrcReg), E = MRI->reg_instr_end();
@@ -1914,19 +1900,15 @@ void RegisterCoalescer::updateRegDefsUses(Register SrcReg, Register DstReg,
19141900
if (DstInt && !Reads && SubIdx && !UseMI->isDebugInstr())
19151901
Reads = DstInt->liveAt(LIS->getInstructionIndex(*UseMI));
19161902

1917-
bool FullDef = true;
1918-
19191903
// Replace SrcReg with DstReg in all UseMI operands.
19201904
for (unsigned Op : Ops) {
19211905
MachineOperand &MO = UseMI->getOperand(Op);
19221906

19231907
// Adjust <undef> flags in case of sub-register joins. We don't want to
19241908
// turn a full def into a read-modify-write sub-register def and vice
19251909
// versa.
1926-
if (SubIdx && MO.isDef()) {
1910+
if (SubIdx && MO.isDef())
19271911
MO.setIsUndef(!Reads);
1928-
FullDef = false;
1929-
}
19301912

19311913
// A subreg use of a partially undef (super) register may be a complete
19321914
// undef use now and then has to be marked that way.
@@ -1959,32 +1941,6 @@ void RegisterCoalescer::updateRegDefsUses(Register SrcReg, Register DstReg,
19591941
MO.substVirtReg(DstReg, SubIdx, *TRI);
19601942
}
19611943

1962-
if (IsSubregToReg && !FullDef) {
1963-
// If the coalesed instruction doesn't fully define the register, we need
1964-
// to preserve the original super register liveness for SUBREG_TO_REG.
1965-
//
1966-
// We pretended SUBREG_TO_REG was a regular copy for coalescing purposes,
1967-
// but it introduces liveness for other subregisters. Downstream users may
1968-
// have been relying on those bits, so we need to ensure their liveness is
1969-
// captured with a def of other lanes.
1970-
1971-
if (DstInt && MRI->shouldTrackSubRegLiveness(DstReg)) {
1972-
assert(DstInt->hasSubRanges() &&
1973-
"SUBREG_TO_REG should have resulted in subrange");
1974-
LaneBitmask DstMask = MRI->getMaxLaneMaskForVReg(DstInt->reg());
1975-
LaneBitmask UsedLanes = TRI->getSubRegIndexLaneMask(SubIdx);
1976-
LaneBitmask UnusedLanes = DstMask & ~UsedLanes & ~DefinedLanes;
1977-
if ((UnusedLanes).any()) {
1978-
BumpPtrAllocator &Allocator = LIS->getVNInfoAllocator();
1979-
DstInt->createSubRangeFrom(Allocator, UnusedLanes, *DstInt);
1980-
DefinedLanes |= UnusedLanes;
1981-
}
1982-
}
1983-
1984-
MachineInstrBuilder MIB(*MF, UseMI);
1985-
MIB.addReg(DstReg, RegState::ImplicitDefine);
1986-
}
1987-
19881944
LLVM_DEBUG({
19891945
dbgs() << "\t\tupdated: ";
19901946
if (!UseMI->isDebugInstr())
@@ -2186,8 +2142,6 @@ bool RegisterCoalescer::joinCopy(
21862142
});
21872143
}
21882144

2189-
const bool IsSubregToReg = CopyMI->isSubregToReg();
2190-
21912145
ShrinkMask = LaneBitmask::getNone();
21922146
ShrinkMainRange = false;
21932147

@@ -2257,12 +2211,9 @@ bool RegisterCoalescer::joinCopy(
22572211

22582212
// Rewrite all SrcReg operands to DstReg.
22592213
// Also update DstReg operands to include DstIdx if it is set.
2260-
if (CP.getDstIdx()) {
2261-
assert(!IsSubregToReg && "can this happen?");
2262-
updateRegDefsUses(CP.getDstReg(), CP.getDstReg(), CP.getDstIdx(), false);
2263-
}
2264-
updateRegDefsUses(CP.getSrcReg(), CP.getDstReg(), CP.getSrcIdx(),
2265-
IsSubregToReg);
2214+
if (CP.getDstIdx())
2215+
updateRegDefsUses(CP.getDstReg(), CP.getDstReg(), CP.getDstIdx());
2216+
updateRegDefsUses(CP.getSrcReg(), CP.getDstReg(), CP.getSrcIdx());
22662217

22672218
// Shrink subregister ranges if necessary.
22682219
if (ShrinkMask.any()) {

0 commit comments

Comments
 (0)