From 194aa6d2a3cc39f3a8406d6ceabcdf2036b361e8 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 12 Nov 2025 23:49:51 -0800 Subject: [PATCH 1/2] [InstrRef] Consistently use MLocTracker::getLocID() before calling lookupOrTrackRegister. The LocID for registers is just the register ID. The getLocID function is supposed to hide this detail, but it wasn't being used consistently. This avoids a bunch of implicit casts from Register or MCRegister to unsigned. --- .../LiveDebugValues/InstrRefBasedImpl.cpp | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp index 0037bdd270ff3..6dda0fddbcec8 100644 --- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp +++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp @@ -1603,8 +1603,8 @@ std::optional 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); @@ -1620,7 +1620,8 @@ std::optional 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)); NewID = ValueIDNum(NewID->getBlock(), NewID->getInst(), NewLoc); } } @@ -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(); @@ -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); } @@ -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; @@ -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); @@ -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); @@ -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. @@ -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); } } From ce07f1f18021144727e1abf818ad2ea816fb94df Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Thu, 13 Nov 2025 07:15:45 -0800 Subject: [PATCH 2/2] fixup! Update InstrRefLDVTest.cpp. --- llvm/unittests/CodeGen/InstrRefLDVTest.cpp | 34 +++++++++++----------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/llvm/unittests/CodeGen/InstrRefLDVTest.cpp b/llvm/unittests/CodeGen/InstrRefLDVTest.cpp index 235a53dcc156e..5211a6c8ef416 100644 --- a/llvm/unittests/CodeGen/InstrRefLDVTest.cpp +++ b/llvm/unittests/CodeGen/InstrRefLDVTest.cpp @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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; @@ -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; @@ -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;