Skip to content

[InstrRef] Consistently use MLocTracker::getLocID() before calling lookupOrTrackRegister #167841

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 13, 2025
Merged

[InstrRef] Consistently use MLocTracker::getLocID() before calling lookupOrTrackRegister #167841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
194aa6d
[InstrRef] Consistently use MLocTracker::getLocID() before calling lo…
topperc Nov 13, 2025
ce07f1f
fixup! Update InstrRefLDVTest.cpp.
topperc Nov 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 19 additions & 14 deletions llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1603,8 +1603,8 @@ std::optional<ValueIDNum> InstrRefBasedLDV::getValueForInstrRef(
unsigned MainRegSize = TRI->getRegSizeInBits(*TRC);
if (Size != MainRegSize || Offset) {
// Enumerate all subregisters, searching.
Register NewReg = 0;
for (MCPhysReg SR : TRI->subregs(Reg)) {
Register NewReg = Register();
for (MCRegister SR : TRI->subregs(Reg)) {
unsigned Subreg = TRI->getSubRegIndex(Reg, SR);
unsigned SubregSize = TRI->getSubRegIdxSize(Subreg);
unsigned SubregOffset = TRI->getSubRegIdxOffset(Subreg);
Expand All @@ -1620,7 +1620,8 @@ std::optional<ValueIDNum> InstrRefBasedLDV::getValueForInstrRef(
} else {
// Re-state the value as being defined within the subregister
// that we found.
LocIdx NewLoc = MTracker->lookupOrTrackRegister(NewReg);
LocIdx NewLoc =
MTracker->lookupOrTrackRegister(MTracker->getLocID(NewReg));
Copy link
Contributor

@s-barannikov s-barannikov Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe provide an overload or replace lookupOrTrackRegister with a version that accepts Register?
The pattern is repeated many times.

NewID = ValueIDNum(NewID->getBlock(), NewID->getInst(), NewLoc);
}
}
Expand Down Expand Up @@ -1818,12 +1819,13 @@ bool InstrRefBasedLDV::transferDebugPHI(MachineInstr &MI) {
Register Reg = MO.getReg();
ValueIDNum Num = MTracker->readReg(Reg);
auto PHIRec = DebugPHIRecord(
{InstrNum, MI.getParent(), Num, MTracker->lookupOrTrackRegister(Reg)});
{InstrNum, MI.getParent(), Num,
MTracker->lookupOrTrackRegister(MTracker->getLocID(Reg))});
DebugPHINumToValue.push_back(PHIRec);

// Ensure this register is tracked.
for (MCRegAliasIterator RAI(MO.getReg(), TRI, true); RAI.isValid(); ++RAI)
MTracker->lookupOrTrackRegister(*RAI);
MTracker->lookupOrTrackRegister(MTracker->getLocID(*RAI));
} else if (MO.isFI()) {
// The value is whatever's in this stack slot.
unsigned FI = MO.getIndex();
Expand Down Expand Up @@ -1949,8 +1951,8 @@ void InstrRefBasedLDV::transferRegisterDef(MachineInstr &MI) {
// different location.

// Inform TTracker about any direct clobbers.
for (uint32_t DeadReg : DeadRegs) {
LocIdx Loc = MTracker->lookupOrTrackRegister(DeadReg);
for (MCRegister DeadReg : DeadRegs) {
LocIdx Loc = MTracker->lookupOrTrackRegister(MTracker->getLocID(DeadReg));
TTracker->clobberMloc(Loc, MI.getIterator(), false);
}

Expand Down Expand Up @@ -1995,9 +1997,9 @@ void InstrRefBasedLDV::performCopy(Register SrcRegNum, Register DstRegNum) {

// Copy subregisters from one location to another.
for (MCSubRegIndexIterator SRI(SrcRegNum, TRI); SRI.isValid(); ++SRI) {
unsigned SrcSubReg = SRI.getSubReg();
MCRegister SrcSubReg = SRI.getSubReg();
unsigned SubRegIdx = SRI.getSubRegIndex();
unsigned DstSubReg = TRI->getSubReg(DstRegNum, SubRegIdx);
MCRegister DstSubReg = TRI->getSubReg(DstRegNum, SubRegIdx);
if (!DstSubReg)
continue;

Expand All @@ -2006,8 +2008,10 @@ void InstrRefBasedLDV::performCopy(Register SrcRegNum, Register DstRegNum) {
// yet.
// This will force SrcSubReg to be tracked, if it isn't yet. Will read
// mphi values if it wasn't tracked.
LocIdx SrcL = MTracker->lookupOrTrackRegister(SrcSubReg);
LocIdx DstL = MTracker->lookupOrTrackRegister(DstSubReg);
LocIdx SrcL =
MTracker->lookupOrTrackRegister(MTracker->getLocID(SrcSubReg));
LocIdx DstL =
MTracker->lookupOrTrackRegister(MTracker->getLocID(DstSubReg));
(void)SrcL;
(void)DstL;
ValueIDNum CpyValue = MTracker->readReg(SrcSubReg);
Expand Down Expand Up @@ -2130,7 +2134,7 @@ bool InstrRefBasedLDV::transferSpillOrRestoreInst(MachineInstr &MI) {
// Then, transfer subreg bits.
for (MCPhysReg SR : TRI->subregs(Reg)) {
// Ensure this reg is tracked,
(void)MTracker->lookupOrTrackRegister(SR);
(void)MTracker->lookupOrTrackRegister(MTracker->getLocID(SR));
unsigned SubregIdx = TRI->getSubRegIndex(Reg, SR);
unsigned SpillID = MTracker->getLocID(Loc, SubregIdx);
DoTransfer(SR, SpillID);
Expand Down Expand Up @@ -2662,7 +2666,7 @@ void InstrRefBasedLDV::placeMLocPHIs(

// For reg units, place PHIs, and then place them for any aliasing registers.
for (Register R : RegUnitsToPHIUp) {
LocIdx L = MTracker->lookupOrTrackRegister(R);
LocIdx L = MTracker->lookupOrTrackRegister(MTracker->getLocID(R));
CollectPHIsForLoc(L);

// Install those PHI values into the live-in value array.
Expand All @@ -2675,7 +2679,8 @@ void InstrRefBasedLDV::placeMLocPHIs(
if (!MTracker->isRegisterTracked(*RAI))
continue;

LocIdx AliasLoc = MTracker->lookupOrTrackRegister(*RAI);
LocIdx AliasLoc =
MTracker->lookupOrTrackRegister(MTracker->getLocID(*RAI));
InstallPHIsAtLoc(AliasLoc);
}
}
Expand Down
34 changes: 17 additions & 17 deletions llvm/unittests/CodeGen/InstrRefLDVTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,7 @@ TEST_F(InstrRefLDVTest, MLocSingleBlock) {
// Add a new register to be tracked, and insert it into the transfer function
// as a copy. The output of $rax should be the live-in value of $rsp.
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(MTracker->getLocID(RAX));
TransferFunc[0].insert({RspLoc, ValueIDNum(0, 1, RspLoc)});
TransferFunc[0].insert({RaxLoc, ValueIDNum(0, 0, RspLoc)});
initValueArray(MInLocs, 1, 2);
Expand All @@ -980,7 +980,7 @@ TEST_F(InstrRefLDVTest, MLocDiamondBlocks) {
ASSERT_TRUE(MTracker->getNumLocs() == 1);
LocIdx RspLoc(0);
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(MTracker->getLocID(RAX));

auto [MInLocs, MOutLocs] = allocValueTables(4, 2);

Expand Down Expand Up @@ -1194,7 +1194,7 @@ TEST_F(InstrRefLDVTest, MLocSimpleLoop) {
ASSERT_TRUE(MTracker->getNumLocs() == 1);
LocIdx RspLoc(0);
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(MTracker->getLocID(RAX));

auto [MInLocs, MOutLocs] = allocValueTables(3, 2);

Expand Down Expand Up @@ -1292,7 +1292,7 @@ TEST_F(InstrRefLDVTest, MLocNestedLoop) {
ASSERT_TRUE(MTracker->getNumLocs() == 1);
LocIdx RspLoc(0);
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(MTracker->getLocID(RAX));

auto [MInLocs, MOutLocs] = allocValueTables(5, 2);

Expand Down Expand Up @@ -1493,7 +1493,7 @@ TEST_F(InstrRefLDVTest, MLocNoDominatingLoop) {
ASSERT_TRUE(MTracker->getNumLocs() == 1);
LocIdx RspLoc(0);
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(MTracker->getLocID(RAX));

auto [MInLocs, MOutLocs] = allocValueTables(5, 2);

Expand Down Expand Up @@ -1648,7 +1648,7 @@ TEST_F(InstrRefLDVTest, MLocBadlyNestedLoops) {
ASSERT_TRUE(MTracker->getNumLocs() == 1);
LocIdx RspLoc(0);
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(MTracker->getLocID(RAX));

auto [MInLocs, MOutLocs] = allocValueTables(5, 2);

Expand Down Expand Up @@ -1780,7 +1780,7 @@ TEST_F(InstrRefLDVTest, pickVPHILocDiamond) {
ASSERT_TRUE(MTracker->getNumLocs() == 1);
LocIdx RspLoc(0);
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(MTracker->getLocID(RAX));

auto [MInLocs, MOutLocs] = allocValueTables(4, 2);

Expand Down Expand Up @@ -1976,7 +1976,7 @@ TEST_F(InstrRefLDVTest, pickVPHILocLoops) {
ASSERT_TRUE(MTracker->getNumLocs() == 1);
LocIdx RspLoc(0);
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(MTracker->getLocID(RAX));

auto [MInLocs, MOutLocs] = allocValueTables(3, 2);

Expand Down Expand Up @@ -2104,9 +2104,9 @@ TEST_F(InstrRefLDVTest, pickVPHILocBadlyNestedLoops) {
ASSERT_TRUE(MTracker->getNumLocs() == 1);
LocIdx RspLoc(0);
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(MTracker->getLocID(RAX));
Register RBX = getRegByName("RBX");
LocIdx RbxLoc = MTracker->lookupOrTrackRegister(RBX);
LocIdx RbxLoc = MTracker->lookupOrTrackRegister(MTracker->getLocID(RBX));

auto [MInLocs, MOutLocs] = allocValueTables(5, 3);

Expand Down Expand Up @@ -2256,7 +2256,7 @@ TEST_F(InstrRefLDVTest, vlocJoinDiamond) {
ASSERT_TRUE(MTracker->getNumLocs() == 1);
LocIdx RspLoc(0);
Register RAX = getRegByName("RAX");
MTracker->lookupOrTrackRegister(RAX);
MTracker->lookupOrTrackRegister(MTracker->getLocID(RAX));

DbgOpID LiveInRspID = DbgOpID(false, 0);
DbgOpID LiveInRaxID = DbgOpID(false, 1);
Expand Down Expand Up @@ -2440,7 +2440,7 @@ TEST_F(InstrRefLDVTest, vlocJoinLoops) {
ASSERT_TRUE(MTracker->getNumLocs() == 1);
LocIdx RspLoc(0);
Register RAX = getRegByName("RAX");
MTracker->lookupOrTrackRegister(RAX);
MTracker->lookupOrTrackRegister(MTracker->getLocID(RAX));

DbgOpID LiveInRspID = DbgOpID(false, 0);
DbgOpID LiveInRaxID = DbgOpID(false, 1);
Expand Down Expand Up @@ -2538,9 +2538,9 @@ TEST_F(InstrRefLDVTest, vlocJoinBadlyNestedLoops) {
ASSERT_TRUE(MTracker->getNumLocs() == 1);
LocIdx RspLoc(0);
Register RAX = getRegByName("RAX");
MTracker->lookupOrTrackRegister(RAX);
MTracker->lookupOrTrackRegister(MTracker->getLocID(RAX));
Register RBX = getRegByName("RBX");
MTracker->lookupOrTrackRegister(RBX);
MTracker->lookupOrTrackRegister(MTracker->getLocID(RBX));

DbgOpID LiveInRspID = DbgOpID(false, 0);
DbgOpID LiveInRaxID = DbgOpID(false, 1);
Expand Down Expand Up @@ -2678,7 +2678,7 @@ TEST_F(InstrRefLDVTest, VLocDiamondBlocks) {
ASSERT_TRUE(MTracker->getNumLocs() == 1);
LocIdx RspLoc(0);
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(MTracker->getLocID(RAX));

unsigned EntryBlk = 0, RetBlk = 3;

Expand Down Expand Up @@ -2896,7 +2896,7 @@ TEST_F(InstrRefLDVTest, VLocSimpleLoop) {
ASSERT_TRUE(MTracker->getNumLocs() == 1);
LocIdx RspLoc(0);
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(MTracker->getLocID(RAX));

unsigned EntryBlk = 0, LoopBlk = 1;

Expand Down Expand Up @@ -3175,7 +3175,7 @@ TEST_F(InstrRefLDVTest, VLocNestedLoop) {
ASSERT_TRUE(MTracker->getNumLocs() == 1);
LocIdx RspLoc(0);
Register RAX = getRegByName("RAX");
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(RAX);
LocIdx RaxLoc = MTracker->lookupOrTrackRegister(MTracker->getLocID(RAX));

unsigned EntryBlk = 0, Loop1Blk = 1, Loop2Blk = 2;

Expand Down
Loading