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

[topperc]

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.

[llvmbot]

@llvm/pr-subscribers-debuginfo

Author: Craig Topper (topperc)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/167841.diff

1 Files Affected:

• (modified) llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp (+19-14)

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<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);
@@ -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));
           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);
     }
   }

[jmorse]

LGTM, cheers!

[s-barannikov]

LGTM

[s-barannikov]

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

[s-barannikov]

There are more uses in llvm/unittests/CodeGen/InstrRefLDVTest.cpp

[topperc]

There are more uses in llvm/unittests/CodeGen/InstrRefLDVTest.cpp

Fixed