Skip to content

AMDGPU: Move some code out of macro for defining regclass decoder #155755

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

Closed
wants to merge 3 commits into from
Closed

AMDGPU: Move some code out of macro for defining regclass decoder #155755

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f5a3321
AMDGPU: Move some code out of macro for defining regclass decoder
arsenm Aug 28, 2025
5ccb1a6
Use lambda to forward arguments
arsenm Aug 28, 2025
e6cd6ef
Revert "Use lambda to forward arguments"
arsenm Aug 28, 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
25 changes: 15 additions & 10 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 @@ -146,17 +146,22 @@ static DecodeStatus decodeDpp8FI(MCInst &Inst, unsigned Val, uint64_t Addr,
return addOperand(Inst, DAsm->DecoderName(Imm)); \
}

// Decoder for registers, decode directly using RegClassID. Imm(8-bit) is
// number of register. Used by VGPR only and AGPR only operands.
// Decoder for registers, decode directly using RegClassID. Imm(8-bit) is number
// of register. Used by VGPR only and AGPR only operands.
template <unsigned RegClassID>
static DecodeStatus decodeRegisterClassImpl(MCInst &Inst, unsigned Imm,
uint64_t /*Addr*/,
const MCDisassembler *Decoder) {
assert(Imm < (1 << 8) && "8-bit encoding");
auto DAsm = static_cast<const AMDGPUDisassembler *>(Decoder);
return addOperand(Inst, DAsm->createRegOperand(RegClassID, Imm));
}

using RegClassDecoder = decltype(&decodeRegisterClassImpl<0>);

#define DECODE_OPERAND_REG_8(RegClass) \
static DecodeStatus Decode##RegClass##RegisterClass( \
MCInst &Inst, unsigned Imm, uint64_t /*Addr*/, \
const MCDisassembler *Decoder) { \
assert(Imm < (1 << 8) && "8-bit encoding"); \
auto DAsm = static_cast<const AMDGPUDisassembler *>(Decoder); \
return addOperand( \
Inst, DAsm->createRegOperand(AMDGPU::RegClass##RegClassID, Imm)); \
}
static const constexpr RegClassDecoder Decode##RegClass##RegisterClass = \
decodeRegisterClassImpl<AMDGPU::RegClass##RegClassID>;
Comment on lines +163 to +164
Copy link
Contributor

@Pierre-vh Pierre-vh Aug 28, 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
static const constexpr RegClassDecoder Decode##RegClass##RegisterClass = \
decodeRegisterClassImpl<AMDGPU::RegClass##RegClassID>;
static const constexpr RegClassDecoder Decode##RegClass##RegisterClass = [](/* params go here*/){return decodeRegisterClassImpl(AMDGPU::RegClass##RegClassID, /* forward rest here */);

What about a constexpr lambda + no template ?
Could use std::bind too but I don't know if that's constexpr everywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That might as well be a macro at that point, the reason I did it this was was to avoid repeating the long argument list 3 times

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static const constexpr RegClassDecoder Decode##RegClass##RegisterClass = \
decodeRegisterClassImpl<AMDGPU::RegClass##RegClassID>;
static const constexpr RegClassDecoder Decode##RegClass##RegisterClass = [](auto... args) { return decodeRegisterClassImpl(AMDGPU::RegClass##RegClassID, args...); }

?

Copy link
Contributor

Choose a reason for hiding this comment

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

Though with that one I think you need to make the RegClassDecoder signature more explicit, or even manually type it out, so you still need to type the args twice unfortunately

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#define DECODE_OPERAND_REG_8(RegClass)                                         \
  static const constexpr auto Decode##RegClass##RegisterClass =                \
      [](auto... args) {                                                       \
        return decodeRegisterClassImpl<AMDGPU::RegClass##RegClassID>(args...); \
      };

Works, but it seems to cost about 5 seconds of build time


#define DECODE_SrcOp(Name, EncSize, OpWidth, EncImm) \
static DecodeStatus Name(MCInst &Inst, unsigned Imm, uint64_t /*Addr*/, \
Expand Down