Skip to content
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.

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

[FrameLowering] Use MCRegister instead of Register in CalleeSavedInfo. NFC #128095

Merged
merged 2 commits into from
Feb 21, 2025

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Feb 21, 2025

Callee saved registers should always be phyiscal registers. They are often passed directly to other functions that take MCRegister like getMinimalPhysRegClass or TargetRegisterClass::contains.

Unfortunately, sometimes the MCRegister is compared to a Register which gave an ambiguous comparison error when the MCRegister is on the LHS. Adding a MCRegister==Register comparison operator created more ambiguous comparison errors elsewhere. These cases were usually comparing against a base or frame pointer register that is a physical register in a Register. For those I added an explicit conversion of Register to MCRegister to fix the error.

…. NFC

Callee saved registers should always be phyiscal registers. They
are often passed directly to other functions that take MCRegister
like getMinimalPhysRegClass.

Unforuntately sometimes the MCRegister is compared to a Register
which gave an ambiguous comparison err when the MCRegister is
on the LHS. Adding a MCRegister==Register comparison operator created
more ambiguous comparison errors elsewhere. These cases were usually
comparing against a base or frame pointer register that is a physical
register in a Register.  For those I added an explicit conversion of
Register to MCRegister to fix the error.
@llvmbot
Copy link
Member

llvmbot commented Feb 21, 2025

@llvm/pr-subscribers-backend-m68k
@llvm/pr-subscribers-backend-powerpc
@llvm/pr-subscribers-backend-msp430
@llvm/pr-subscribers-backend-xtensa
@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-backend-hexagon
@llvm/pr-subscribers-backend-loongarch

@llvm/pr-subscribers-backend-systemz

Author: Craig Topper (topperc)

Changes

Callee saved registers should always be phyiscal registers. They are often passed directly to other functions that take MCRegister like getMinimalPhysRegClass or TargetRegisterClass::contains.

Unfortunately, sometimes the MCRegister is compared to a Register which gave an ambiguous comparison error when the MCRegister is on the LHS. Adding a MCRegister==Register comparison operator created more ambiguous comparison errors elsewhere. These cases were usually comparing against a base or frame pointer register that is a physical register in a Register. For those I added an explicit conversion of Register to MCRegister to fix the error.


Patch is 41.55 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/128095.diff

24 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/MachineFrameInfo.h (+2-2)
  • (modified) llvm/lib/CodeGen/LivePhysRegs.cpp (+1-1)
  • (modified) llvm/lib/CodeGen/PrologEpilogInserter.cpp (+4-4)
  • (modified) llvm/lib/Target/AArch64/AArch64FrameLowering.cpp (+5-5)
  • (modified) llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp (+2-2)
  • (modified) llvm/lib/Target/AArch64/AArch64RegisterInfo.h (+1-1)
  • (modified) llvm/lib/Target/AMDGPU/SIFrameLowering.cpp (+3-2)
  • (modified) llvm/lib/Target/ARC/ARCFrameLowering.cpp (+1-1)
  • (modified) llvm/lib/Target/ARM/ARMFrameLowering.cpp (+7-7)
  • (modified) llvm/lib/Target/ARM/Thumb1FrameLowering.cpp (+12-12)
  • (modified) llvm/lib/Target/AVR/AVRFrameLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/CSKY/CSKYFrameLowering.cpp (+3-3)
  • (modified) llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp (+6-6)
  • (modified) llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp (+1-1)
  • (modified) llvm/lib/Target/M68k/M68kFrameLowering.cpp (+4-4)
  • (modified) llvm/lib/Target/MSP430/MSP430FrameLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/Mips/Mips16FrameLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/Mips/MipsSEFrameLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/PowerPC/PPCFrameLowering.cpp (+6-6)
  • (modified) llvm/lib/Target/RISCV/RISCVFrameLowering.cpp (+7-10)
  • (modified) llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp (+14-14)
  • (modified) llvm/lib/Target/X86/X86FrameLowering.cpp (+8-8)
  • (modified) llvm/lib/Target/XCore/XCoreFrameLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/Xtensa/XtensaFrameLowering.cpp (+2-2)
diff --git a/llvm/include/llvm/CodeGen/MachineFrameInfo.h b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
index 213b7ec6b3fbf..cf9c757a9721f 100644
--- a/llvm/include/llvm/CodeGen/MachineFrameInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
@@ -32,7 +32,7 @@ class AllocaInst;
 /// Callee saved reg can also be saved to a different register rather than
 /// on the stack by setting DstReg instead of FrameIdx.
 class CalleeSavedInfo {
-  Register Reg;
+  MCRegister Reg;
   union {
     int FrameIdx;
     unsigned DstReg;
@@ -58,7 +58,7 @@ class CalleeSavedInfo {
   explicit CalleeSavedInfo(unsigned R, int FI = 0) : Reg(R), FrameIdx(FI) {}
 
   // Accessors.
-  Register getReg()                        const { return Reg; }
+  MCRegister getReg()                      const { return Reg; }
   int getFrameIdx()                        const { return FrameIdx; }
   unsigned getDstReg()                     const { return DstReg; }
   void setFrameIdx(int FI) {
diff --git a/llvm/lib/CodeGen/LivePhysRegs.cpp b/llvm/lib/CodeGen/LivePhysRegs.cpp
index 2ba17e46be5a6..f5677bcd6b5f9 100644
--- a/llvm/lib/CodeGen/LivePhysRegs.cpp
+++ b/llvm/lib/CodeGen/LivePhysRegs.cpp
@@ -301,7 +301,7 @@ void llvm::recomputeLivenessFlags(MachineBasicBlock &MBB) {
       // the last instruction in the block.
       if (MI.isReturn() && MFI.isCalleeSavedInfoValid()) {
         for (const CalleeSavedInfo &Info : MFI.getCalleeSavedInfo()) {
-          if (Info.getReg() == Reg) {
+          if (Info.getReg() == Reg.asMCReg()) {
             IsNotLive = !Info.isRestored();
             break;
           }
diff --git a/llvm/lib/CodeGen/PrologEpilogInserter.cpp b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
index eb8929cae069e..c582dc527c017 100644
--- a/llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -481,7 +481,7 @@ static void assignCalleeSavedSpillSlots(MachineFunction &F,
       if (CS.isSpilledToReg())
         continue;
 
-      unsigned Reg = CS.getReg();
+      MCRegister Reg = CS.getReg();
       const TargetRegisterClass *RC = RegInfo->getMinimalPhysRegClass(Reg);
 
       int FrameIdx;
@@ -570,7 +570,7 @@ static void updateLiveness(MachineFunction &MF) {
   MachineRegisterInfo &MRI = MF.getRegInfo();
   for (const CalleeSavedInfo &I : CSI) {
     for (MachineBasicBlock *MBB : Visited) {
-      MCPhysReg Reg = I.getReg();
+      MCRegister Reg = I.getReg();
       // Add the callee-saved register as live-in.
       // It's killed at the spill.
       if (!MRI.isReserved(Reg) && !MBB->isLiveIn(Reg))
@@ -605,7 +605,7 @@ static void insertCSRSaves(MachineBasicBlock &SaveBlock,
   if (!TFI->spillCalleeSavedRegisters(SaveBlock, I, CSI, TRI)) {
     for (const CalleeSavedInfo &CS : CSI) {
       // Insert the spill to the stack frame.
-      unsigned Reg = CS.getReg();
+      MCRegister Reg = CS.getReg();
 
       if (CS.isSpilledToReg()) {
         BuildMI(SaveBlock, I, DebugLoc(),
@@ -634,7 +634,7 @@ static void insertCSRRestores(MachineBasicBlock &RestoreBlock,
 
   if (!TFI->restoreCalleeSavedRegisters(RestoreBlock, I, CSI, TRI)) {
     for (const CalleeSavedInfo &CI : reverse(CSI)) {
-      unsigned Reg = CI.getReg();
+      MCRegister Reg = CI.getReg();
       if (CI.isSpilledToReg()) {
         BuildMI(RestoreBlock, I, DebugLoc(), TII.get(TargetOpcode::COPY), Reg)
           .addReg(CI.getDstReg(), getKillRegState(true));
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index d118022395762..1761f58faf0fe 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -655,7 +655,7 @@ void AArch64FrameLowering::emitCalleeSavedSVELocations(
     // Not all unwinders may know about SVE registers, so assume the lowest
     // common demoninator.
     assert(!Info.isSpilledToReg() && "Spilling to registers not implemented");
-    unsigned Reg = Info.getReg();
+    MCRegister Reg = Info.getReg();
     if (!static_cast<const AArch64RegisterInfo &>(TRI).regNeedsCFI(Reg, Reg))
       continue;
 
@@ -716,7 +716,7 @@ void AArch64FrameLowering::resetCFIToInitialState(
   const std::vector<CalleeSavedInfo> &CSI =
       MF.getFrameInfo().getCalleeSavedInfo();
   for (const auto &Info : CSI) {
-    unsigned Reg = Info.getReg();
+    MCRegister Reg = Info.getReg();
     if (!TRI.regNeedsCFI(Reg, Reg))
       continue;
     insertCFISameValue(CFIDesc, MF, MBB, InsertPt,
@@ -744,7 +744,7 @@ static void emitCalleeSavedRestores(MachineBasicBlock &MBB,
         (MFI.getStackID(Info.getFrameIdx()) == TargetStackID::ScalableVector))
       continue;
 
-    unsigned Reg = Info.getReg();
+    MCRegister Reg = Info.getReg();
     if (SVE &&
         !static_cast<const AArch64RegisterInfo &>(TRI).regNeedsCFI(Reg, Reg))
       continue;
@@ -3051,7 +3051,7 @@ static void computeCalleeSaveRegisterPairs(
     int Scale = TRI->getSpillSize(*RPI.RC);
     // Add the next reg to the pair if it is in the same register class.
     if (unsigned(i + RegInc) < Count && !AFI->hasStackHazardSlotIndex()) {
-      Register NextReg = CSI[i + RegInc].getReg();
+      MCRegister NextReg = CSI[i + RegInc].getReg();
       bool IsFirst = i == FirstReg;
       switch (RPI.Type) {
       case RegPairInfo::GPR:
@@ -3986,7 +3986,7 @@ bool AArch64FrameLowering::assignCalleeSavedSpillSlots(
   Register LastReg = 0;
   int HazardSlotIndex = std::numeric_limits<int>::max();
   for (auto &CS : CSI) {
-    Register Reg = CS.getReg();
+    MCRegister Reg = CS.getReg();
     const TargetRegisterClass *RC = RegInfo->getMinimalPhysRegClass(Reg);
 
     // Create a hazard slot as we switch between GPR and FPR CSRs.
diff --git a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
index 8fd34325bb00d..52b362875b4ef 100644
--- a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
@@ -49,8 +49,8 @@ AArch64RegisterInfo::AArch64RegisterInfo(const Triple &TT, unsigned HwMode)
 /// callee-saves required by the base ABI. For the SVE registers z8-z15 only the
 /// lower 64-bits (d8-d15) need to be saved. The lower 64-bits subreg is
 /// returned in \p RegToUseForCFI.
-bool AArch64RegisterInfo::regNeedsCFI(unsigned Reg,
-                                      unsigned &RegToUseForCFI) const {
+bool AArch64RegisterInfo::regNeedsCFI(MCRegister Reg,
+                                      MCRegister &RegToUseForCFI) const {
   if (AArch64::PPRRegClass.contains(Reg))
     return false;
 
diff --git a/llvm/lib/Target/AArch64/AArch64RegisterInfo.h b/llvm/lib/Target/AArch64/AArch64RegisterInfo.h
index 898a509f75908..ddee0d6a0dc38 100644
--- a/llvm/lib/Target/AArch64/AArch64RegisterInfo.h
+++ b/llvm/lib/Target/AArch64/AArch64RegisterInfo.h
@@ -140,7 +140,7 @@ class AArch64RegisterInfo final : public AArch64GenRegisterInfo {
                              const LiveRegMatrix *Matrix) const override;
 
   unsigned getLocalAddressRegister(const MachineFunction &MF) const;
-  bool regNeedsCFI(unsigned Reg, unsigned &RegToUseForCFI) const;
+  bool regNeedsCFI(MCRegister Reg, MCRegister &RegToUseForCFI) const;
 
   /// SrcRC and DstRC will be morphed into NewRC if this returns true
   bool shouldCoalesce(MachineInstr *MI, const TargetRegisterClass *SrcRC,
diff --git a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
index 060db477a59f8..ce21f8963fe88 100644
--- a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
@@ -1719,11 +1719,12 @@ bool SIFrameLowering::assignCalleeSavedSpillSlots(
     NumModifiedRegs++;
 
   for (auto &CS : CSI) {
-    if (CS.getReg() == FramePtrReg && SGPRForFPSaveRestoreCopy) {
+    if (CS.getReg() == FramePtrReg.asMCReg() && SGPRForFPSaveRestoreCopy) {
       CS.setDstReg(SGPRForFPSaveRestoreCopy);
       if (--NumModifiedRegs)
         break;
-    } else if (CS.getReg() == BasePtrReg && SGPRForBPSaveRestoreCopy) {
+    } else if (CS.getReg() == BasePtrReg.asMCReg() &&
+               SGPRForBPSaveRestoreCopy) {
       CS.setDstReg(SGPRForBPSaveRestoreCopy);
       if (--NumModifiedRegs)
         break;
diff --git a/llvm/lib/Target/ARC/ARCFrameLowering.cpp b/llvm/lib/Target/ARC/ARCFrameLowering.cpp
index 95054eac8c4fa..9f6a79e3210c4 100644
--- a/llvm/lib/Target/ARC/ARCFrameLowering.cpp
+++ b/llvm/lib/Target/ARC/ARCFrameLowering.cpp
@@ -219,7 +219,7 @@ void ARCFrameLowering::emitPrologue(MachineFunction &MF,
   }
   // CFI for the rest of the registers.
   for (const auto &Entry : CSI) {
-    unsigned Reg = Entry.getReg();
+    MCRegister Reg = Entry.getReg();
     int FI = Entry.getFrameIdx();
     // Skip BLINK and FP.
     if ((hasFP(MF) && Reg == ARC::FP) || (MFI.hasCalls() && Reg == ARC::BLINK))
diff --git a/llvm/lib/Target/ARM/ARMFrameLowering.cpp b/llvm/lib/Target/ARM/ARMFrameLowering.cpp
index 3393c55f1639d..6e885ab574cea 100644
--- a/llvm/lib/Target/ARM/ARMFrameLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMFrameLowering.cpp
@@ -952,13 +952,13 @@ void ARMFrameLowering::emitPrologue(MachineFunction &MF,
   SpillArea FramePtrSpillArea = SpillArea::GPRCS1;
   bool BeforeFPPush = true;
   for (const CalleeSavedInfo &I : CSI) {
-    Register Reg = I.getReg();
+    MCRegister Reg = I.getReg();
     int FI = I.getFrameIdx();
 
     SpillArea Area = getSpillArea(Reg, PushPopSplit,
                                   AFI->getNumAlignedDPRCS2Regs(), RegInfo);
 
-    if (Reg == FramePtr) {
+    if (Reg == FramePtr.asMCReg()) {
       FramePtrSpillFI = FI;
       FramePtrSpillArea = Area;
     }
@@ -1280,7 +1280,7 @@ void ARMFrameLowering::emitPrologue(MachineFunction &MF,
   // recording where each register ended up:
   if (!NeedsWinCFI) {
     for (const auto &Entry : reverse(CSI)) {
-      Register Reg = Entry.getReg();
+      MCRegister Reg = Entry.getReg();
       int FI = Entry.getFrameIdx();
       MachineBasicBlock::iterator CFIPos;
       switch (getSpillArea(Reg, PushPopSplit, AFI->getNumAlignedDPRCS2Regs(),
@@ -1668,7 +1668,7 @@ void ARMFrameLowering::emitPushInst(MachineBasicBlock &MBB,
   while (i != 0) {
     unsigned LastReg = 0;
     for (; i != 0; --i) {
-      Register Reg = CSI[i-1].getReg();
+      MCRegister Reg = CSI[i-1].getReg();
       if (!Func(Reg))
         continue;
 
@@ -1761,7 +1761,7 @@ void ARMFrameLowering::emitPopInst(MachineBasicBlock &MBB,
     bool DeleteRet = false;
     for (; i != 0; --i) {
       CalleeSavedInfo &Info = CSI[i-1];
-      Register Reg = Info.getReg();
+      MCRegister Reg = Info.getReg();
       if (!Func(Reg))
         continue;
 
@@ -3003,7 +3003,7 @@ bool ARMFrameLowering::assignCalleeSavedSpillSlots(
       // LR, R7, R6, R5, R4, <R12>, R11, R10,  R9,  R8, D15-D8
       CSI.insert(find_if(CSI,
                          [=](const auto &CS) {
-                           Register Reg = CS.getReg();
+                           MCRegister Reg = CS.getReg();
                            return Reg == ARM::R10 || Reg == ARM::R11 ||
                                   Reg == ARM::R8 || Reg == ARM::R9 ||
                                   ARM::DPRRegClass.contains(Reg);
@@ -3021,7 +3021,7 @@ bool ARMFrameLowering::assignCalleeSavedSpillSlots(
              "address.");
       CSI.insert(find_if(CSI,
                          [=](const auto &CS) {
-                           Register Reg = CS.getReg();
+                           MCRegister Reg = CS.getReg();
                            return Reg != ARM::LR;
                          }),
                  CalleeSavedInfo(ARM::R12));
diff --git a/llvm/lib/Target/ARM/Thumb1FrameLowering.cpp b/llvm/lib/Target/ARM/Thumb1FrameLowering.cpp
index faa0352507fba..a69e307a5da20 100644
--- a/llvm/lib/Target/ARM/Thumb1FrameLowering.cpp
+++ b/llvm/lib/Target/ARM/Thumb1FrameLowering.cpp
@@ -210,9 +210,9 @@ void Thumb1FrameLowering::emitPrologue(MachineFunction &MF,
   bool HasFrameRecordArea = hasFP(MF) && ARM::hGPRRegClass.contains(FramePtr);
 
   for (const CalleeSavedInfo &I : CSI) {
-    Register Reg = I.getReg();
+    MCRegister Reg = I.getReg();
     int FI = I.getFrameIdx();
-    if (Reg == FramePtr)
+    if (Reg == FramePtr.asMCReg())
       FramePtrSpillFI = FI;
     switch (Reg) {
     case ARM::R11:
@@ -371,7 +371,7 @@ void Thumb1FrameLowering::emitPrologue(MachineFunction &MF,
           .setMIFlags(MachineInstr::FrameSetup);
     }
     for (const CalleeSavedInfo &I : CSI) {
-      Register Reg = I.getReg();
+      MCRegister Reg = I.getReg();
       int FI = I.getFrameIdx();
       switch (Reg) {
       case ARM::R8:
@@ -403,7 +403,7 @@ void Thumb1FrameLowering::emitPrologue(MachineFunction &MF,
   if (GPRCS2Size > 0) {
     MachineBasicBlock::iterator Pos = std::next(GPRCS2Push);
     for (auto &I : CSI) {
-      Register Reg = I.getReg();
+      MCRegister Reg = I.getReg();
       int FI = I.getFrameIdx();
       switch (Reg) {
       case ARM::R8:
@@ -432,8 +432,8 @@ void Thumb1FrameLowering::emitPrologue(MachineFunction &MF,
     // at this point in the prologue, so pick one.
     unsigned ScratchRegister = ARM::NoRegister;
     for (auto &I : CSI) {
-      Register Reg = I.getReg();
-      if (isARMLowRegister(Reg) && !(HasFP && Reg == FramePtr)) {
+      MCRegister Reg = I.getReg();
+      if (isARMLowRegister(Reg) && !(HasFP && Reg == FramePtr.asMCReg())) {
         ScratchRegister = Reg;
         break;
       }
@@ -552,8 +552,8 @@ void Thumb1FrameLowering::emitEpilogue(MachineFunction &MF,
     unsigned ScratchRegister = ARM::NoRegister;
     bool HasFP = hasFP(MF);
     for (auto &I : MFI.getCalleeSavedInfo()) {
-      Register Reg = I.getReg();
-      if (isARMLowRegister(Reg) && !(HasFP && Reg == FramePtr)) {
+      MCRegister Reg = I.getReg();
+      if (isARMLowRegister(Reg) && !(HasFP && Reg == FramePtr.asMCReg())) {
         ScratchRegister = Reg;
         break;
       }
@@ -1118,8 +1118,8 @@ bool Thumb1FrameLowering::spillCalleeSavedRegisters(
   std::set<Register> FrameRecord;
   std::set<Register> SpilledGPRs;
   for (const CalleeSavedInfo &I : CSI) {
-    Register Reg = I.getReg();
-    if (NeedsFrameRecordPush && (Reg == FPReg || Reg == ARM::LR))
+    MCRegister Reg = I.getReg();
+    if (NeedsFrameRecordPush && (Reg == FPReg.asMCReg() || Reg == ARM::LR))
       FrameRecord.insert(Reg);
     else
       SpilledGPRs.insert(Reg);
@@ -1206,8 +1206,8 @@ bool Thumb1FrameLowering::restoreCalleeSavedRegisters(
   std::set<Register> FrameRecord;
   std::set<Register> SpilledGPRs;
   for (CalleeSavedInfo &I : CSI) {
-    Register Reg = I.getReg();
-    if (NeedsFrameRecordPop && (Reg == FPReg || Reg == ARM::LR))
+    MCRegister Reg = I.getReg();
+    if (NeedsFrameRecordPop && (Reg == FPReg.asMCReg() || Reg == ARM::LR))
       FrameRecord.insert(Reg);
     else
       SpilledGPRs.insert(Reg);
diff --git a/llvm/lib/Target/AVR/AVRFrameLowering.cpp b/llvm/lib/Target/AVR/AVRFrameLowering.cpp
index ea94a90580c9c..b919be3d4466d 100644
--- a/llvm/lib/Target/AVR/AVRFrameLowering.cpp
+++ b/llvm/lib/Target/AVR/AVRFrameLowering.cpp
@@ -254,7 +254,7 @@ bool AVRFrameLowering::spillCalleeSavedRegisters(
   AVRMachineFunctionInfo *AVRFI = MF.getInfo<AVRMachineFunctionInfo>();
 
   for (const CalleeSavedInfo &I : llvm::reverse(CSI)) {
-    Register Reg = I.getReg();
+    MCRegister Reg = I.getReg();
     bool IsNotLiveIn = !MBB.isLiveIn(Reg);
 
     // Check if Reg is a sub register of a 16-bit livein register, and then
@@ -302,7 +302,7 @@ bool AVRFrameLowering::restoreCalleeSavedRegisters(
   const TargetInstrInfo &TII = *STI.getInstrInfo();
 
   for (const CalleeSavedInfo &CCSI : CSI) {
-    Register Reg = CCSI.getReg();
+    MCRegister Reg = CCSI.getReg();
 
     assert(TRI->getRegSizeInBits(*TRI->getMinimalPhysRegClass(Reg)) == 8 &&
            "Invalid register size");
diff --git a/llvm/lib/Target/CSKY/CSKYFrameLowering.cpp b/llvm/lib/Target/CSKY/CSKYFrameLowering.cpp
index f29caafe39526..e81bb4745faff 100644
--- a/llvm/lib/Target/CSKY/CSKYFrameLowering.cpp
+++ b/llvm/lib/Target/CSKY/CSKYFrameLowering.cpp
@@ -135,7 +135,7 @@ void CSKYFrameLowering::emitPrologue(MachineFunction &MF,
   // directives.
   for (const auto &Entry : CSI) {
     int64_t Offset = MFI.getObjectOffset(Entry.getFrameIdx());
-    Register Reg = Entry.getReg();
+    MCRegister Reg = Entry.getReg();
 
     unsigned Num = TRI->getRegSizeInBits(Reg, MRI) / 32;
     for (unsigned i = 0; i < Num; i++) {
@@ -474,7 +474,7 @@ bool CSKYFrameLowering::spillCalleeSavedRegisters(
 
   for (auto &CS : CSI) {
     // Insert the spill to the stack frame.
-    Register Reg = CS.getReg();
+    MCRegister Reg = CS.getReg();
     const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(Reg);
     TII.storeRegToStackSlot(MBB, MI, Reg, true, CS.getFrameIdx(), RC, TRI,
                             Register());
@@ -496,7 +496,7 @@ bool CSKYFrameLowering::restoreCalleeSavedRegisters(
     DL = MI->getDebugLoc();
 
   for (auto &CS : reverse(CSI)) {
-    Register Reg = CS.getReg();
+    MCRegister Reg = CS.getReg();
     const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(Reg);
     TII.loadRegFromStackSlot(MBB, MI, Reg, CS.getFrameIdx(), RC, TRI,
                              Register());
diff --git a/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp b/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp
index a35f7a3350f8c..8ac6b17f52ca9 100644
--- a/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp
@@ -1076,7 +1076,7 @@ void HexagonFrameLowering::insertCFIInstructionsAt(MachineBasicBlock &MBB,
         .addCFIIndex(MF.addFrameInst(OffR30));
   }
 
-  static Register RegsToMove[] = {
+  static MCRegister RegsToMove[] = {
     Hexagon::R1,  Hexagon::R0,  Hexagon::R3,  Hexagon::R2,
     Hexagon::R17, Hexagon::R16, Hexagon::R19, Hexagon::R18,
     Hexagon::R21, Hexagon::R20, Hexagon::R23, Hexagon::R22,
@@ -1089,7 +1089,7 @@ void HexagonFrameLowering::insertCFIInstructionsAt(MachineBasicBlock &MBB,
   const std::vector<CalleeSavedInfo> &CSI = MFI.getCalleeSavedInfo();
 
   for (unsigned i = 0; RegsToMove[i] != Hexagon::NoRegister; ++i) {
-    Register Reg = RegsToMove[i];
+    MCRegister Reg = RegsToMove[i];
     auto IfR = [Reg] (const CalleeSavedInfo &C) -> bool {
       return C.getReg() == Reg;
     };
@@ -1411,7 +1411,7 @@ bool HexagonFrameLowering::insertCSRSpillsInBlock(MachineBasicBlock &MBB,
   }
 
   for (const CalleeSavedInfo &I : CSI) {
-    Register Reg = I.getReg();
+    MCRegister Reg = I.getReg();
     // Add live in registers. We treat eh_return callee saved register r0 - r3
     // specially. They are not really callee saved registers as they are not
     // supposed to be killed.
@@ -1480,7 +1480,7 @@ bool HexagonFrameLowering::insertCSRRestoresInBlock(MachineBasicBlock &MBB,
   }
 
   for (const CalleeSavedInfo &I : CSI) {
-    Register Reg = I.getReg();
+    MCRegister Reg = I.getReg();
     const TargetRegisterClass *RC = HRI.getMinimalPhysRegClass(Reg);
     int FI = I.getFrameIdx();
     HII.loadRegFromStackSlot(MBB, MI, Reg, FI, RC, &HRI, Register());
@@ -1514,7 +1514,7 @@ void HexagonFrameLowering::processFunctionBeforeFrameFinalized(
     return;
 
   // Set the physical aligned-stack base address register.
-  Register AP = 0;
+  MCRegister AP;
   if (const MachineInstr *AI = getAlignaInstr(MF))
     AP = AI->getOperand(0).getReg();
   auto &HMFI = *MF.getInfo<HexagonMachineFunctionInfo>();
@@ -2590,7 +2590,7 @@ bool HexagonFrameLowering::shouldInlineCSR(const MachineFunction &MF,
   // a contiguous block starting from D8.
   BitVector Regs(Hexagon::NUM_TARGET_REGS);
   for (const CalleeSavedInfo &I : CSI) {
-    Register R = I.getReg();
+    MCRegister R = I.getReg();
     if (!Hexagon::DoubleRegsRegClass.contains(R))
       return true;
     Regs[R] = true;
diff --git a/llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp b/llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp
index 4b7b5483f5b81..ac5e7f3891c72 100644
--- a/llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp
+++ b/llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp
@@ -438,7 +438,7 @@ bool LoongArchFrameLowering::spillCalleeSavedRegisters(
 
   // Insert the spill to the stack frame.
   for (auto &CS : CSI) {
-    Register Reg = CS.getReg();
+    MCRegister Reg = CS.getReg();
     // If the register is RA and the return address is taken by method
     // LoongArchTargetLowering::lowerRETURNADDR, don't set kill flag.
     bool IsKill =
diff --git...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Feb 21, 2025

@llvm/pr-subscribers-backend-aarch64

Author: Craig Topper (topperc)

Changes

Callee saved registers should always be phyiscal registers. They are often passed directly to other functions that take MCRegister like getMinimalPhysRegClass or TargetRegisterClass::contains.

Unfortunately, sometimes the MCRegister is compared to a Register which gave an ambiguous comparison error when the MCRegister is on the LHS. Adding a MCRegister==Register comparison operator created more ambiguous comparison errors elsewhere. These cases were usually comparing against a base or frame pointer register that is a physical register in a Register. For those I added an explicit conversion of Register to MCRegister to fix the error.


Patch is 41.55 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/128095.diff

24 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/MachineFrameInfo.h (+2-2)
  • (modified) llvm/lib/CodeGen/LivePhysRegs.cpp (+1-1)
  • (modified) llvm/lib/CodeGen/PrologEpilogInserter.cpp (+4-4)
  • (modified) llvm/lib/Target/AArch64/AArch64FrameLowering.cpp (+5-5)
  • (modified) llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp (+2-2)
  • (modified) llvm/lib/Target/AArch64/AArch64RegisterInfo.h (+1-1)
  • (modified) llvm/lib/Target/AMDGPU/SIFrameLowering.cpp (+3-2)
  • (modified) llvm/lib/Target/ARC/ARCFrameLowering.cpp (+1-1)
  • (modified) llvm/lib/Target/ARM/ARMFrameLowering.cpp (+7-7)
  • (modified) llvm/lib/Target/ARM/Thumb1FrameLowering.cpp (+12-12)
  • (modified) llvm/lib/Target/AVR/AVRFrameLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/CSKY/CSKYFrameLowering.cpp (+3-3)
  • (modified) llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp (+6-6)
  • (modified) llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp (+1-1)
  • (modified) llvm/lib/Target/M68k/M68kFrameLowering.cpp (+4-4)
  • (modified) llvm/lib/Target/MSP430/MSP430FrameLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/Mips/Mips16FrameLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/Mips/MipsSEFrameLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/PowerPC/PPCFrameLowering.cpp (+6-6)
  • (modified) llvm/lib/Target/RISCV/RISCVFrameLowering.cpp (+7-10)
  • (modified) llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp (+14-14)
  • (modified) llvm/lib/Target/X86/X86FrameLowering.cpp (+8-8)
  • (modified) llvm/lib/Target/XCore/XCoreFrameLowering.cpp (+2-2)
  • (modified) llvm/lib/Target/Xtensa/XtensaFrameLowering.cpp (+2-2)
diff --git a/llvm/include/llvm/CodeGen/MachineFrameInfo.h b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
index 213b7ec6b3fbf..cf9c757a9721f 100644
--- a/llvm/include/llvm/CodeGen/MachineFrameInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
@@ -32,7 +32,7 @@ class AllocaInst;
 /// Callee saved reg can also be saved to a different register rather than
 /// on the stack by setting DstReg instead of FrameIdx.
 class CalleeSavedInfo {
-  Register Reg;
+  MCRegister Reg;
   union {
     int FrameIdx;
     unsigned DstReg;
@@ -58,7 +58,7 @@ class CalleeSavedInfo {
   explicit CalleeSavedInfo(unsigned R, int FI = 0) : Reg(R), FrameIdx(FI) {}
 
   // Accessors.
-  Register getReg()                        const { return Reg; }
+  MCRegister getReg()                      const { return Reg; }
   int getFrameIdx()                        const { return FrameIdx; }
   unsigned getDstReg()                     const { return DstReg; }
   void setFrameIdx(int FI) {
diff --git a/llvm/lib/CodeGen/LivePhysRegs.cpp b/llvm/lib/CodeGen/LivePhysRegs.cpp
index 2ba17e46be5a6..f5677bcd6b5f9 100644
--- a/llvm/lib/CodeGen/LivePhysRegs.cpp
+++ b/llvm/lib/CodeGen/LivePhysRegs.cpp
@@ -301,7 +301,7 @@ void llvm::recomputeLivenessFlags(MachineBasicBlock &MBB) {
       // the last instruction in the block.
       if (MI.isReturn() && MFI.isCalleeSavedInfoValid()) {
         for (const CalleeSavedInfo &Info : MFI.getCalleeSavedInfo()) {
-          if (Info.getReg() == Reg) {
+          if (Info.getReg() == Reg.asMCReg()) {
             IsNotLive = !Info.isRestored();
             break;
           }
diff --git a/llvm/lib/CodeGen/PrologEpilogInserter.cpp b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
index eb8929cae069e..c582dc527c017 100644
--- a/llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -481,7 +481,7 @@ static void assignCalleeSavedSpillSlots(MachineFunction &F,
       if (CS.isSpilledToReg())
         continue;
 
-      unsigned Reg = CS.getReg();
+      MCRegister Reg = CS.getReg();
       const TargetRegisterClass *RC = RegInfo->getMinimalPhysRegClass(Reg);
 
       int FrameIdx;
@@ -570,7 +570,7 @@ static void updateLiveness(MachineFunction &MF) {
   MachineRegisterInfo &MRI = MF.getRegInfo();
   for (const CalleeSavedInfo &I : CSI) {
     for (MachineBasicBlock *MBB : Visited) {
-      MCPhysReg Reg = I.getReg();
+      MCRegister Reg = I.getReg();
       // Add the callee-saved register as live-in.
       // It's killed at the spill.
       if (!MRI.isReserved(Reg) && !MBB->isLiveIn(Reg))
@@ -605,7 +605,7 @@ static void insertCSRSaves(MachineBasicBlock &SaveBlock,
   if (!TFI->spillCalleeSavedRegisters(SaveBlock, I, CSI, TRI)) {
     for (const CalleeSavedInfo &CS : CSI) {
       // Insert the spill to the stack frame.
-      unsigned Reg = CS.getReg();
+      MCRegister Reg = CS.getReg();
 
       if (CS.isSpilledToReg()) {
         BuildMI(SaveBlock, I, DebugLoc(),
@@ -634,7 +634,7 @@ static void insertCSRRestores(MachineBasicBlock &RestoreBlock,
 
   if (!TFI->restoreCalleeSavedRegisters(RestoreBlock, I, CSI, TRI)) {
     for (const CalleeSavedInfo &CI : reverse(CSI)) {
-      unsigned Reg = CI.getReg();
+      MCRegister Reg = CI.getReg();
       if (CI.isSpilledToReg()) {
         BuildMI(RestoreBlock, I, DebugLoc(), TII.get(TargetOpcode::COPY), Reg)
           .addReg(CI.getDstReg(), getKillRegState(true));
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index d118022395762..1761f58faf0fe 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -655,7 +655,7 @@ void AArch64FrameLowering::emitCalleeSavedSVELocations(
     // Not all unwinders may know about SVE registers, so assume the lowest
     // common demoninator.
     assert(!Info.isSpilledToReg() && "Spilling to registers not implemented");
-    unsigned Reg = Info.getReg();
+    MCRegister Reg = Info.getReg();
     if (!static_cast<const AArch64RegisterInfo &>(TRI).regNeedsCFI(Reg, Reg))
       continue;
 
@@ -716,7 +716,7 @@ void AArch64FrameLowering::resetCFIToInitialState(
   const std::vector<CalleeSavedInfo> &CSI =
       MF.getFrameInfo().getCalleeSavedInfo();
   for (const auto &Info : CSI) {
-    unsigned Reg = Info.getReg();
+    MCRegister Reg = Info.getReg();
     if (!TRI.regNeedsCFI(Reg, Reg))
       continue;
     insertCFISameValue(CFIDesc, MF, MBB, InsertPt,
@@ -744,7 +744,7 @@ static void emitCalleeSavedRestores(MachineBasicBlock &MBB,
         (MFI.getStackID(Info.getFrameIdx()) == TargetStackID::ScalableVector))
       continue;
 
-    unsigned Reg = Info.getReg();
+    MCRegister Reg = Info.getReg();
     if (SVE &&
         !static_cast<const AArch64RegisterInfo &>(TRI).regNeedsCFI(Reg, Reg))
       continue;
@@ -3051,7 +3051,7 @@ static void computeCalleeSaveRegisterPairs(
     int Scale = TRI->getSpillSize(*RPI.RC);
     // Add the next reg to the pair if it is in the same register class.
     if (unsigned(i + RegInc) < Count && !AFI->hasStackHazardSlotIndex()) {
-      Register NextReg = CSI[i + RegInc].getReg();
+      MCRegister NextReg = CSI[i + RegInc].getReg();
       bool IsFirst = i == FirstReg;
       switch (RPI.Type) {
       case RegPairInfo::GPR:
@@ -3986,7 +3986,7 @@ bool AArch64FrameLowering::assignCalleeSavedSpillSlots(
   Register LastReg = 0;
   int HazardSlotIndex = std::numeric_limits<int>::max();
   for (auto &CS : CSI) {
-    Register Reg = CS.getReg();
+    MCRegister Reg = CS.getReg();
     const TargetRegisterClass *RC = RegInfo->getMinimalPhysRegClass(Reg);
 
     // Create a hazard slot as we switch between GPR and FPR CSRs.
diff --git a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
index 8fd34325bb00d..52b362875b4ef 100644
--- a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
@@ -49,8 +49,8 @@ AArch64RegisterInfo::AArch64RegisterInfo(const Triple &TT, unsigned HwMode)
 /// callee-saves required by the base ABI. For the SVE registers z8-z15 only the
 /// lower 64-bits (d8-d15) need to be saved. The lower 64-bits subreg is
 /// returned in \p RegToUseForCFI.
-bool AArch64RegisterInfo::regNeedsCFI(unsigned Reg,
-                                      unsigned &RegToUseForCFI) const {
+bool AArch64RegisterInfo::regNeedsCFI(MCRegister Reg,
+                                      MCRegister &RegToUseForCFI) const {
   if (AArch64::PPRRegClass.contains(Reg))
     return false;
 
diff --git a/llvm/lib/Target/AArch64/AArch64RegisterInfo.h b/llvm/lib/Target/AArch64/AArch64RegisterInfo.h
index 898a509f75908..ddee0d6a0dc38 100644
--- a/llvm/lib/Target/AArch64/AArch64RegisterInfo.h
+++ b/llvm/lib/Target/AArch64/AArch64RegisterInfo.h
@@ -140,7 +140,7 @@ class AArch64RegisterInfo final : public AArch64GenRegisterInfo {
                              const LiveRegMatrix *Matrix) const override;
 
   unsigned getLocalAddressRegister(const MachineFunction &MF) const;
-  bool regNeedsCFI(unsigned Reg, unsigned &RegToUseForCFI) const;
+  bool regNeedsCFI(MCRegister Reg, MCRegister &RegToUseForCFI) const;
 
   /// SrcRC and DstRC will be morphed into NewRC if this returns true
   bool shouldCoalesce(MachineInstr *MI, const TargetRegisterClass *SrcRC,
diff --git a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
index 060db477a59f8..ce21f8963fe88 100644
--- a/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
@@ -1719,11 +1719,12 @@ bool SIFrameLowering::assignCalleeSavedSpillSlots(
     NumModifiedRegs++;
 
   for (auto &CS : CSI) {
-    if (CS.getReg() == FramePtrReg && SGPRForFPSaveRestoreCopy) {
+    if (CS.getReg() == FramePtrReg.asMCReg() && SGPRForFPSaveRestoreCopy) {
       CS.setDstReg(SGPRForFPSaveRestoreCopy);
       if (--NumModifiedRegs)
         break;
-    } else if (CS.getReg() == BasePtrReg && SGPRForBPSaveRestoreCopy) {
+    } else if (CS.getReg() == BasePtrReg.asMCReg() &&
+               SGPRForBPSaveRestoreCopy) {
       CS.setDstReg(SGPRForBPSaveRestoreCopy);
       if (--NumModifiedRegs)
         break;
diff --git a/llvm/lib/Target/ARC/ARCFrameLowering.cpp b/llvm/lib/Target/ARC/ARCFrameLowering.cpp
index 95054eac8c4fa..9f6a79e3210c4 100644
--- a/llvm/lib/Target/ARC/ARCFrameLowering.cpp
+++ b/llvm/lib/Target/ARC/ARCFrameLowering.cpp
@@ -219,7 +219,7 @@ void ARCFrameLowering::emitPrologue(MachineFunction &MF,
   }
   // CFI for the rest of the registers.
   for (const auto &Entry : CSI) {
-    unsigned Reg = Entry.getReg();
+    MCRegister Reg = Entry.getReg();
     int FI = Entry.getFrameIdx();
     // Skip BLINK and FP.
     if ((hasFP(MF) && Reg == ARC::FP) || (MFI.hasCalls() && Reg == ARC::BLINK))
diff --git a/llvm/lib/Target/ARM/ARMFrameLowering.cpp b/llvm/lib/Target/ARM/ARMFrameLowering.cpp
index 3393c55f1639d..6e885ab574cea 100644
--- a/llvm/lib/Target/ARM/ARMFrameLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMFrameLowering.cpp
@@ -952,13 +952,13 @@ void ARMFrameLowering::emitPrologue(MachineFunction &MF,
   SpillArea FramePtrSpillArea = SpillArea::GPRCS1;
   bool BeforeFPPush = true;
   for (const CalleeSavedInfo &I : CSI) {
-    Register Reg = I.getReg();
+    MCRegister Reg = I.getReg();
     int FI = I.getFrameIdx();
 
     SpillArea Area = getSpillArea(Reg, PushPopSplit,
                                   AFI->getNumAlignedDPRCS2Regs(), RegInfo);
 
-    if (Reg == FramePtr) {
+    if (Reg == FramePtr.asMCReg()) {
       FramePtrSpillFI = FI;
       FramePtrSpillArea = Area;
     }
@@ -1280,7 +1280,7 @@ void ARMFrameLowering::emitPrologue(MachineFunction &MF,
   // recording where each register ended up:
   if (!NeedsWinCFI) {
     for (const auto &Entry : reverse(CSI)) {
-      Register Reg = Entry.getReg();
+      MCRegister Reg = Entry.getReg();
       int FI = Entry.getFrameIdx();
       MachineBasicBlock::iterator CFIPos;
       switch (getSpillArea(Reg, PushPopSplit, AFI->getNumAlignedDPRCS2Regs(),
@@ -1668,7 +1668,7 @@ void ARMFrameLowering::emitPushInst(MachineBasicBlock &MBB,
   while (i != 0) {
     unsigned LastReg = 0;
     for (; i != 0; --i) {
-      Register Reg = CSI[i-1].getReg();
+      MCRegister Reg = CSI[i-1].getReg();
       if (!Func(Reg))
         continue;
 
@@ -1761,7 +1761,7 @@ void ARMFrameLowering::emitPopInst(MachineBasicBlock &MBB,
     bool DeleteRet = false;
     for (; i != 0; --i) {
       CalleeSavedInfo &Info = CSI[i-1];
-      Register Reg = Info.getReg();
+      MCRegister Reg = Info.getReg();
       if (!Func(Reg))
         continue;
 
@@ -3003,7 +3003,7 @@ bool ARMFrameLowering::assignCalleeSavedSpillSlots(
       // LR, R7, R6, R5, R4, <R12>, R11, R10,  R9,  R8, D15-D8
       CSI.insert(find_if(CSI,
                          [=](const auto &CS) {
-                           Register Reg = CS.getReg();
+                           MCRegister Reg = CS.getReg();
                            return Reg == ARM::R10 || Reg == ARM::R11 ||
                                   Reg == ARM::R8 || Reg == ARM::R9 ||
                                   ARM::DPRRegClass.contains(Reg);
@@ -3021,7 +3021,7 @@ bool ARMFrameLowering::assignCalleeSavedSpillSlots(
              "address.");
       CSI.insert(find_if(CSI,
                          [=](const auto &CS) {
-                           Register Reg = CS.getReg();
+                           MCRegister Reg = CS.getReg();
                            return Reg != ARM::LR;
                          }),
                  CalleeSavedInfo(ARM::R12));
diff --git a/llvm/lib/Target/ARM/Thumb1FrameLowering.cpp b/llvm/lib/Target/ARM/Thumb1FrameLowering.cpp
index faa0352507fba..a69e307a5da20 100644
--- a/llvm/lib/Target/ARM/Thumb1FrameLowering.cpp
+++ b/llvm/lib/Target/ARM/Thumb1FrameLowering.cpp
@@ -210,9 +210,9 @@ void Thumb1FrameLowering::emitPrologue(MachineFunction &MF,
   bool HasFrameRecordArea = hasFP(MF) && ARM::hGPRRegClass.contains(FramePtr);
 
   for (const CalleeSavedInfo &I : CSI) {
-    Register Reg = I.getReg();
+    MCRegister Reg = I.getReg();
     int FI = I.getFrameIdx();
-    if (Reg == FramePtr)
+    if (Reg == FramePtr.asMCReg())
       FramePtrSpillFI = FI;
     switch (Reg) {
     case ARM::R11:
@@ -371,7 +371,7 @@ void Thumb1FrameLowering::emitPrologue(MachineFunction &MF,
           .setMIFlags(MachineInstr::FrameSetup);
     }
     for (const CalleeSavedInfo &I : CSI) {
-      Register Reg = I.getReg();
+      MCRegister Reg = I.getReg();
       int FI = I.getFrameIdx();
       switch (Reg) {
       case ARM::R8:
@@ -403,7 +403,7 @@ void Thumb1FrameLowering::emitPrologue(MachineFunction &MF,
   if (GPRCS2Size > 0) {
     MachineBasicBlock::iterator Pos = std::next(GPRCS2Push);
     for (auto &I : CSI) {
-      Register Reg = I.getReg();
+      MCRegister Reg = I.getReg();
       int FI = I.getFrameIdx();
       switch (Reg) {
       case ARM::R8:
@@ -432,8 +432,8 @@ void Thumb1FrameLowering::emitPrologue(MachineFunction &MF,
     // at this point in the prologue, so pick one.
     unsigned ScratchRegister = ARM::NoRegister;
     for (auto &I : CSI) {
-      Register Reg = I.getReg();
-      if (isARMLowRegister(Reg) && !(HasFP && Reg == FramePtr)) {
+      MCRegister Reg = I.getReg();
+      if (isARMLowRegister(Reg) && !(HasFP && Reg == FramePtr.asMCReg())) {
         ScratchRegister = Reg;
         break;
       }
@@ -552,8 +552,8 @@ void Thumb1FrameLowering::emitEpilogue(MachineFunction &MF,
     unsigned ScratchRegister = ARM::NoRegister;
     bool HasFP = hasFP(MF);
     for (auto &I : MFI.getCalleeSavedInfo()) {
-      Register Reg = I.getReg();
-      if (isARMLowRegister(Reg) && !(HasFP && Reg == FramePtr)) {
+      MCRegister Reg = I.getReg();
+      if (isARMLowRegister(Reg) && !(HasFP && Reg == FramePtr.asMCReg())) {
         ScratchRegister = Reg;
         break;
       }
@@ -1118,8 +1118,8 @@ bool Thumb1FrameLowering::spillCalleeSavedRegisters(
   std::set<Register> FrameRecord;
   std::set<Register> SpilledGPRs;
   for (const CalleeSavedInfo &I : CSI) {
-    Register Reg = I.getReg();
-    if (NeedsFrameRecordPush && (Reg == FPReg || Reg == ARM::LR))
+    MCRegister Reg = I.getReg();
+    if (NeedsFrameRecordPush && (Reg == FPReg.asMCReg() || Reg == ARM::LR))
       FrameRecord.insert(Reg);
     else
       SpilledGPRs.insert(Reg);
@@ -1206,8 +1206,8 @@ bool Thumb1FrameLowering::restoreCalleeSavedRegisters(
   std::set<Register> FrameRecord;
   std::set<Register> SpilledGPRs;
   for (CalleeSavedInfo &I : CSI) {
-    Register Reg = I.getReg();
-    if (NeedsFrameRecordPop && (Reg == FPReg || Reg == ARM::LR))
+    MCRegister Reg = I.getReg();
+    if (NeedsFrameRecordPop && (Reg == FPReg.asMCReg() || Reg == ARM::LR))
       FrameRecord.insert(Reg);
     else
       SpilledGPRs.insert(Reg);
diff --git a/llvm/lib/Target/AVR/AVRFrameLowering.cpp b/llvm/lib/Target/AVR/AVRFrameLowering.cpp
index ea94a90580c9c..b919be3d4466d 100644
--- a/llvm/lib/Target/AVR/AVRFrameLowering.cpp
+++ b/llvm/lib/Target/AVR/AVRFrameLowering.cpp
@@ -254,7 +254,7 @@ bool AVRFrameLowering::spillCalleeSavedRegisters(
   AVRMachineFunctionInfo *AVRFI = MF.getInfo<AVRMachineFunctionInfo>();
 
   for (const CalleeSavedInfo &I : llvm::reverse(CSI)) {
-    Register Reg = I.getReg();
+    MCRegister Reg = I.getReg();
     bool IsNotLiveIn = !MBB.isLiveIn(Reg);
 
     // Check if Reg is a sub register of a 16-bit livein register, and then
@@ -302,7 +302,7 @@ bool AVRFrameLowering::restoreCalleeSavedRegisters(
   const TargetInstrInfo &TII = *STI.getInstrInfo();
 
   for (const CalleeSavedInfo &CCSI : CSI) {
-    Register Reg = CCSI.getReg();
+    MCRegister Reg = CCSI.getReg();
 
     assert(TRI->getRegSizeInBits(*TRI->getMinimalPhysRegClass(Reg)) == 8 &&
            "Invalid register size");
diff --git a/llvm/lib/Target/CSKY/CSKYFrameLowering.cpp b/llvm/lib/Target/CSKY/CSKYFrameLowering.cpp
index f29caafe39526..e81bb4745faff 100644
--- a/llvm/lib/Target/CSKY/CSKYFrameLowering.cpp
+++ b/llvm/lib/Target/CSKY/CSKYFrameLowering.cpp
@@ -135,7 +135,7 @@ void CSKYFrameLowering::emitPrologue(MachineFunction &MF,
   // directives.
   for (const auto &Entry : CSI) {
     int64_t Offset = MFI.getObjectOffset(Entry.getFrameIdx());
-    Register Reg = Entry.getReg();
+    MCRegister Reg = Entry.getReg();
 
     unsigned Num = TRI->getRegSizeInBits(Reg, MRI) / 32;
     for (unsigned i = 0; i < Num; i++) {
@@ -474,7 +474,7 @@ bool CSKYFrameLowering::spillCalleeSavedRegisters(
 
   for (auto &CS : CSI) {
     // Insert the spill to the stack frame.
-    Register Reg = CS.getReg();
+    MCRegister Reg = CS.getReg();
     const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(Reg);
     TII.storeRegToStackSlot(MBB, MI, Reg, true, CS.getFrameIdx(), RC, TRI,
                             Register());
@@ -496,7 +496,7 @@ bool CSKYFrameLowering::restoreCalleeSavedRegisters(
     DL = MI->getDebugLoc();
 
   for (auto &CS : reverse(CSI)) {
-    Register Reg = CS.getReg();
+    MCRegister Reg = CS.getReg();
     const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(Reg);
     TII.loadRegFromStackSlot(MBB, MI, Reg, CS.getFrameIdx(), RC, TRI,
                              Register());
diff --git a/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp b/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp
index a35f7a3350f8c..8ac6b17f52ca9 100644
--- a/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp
@@ -1076,7 +1076,7 @@ void HexagonFrameLowering::insertCFIInstructionsAt(MachineBasicBlock &MBB,
         .addCFIIndex(MF.addFrameInst(OffR30));
   }
 
-  static Register RegsToMove[] = {
+  static MCRegister RegsToMove[] = {
     Hexagon::R1,  Hexagon::R0,  Hexagon::R3,  Hexagon::R2,
     Hexagon::R17, Hexagon::R16, Hexagon::R19, Hexagon::R18,
     Hexagon::R21, Hexagon::R20, Hexagon::R23, Hexagon::R22,
@@ -1089,7 +1089,7 @@ void HexagonFrameLowering::insertCFIInstructionsAt(MachineBasicBlock &MBB,
   const std::vector<CalleeSavedInfo> &CSI = MFI.getCalleeSavedInfo();
 
   for (unsigned i = 0; RegsToMove[i] != Hexagon::NoRegister; ++i) {
-    Register Reg = RegsToMove[i];
+    MCRegister Reg = RegsToMove[i];
     auto IfR = [Reg] (const CalleeSavedInfo &C) -> bool {
       return C.getReg() == Reg;
     };
@@ -1411,7 +1411,7 @@ bool HexagonFrameLowering::insertCSRSpillsInBlock(MachineBasicBlock &MBB,
   }
 
   for (const CalleeSavedInfo &I : CSI) {
-    Register Reg = I.getReg();
+    MCRegister Reg = I.getReg();
     // Add live in registers. We treat eh_return callee saved register r0 - r3
     // specially. They are not really callee saved registers as they are not
     // supposed to be killed.
@@ -1480,7 +1480,7 @@ bool HexagonFrameLowering::insertCSRRestoresInBlock(MachineBasicBlock &MBB,
   }
 
   for (const CalleeSavedInfo &I : CSI) {
-    Register Reg = I.getReg();
+    MCRegister Reg = I.getReg();
     const TargetRegisterClass *RC = HRI.getMinimalPhysRegClass(Reg);
     int FI = I.getFrameIdx();
     HII.loadRegFromStackSlot(MBB, MI, Reg, FI, RC, &HRI, Register());
@@ -1514,7 +1514,7 @@ void HexagonFrameLowering::processFunctionBeforeFrameFinalized(
     return;
 
   // Set the physical aligned-stack base address register.
-  Register AP = 0;
+  MCRegister AP;
   if (const MachineInstr *AI = getAlignaInstr(MF))
     AP = AI->getOperand(0).getReg();
   auto &HMFI = *MF.getInfo<HexagonMachineFunctionInfo>();
@@ -2590,7 +2590,7 @@ bool HexagonFrameLowering::shouldInlineCSR(const MachineFunction &MF,
   // a contiguous block starting from D8.
   BitVector Regs(Hexagon::NUM_TARGET_REGS);
   for (const CalleeSavedInfo &I : CSI) {
-    Register R = I.getReg();
+    MCRegister R = I.getReg();
     if (!Hexagon::DoubleRegsRegClass.contains(R))
       return true;
     Regs[R] = true;
diff --git a/llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp b/llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp
index 4b7b5483f5b81..ac5e7f3891c72 100644
--- a/llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp
+++ b/llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp
@@ -438,7 +438,7 @@ bool LoongArchFrameLowering::spillCalleeSavedRegisters(
 
   // Insert the spill to the stack frame.
   for (auto &CS : CSI) {
-    Register Reg = CS.getReg();
+    MCRegister Reg = CS.getReg();
     // If the register is RA and the return address is taken by method
     // LoongArchTargetLowering::lowerRETURNADDR, don't set kill flag.
     bool IsKill =
diff --git...
[truncated]

Copy link

github-actions bot commented Feb 21, 2025

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff d46902e31b29974e56285bbc6932c90656f6dd12 b0f65b160f445af1f238c173958bb331dbc61005 --extensions h,cpp -- llvm/include/llvm/CodeGen/MachineFrameInfo.h llvm/lib/CodeGen/LivePhysRegs.cpp llvm/lib/CodeGen/PrologEpilogInserter.cpp llvm/lib/Target/AArch64/AArch64FrameLowering.cpp llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp llvm/lib/Target/AArch64/AArch64RegisterInfo.h llvm/lib/Target/AMDGPU/SIFrameLowering.cpp llvm/lib/Target/ARC/ARCFrameLowering.cpp llvm/lib/Target/ARM/ARMFrameLowering.cpp llvm/lib/Target/ARM/Thumb1FrameLowering.cpp llvm/lib/Target/AVR/AVRFrameLowering.cpp llvm/lib/Target/CSKY/CSKYFrameLowering.cpp llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp llvm/lib/Target/LoongArch/LoongArchFrameLowering.cpp llvm/lib/Target/M68k/M68kFrameLowering.cpp llvm/lib/Target/MSP430/MSP430FrameLowering.cpp llvm/lib/Target/Mips/Mips16FrameLowering.cpp llvm/lib/Target/Mips/MipsSEFrameLowering.cpp llvm/lib/Target/PowerPC/PPCFrameLowering.cpp llvm/lib/Target/RISCV/RISCVFrameLowering.cpp llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp llvm/lib/Target/X86/X86FrameLowering.cpp llvm/lib/Target/XCore/XCoreFrameLowering.cpp llvm/lib/Target/Xtensa/XtensaFrameLowering.cpp
View the diff from clang-format here.
diff --git a/llvm/include/llvm/CodeGen/MachineFrameInfo.h b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
index cf9c757a97..2112373dfd 100644
--- a/llvm/include/llvm/CodeGen/MachineFrameInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
@@ -58,7 +58,7 @@ public:
   explicit CalleeSavedInfo(unsigned R, int FI = 0) : Reg(R), FrameIdx(FI) {}
 
   // Accessors.
-  MCRegister getReg()                      const { return Reg; }
+  MCRegister getReg() const { return Reg; }
   int getFrameIdx()                        const { return FrameIdx; }
   unsigned getDstReg()                     const { return DstReg; }
   void setFrameIdx(int FI) {
diff --git a/llvm/lib/Target/ARM/ARMFrameLowering.cpp b/llvm/lib/Target/ARM/ARMFrameLowering.cpp
index 6e885ab574..7304e78ce9 100644
--- a/llvm/lib/Target/ARM/ARMFrameLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMFrameLowering.cpp
@@ -1668,7 +1668,7 @@ void ARMFrameLowering::emitPushInst(MachineBasicBlock &MBB,
   while (i != 0) {
     unsigned LastReg = 0;
     for (; i != 0; --i) {
-      MCRegister Reg = CSI[i-1].getReg();
+      MCRegister Reg = CSI[i - 1].getReg();
       if (!Func(Reg))
         continue;
 

@topperc topperc merged commit af64f0a into llvm:main Feb 21, 2025
5 of 11 checks passed
@topperc topperc deleted the pr/calleesaved branch February 21, 2025 07:44
cor3ntin pushed a commit to cor3ntin/llvm-project that referenced this pull request Feb 21, 2025
…. NFC (llvm#128095)

Callee saved registers should always be phyiscal registers. They are
often passed directly to other functions that take MCRegister like
getMinimalPhysRegClass or TargetRegisterClass::contains.

Unfortunately, sometimes the MCRegister is compared to a Register which
gave an ambiguous comparison error when the MCRegister is on the LHS.
Adding a MCRegister==Register comparison operator created more ambiguous
comparison errors elsewhere. These cases were usually comparing against
a base or frame pointer register that is a physical register in a
Register. For those I added an explicit conversion of Register to
MCRegister to fix the error.
@jplehr
Copy link
Contributor

jplehr commented Feb 21, 2025

This appears to have broken our build of libc on GPU on this bot: https://lab.llvm.org/staging/#/builders/97
Can you please take a look?

/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/./bin/clang++ --target=amdgcn-amd-amdhsa -DLIBC_NAMESPACE=__llvm_libc_21_0_0_git -D__LIBC_USE_FLOAT16_CONVERSION -I/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc -isystem /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.build/include/amdgcn-amd-amdhsa -O3 -DNDEBUG --target=amdgcn-amd-amdhsa -DLIBC_QSORT_IMPL=LIBC_QSORT_QUICK_SORT -DLIBC_ADD_NULL_CHECKS "-DLIBC_MATH=(LIBC_MATH_SKIP_ACCURATE_PASS | LIBC_MATH_SMALL_TABLES | LIBC_MATH_NO_ERRNO | LIBC_MATH_NO_EXCEPT)" -fpie -DLIBC_FULL_BUILD -nostdlibinc -ffixed-point -fno-exceptions -fno-lax-vector-conversions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -ftrivial-auto-var-init=pattern -fno-omit-frame-pointer -Wall -Wextra -Werror -Wconversion -Wno-sign-conversion -Wdeprecated -Wno-c99-extensions -Wno-gnu-imaginary-constant -Wno-pedantic -Wimplicit-fallthrough -Wwrite-strings -Wextra-semi -Wnewline-eof -Wnonportable-system-include-path -Wstrict-prototypes -Wthread-safety -Wglobal-constructors -nogpulib -fvisibility=hidden -fconvergent-functions -flto -Wno-multi-gpu -Xclang -mcode-object-version=none -DLIBC_COPT_PUBLIC_PACKAGING -UNDEBUG -MD -MT libc/src/strings/CMakeFiles/libc.src.strings.bcopy.dir/bcopy.cpp.o -MF libc/src/strings/CMakeFiles/libc.src.strings.bcopy.dir/bcopy.cpp.o.d -o libc/src/strings/CMakeFiles/libc.src.strings.bcopy.dir/bcopy.cpp.o -c /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/strings/bcopy.cpp
In file included from /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/strings/bcopy.cpp:12:
In file included from /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/string/memory_utils/inline_memmove.h:39:
In file included from /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/string/memory_utils/generic/builtin.h:14:
In file included from /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/string/memory_utils/utils.h:15:
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/__support/endian_internal.h:43:13: error: unknown type name 'uint8_t'
   43 | LIBC_INLINE uint8_t
      |             ^
/home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/libc/src/__support/endian_internal.h:44:48: error: use of undeclared identifier 'uint8_t'
   44 | Endian<__ORDER_LITTLE_ENDIAN__>::to_big_endian<uint8_t>(uint8_t v) {
      |

Edit: I looked closer at the change and wonder how this would break the build.
I'll start investigating myself, too.

@jplehr
Copy link
Contributor

jplehr commented Feb 21, 2025

Disregard the previous comment. The build appears fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants