Skip to content

[AMDGPU] Remove implicit conversions of MCRegister to unsigned. NFC #167284

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 11, 2025
Merged

[AMDGPU] Remove implicit conversions of MCRegister to unsigned. NFC #167284

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
65817d7
[AMDGPU] Remove implicit conversions of MCRegister to unsigned. NFC
topperc Nov 10, 2025
962590d
fixup! RegId->Reg
topperc Nov 11, 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
2 changes: 1 addition & 1 deletion llvm/lib/Target/AMDGPU/AMDGPUArgumentUsageInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ struct ArgDescriptor {
}

static ArgDescriptor createArg(const ArgDescriptor &Arg, unsigned Mask) {
return ArgDescriptor(Arg.Reg, Mask, Arg.IsStack, Arg.IsSet);
return ArgDescriptor(Arg.Reg.id(), Mask, Arg.IsStack, Arg.IsSet);
}

bool isSet() const {
Expand Down
9 changes: 4 additions & 5 deletions llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1199,8 +1199,8 @@ void AMDGPUDisassembler::convertVOP3DPPInst(MCInst &MI) const {

// Given a wide tuple \p Reg check if it will overflow 256 registers.
// \returns \p Reg on success or NoRegister otherwise.
static unsigned CheckVGPROverflow(unsigned Reg, const MCRegisterClass &RC,
const MCRegisterInfo &MRI) {
static MCRegister CheckVGPROverflow(MCRegister Reg, const MCRegisterClass &RC,
const MCRegisterInfo &MRI) {
unsigned NumRegs = RC.getSizeInBits() / 32;
MCRegister Sub0 = MRI.getSubReg(Reg, AMDGPU::sub0);
if (!Sub0)
Expand All @@ -1214,7 +1214,7 @@ static unsigned CheckVGPROverflow(unsigned Reg, const MCRegisterClass &RC,

assert(BaseReg && "Only vector registers expected");

return (Sub0 - BaseReg + NumRegs <= 256) ? Reg : AMDGPU::NoRegister;
return (Sub0 - BaseReg + NumRegs <= 256) ? Reg : MCRegister();
Copy link
Contributor

@s-barannikov s-barannikov Nov 11, 2025

Choose a reason for hiding this comment

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

Sub0/BaseReg are implicitly converted to unsigned?

Copy link
Collaborator Author

@topperc topperc Nov 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I currently have a operator-(MCRegister, MCRegister) in my tree to reduce the number of changes needed to make it manageable. That's hiding the implicit conversion here from my build. I'm not sure if we should that operator in the end but it is helpful for migration.

Copy link
Contributor

@s-barannikov s-barannikov Nov 11, 2025

Choose a reason for hiding this comment

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

If you're inclined to leaving the operator out, [[deprecated]] on it could help detect such cases without breaking the build

}

// Note that before gfx10, the MIMG encoding provided no information about
Expand Down Expand Up @@ -1456,8 +1456,7 @@ MCOperand AMDGPUDisassembler::errOperand(unsigned V,
return MCOperand();
}

inline
MCOperand AMDGPUDisassembler::createRegOperand(unsigned int RegId) const {
inline MCOperand AMDGPUDisassembler::createRegOperand(MCRegister RegId) const {
return MCOperand::createReg(AMDGPU::getMCReg(RegId, STI));
Copy link
Contributor

@s-barannikov s-barannikov Nov 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
inline MCOperand AMDGPUDisassembler::createRegOperand(MCRegister RegId) const {
return MCOperand::createReg(AMDGPU::getMCReg(RegId, STI));
inline MCOperand AMDGPUDisassembler::createRegOperand(MCRegister Reg) const {
return MCOperand::createReg(AMDGPU::getMCReg(Reg, STI));

}

Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class AMDGPUDisassembler : public MCDisassembler {

const char* getRegClassName(unsigned RegClassID) const;

MCOperand createRegOperand(unsigned int RegId) const;
MCOperand createRegOperand(MCRegister RegId) const;
Copy link
Contributor

@s-barannikov s-barannikov Nov 11, 2025

Choose a reason for hiding this comment

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

Suggested change
MCOperand createRegOperand(MCRegister RegId) const;
MCOperand createRegOperand(MCRegister Reg) const;

MCOperand createRegOperand(unsigned RegClassID, unsigned Val) const;
MCOperand createSRegOperand(unsigned SRegClassID, unsigned Val) const;
MCOperand createVGPR16Operand(unsigned RegIdx, bool IsHi) const;
Expand Down
14 changes: 7 additions & 7 deletions llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ void AMDGPUInstPrinter::printSymbolicFormat(const MCInst *MI,

// \returns a low 256 vgpr representing a high vgpr \p Reg [v256..v1023] or
// \p Reg itself otherwise.
static MCPhysReg getRegForPrinting(MCPhysReg Reg, const MCRegisterInfo &MRI) {
static MCRegister getRegForPrinting(MCRegister Reg, const MCRegisterInfo &MRI) {
unsigned Enc = MRI.getEncodingValue(Reg);
unsigned Idx = Enc & AMDGPU::HWEncoding::REG_IDX_MASK;
if (Idx < 0x100)
Expand All @@ -355,10 +355,10 @@ static MCPhysReg getRegForPrinting(MCPhysReg Reg, const MCRegisterInfo &MRI) {
}

// Restore MSBs of a VGPR above 255 from the MCInstrAnalysis.
static MCPhysReg getRegFromMIA(MCPhysReg Reg, unsigned OpNo,
const MCInstrDesc &Desc,
const MCRegisterInfo &MRI,
const AMDGPUMCInstrAnalysis &MIA) {
static MCRegister getRegFromMIA(MCRegister Reg, unsigned OpNo,
const MCInstrDesc &Desc,
const MCRegisterInfo &MRI,
const AMDGPUMCInstrAnalysis &MIA) {
unsigned VgprMSBs = MIA.getVgprMSBs();
if (!VgprMSBs)
return Reg;
Expand Down Expand Up @@ -403,10 +403,10 @@ void AMDGPUInstPrinter::printRegOperand(MCRegister Reg, raw_ostream &O,
}
#endif

unsigned PrintReg = getRegForPrinting(Reg, MRI);
MCRegister PrintReg = getRegForPrinting(Reg, MRI);
O << getRegisterName(PrintReg);

if (PrintReg != Reg.id())
if (PrintReg != Reg)
O << " /*" << getRegisterName(Reg) << "*/";
}

Expand Down
29 changes: 15 additions & 14 deletions llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,7 @@ unsigned ComponentInfo::getIndexInParsedOperands(unsigned CompOprIdx) const {
}

std::optional<unsigned> InstInfo::getInvalidCompOperandIndex(
std::function<unsigned(unsigned, unsigned)> GetRegIdx,
std::function<MCRegister(unsigned, unsigned)> GetRegIdx,
const MCRegisterInfo &MRI, bool SkipSrc, bool AllowSameVGPR,
bool VOPD3) const {

Expand All @@ -914,12 +914,13 @@ std::optional<unsigned> InstInfo::getInvalidCompOperandIndex(
BaseX = X;
if (!BaseY)
BaseY = Y;
if ((BaseX & BanksMask) == (BaseY & BanksMask))
if ((BaseX.id() & BanksMask) == (BaseY.id() & BanksMask))
return true;
if (BaseX != X /* This is 64-bit register */ &&
((BaseX + 1) & BanksMask) == (BaseY & BanksMask))
((BaseX.id() + 1) & BanksMask) == (BaseY.id() & BanksMask))
return true;
if (BaseY != Y && (BaseX & BanksMask) == ((BaseY + 1) & BanksMask))
if (BaseY != Y &&
(BaseX.id() & BanksMask) == ((BaseY.id() + 1) & BanksMask))
return true;

// If both are 64-bit bank conflict will be detected yet while checking
Expand Down Expand Up @@ -968,7 +969,7 @@ std::optional<unsigned> InstInfo::getInvalidCompOperandIndex(
// if the operand is not a register or not a VGPR.
InstInfo::RegIndices
InstInfo::getRegIndices(unsigned CompIdx,
std::function<unsigned(unsigned, unsigned)> GetRegIdx,
std::function<MCRegister(unsigned, unsigned)> GetRegIdx,
bool VOPD3) const {
assert(CompIdx < COMPONENTS_NUM);

Expand All @@ -983,7 +984,7 @@ InstInfo::getRegIndices(unsigned CompIdx,
Comp.hasRegSrcOperand(CompSrcIdx)
? GetRegIdx(CompIdx,
Comp.getIndexOfSrcInMCOperands(CompSrcIdx, VOPD3))
: 0;
: MCRegister();
}
return RegIndices;
}
Expand Down Expand Up @@ -2697,8 +2698,8 @@ MCRegister getMCReg(MCRegister Reg, const MCSubtargetInfo &STI) {

MCRegister mc2PseudoReg(MCRegister Reg) { MAP_REG2REG }

bool isInlineValue(unsigned Reg) {
switch (Reg) {
bool isInlineValue(MCRegister Reg) {
switch (Reg.id()) {
case AMDGPU::SRC_SHARED_BASE_LO:
case AMDGPU::SRC_SHARED_BASE:
case AMDGPU::SRC_SHARED_LIMIT_LO:
Expand Down Expand Up @@ -3361,7 +3362,7 @@ const GcnBufferFormatInfo *getGcnBufferFormatInfo(uint8_t Format,
: getGfx9BufferFormatInfo(Format);
}

const MCRegisterClass *getVGPRPhysRegClass(MCPhysReg Reg,
const MCRegisterClass *getVGPRPhysRegClass(MCRegister Reg,
const MCRegisterInfo &MRI) {
const unsigned VGPRClasses[] = {
AMDGPU::VGPR_16RegClassID, AMDGPU::VGPR_32RegClassID,
Expand All @@ -3382,22 +3383,22 @@ const MCRegisterClass *getVGPRPhysRegClass(MCPhysReg Reg,
return nullptr;
}

unsigned getVGPREncodingMSBs(MCPhysReg Reg, const MCRegisterInfo &MRI) {
unsigned getVGPREncodingMSBs(MCRegister Reg, const MCRegisterInfo &MRI) {
unsigned Enc = MRI.getEncodingValue(Reg);
unsigned Idx = Enc & AMDGPU::HWEncoding::REG_IDX_MASK;
return Idx >> 8;
}

MCPhysReg getVGPRWithMSBs(MCPhysReg Reg, unsigned MSBs,
const MCRegisterInfo &MRI) {
MCRegister getVGPRWithMSBs(MCRegister Reg, unsigned MSBs,
const MCRegisterInfo &MRI) {
unsigned Enc = MRI.getEncodingValue(Reg);
unsigned Idx = Enc & AMDGPU::HWEncoding::REG_IDX_MASK;
if (Idx >= 0x100)
return AMDGPU::NoRegister;
return MCRegister();

const MCRegisterClass *RC = getVGPRPhysRegClass(Reg, MRI);
if (!RC)
return AMDGPU::NoRegister;
return MCRegister();

Idx |= MSBs << 8;
if (RC->getID() == AMDGPU::VGPR_16RegClassID) {
Expand Down
23 changes: 12 additions & 11 deletions llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,7 @@ class InstInfo {
const ComponentInfo CompInfo[COMPONENTS_NUM];

public:
using RegIndices = std::array<unsigned, Component::MAX_OPR_NUM>;
using RegIndices = std::array<MCRegister, Component::MAX_OPR_NUM>;

InstInfo(const MCInstrDesc &OpX, const MCInstrDesc &OpY)
: CompInfo{OpX, OpY} {}
Expand All @@ -932,9 +932,10 @@ class InstInfo {
// even though it violates requirement to be from different banks.
// If \p VOPD3 is set to true both dst registers allowed to be either odd
// or even and instruction may have real src2 as opposed to tied accumulator.
bool hasInvalidOperand(std::function<unsigned(unsigned, unsigned)> GetRegIdx,
const MCRegisterInfo &MRI, bool SkipSrc = false,
bool AllowSameVGPR = false, bool VOPD3 = false) const {
bool
hasInvalidOperand(std::function<MCRegister(unsigned, unsigned)> GetRegIdx,
const MCRegisterInfo &MRI, bool SkipSrc = false,
bool AllowSameVGPR = false, bool VOPD3 = false) const {
return getInvalidCompOperandIndex(GetRegIdx, MRI, SkipSrc, AllowSameVGPR,
VOPD3)
.has_value();
Expand All @@ -949,14 +950,14 @@ class InstInfo {
// If \p VOPD3 is set to true both dst registers allowed to be either odd
// or even and instruction may have real src2 as opposed to tied accumulator.
std::optional<unsigned> getInvalidCompOperandIndex(
std::function<unsigned(unsigned, unsigned)> GetRegIdx,
std::function<MCRegister(unsigned, unsigned)> GetRegIdx,
const MCRegisterInfo &MRI, bool SkipSrc = false,
bool AllowSameVGPR = false, bool VOPD3 = false) const;

private:
RegIndices
getRegIndices(unsigned ComponentIdx,
std::function<unsigned(unsigned, unsigned)> GetRegIdx,
std::function<MCRegister(unsigned, unsigned)> GetRegIdx,
bool VOPD3) const;
};

Expand Down Expand Up @@ -1599,7 +1600,7 @@ LLVM_READNONE
MCRegister mc2PseudoReg(MCRegister Reg);

LLVM_READNONE
bool isInlineValue(unsigned Reg);
bool isInlineValue(MCRegister Reg);

/// Is this an AMDGPU specific source operand? These include registers,
/// inline constants, literals and mandatory literals (KImm).
Expand Down Expand Up @@ -1798,16 +1799,16 @@ bool isIntrinsicAlwaysUniform(unsigned IntrID);

/// \returns a register class for the physical register \p Reg if it is a VGPR
/// or nullptr otherwise.
const MCRegisterClass *getVGPRPhysRegClass(MCPhysReg Reg,
const MCRegisterClass *getVGPRPhysRegClass(MCRegister Reg,
const MCRegisterInfo &MRI);

/// \returns the MODE bits which have to be set by the S_SET_VGPR_MSB for the
/// physical register \p Reg.
unsigned getVGPREncodingMSBs(MCPhysReg Reg, const MCRegisterInfo &MRI);
unsigned getVGPREncodingMSBs(MCRegister Reg, const MCRegisterInfo &MRI);

/// If \p Reg is a low VGPR return a corresponding high VGPR with \p MSBs set.
MCPhysReg getVGPRWithMSBs(MCPhysReg Reg, unsigned MSBs,
const MCRegisterInfo &MRI);
MCRegister getVGPRWithMSBs(MCRegister Reg, unsigned MSBs,
const MCRegisterInfo &MRI);

// Returns a table for the opcode with a given \p Desc to map the VGPR MSB
// set by the S_SET_VGPR_MSB to one of 4 sources. In case of VOPD returns 2
Expand Down