AMDGPU: Move some code out of macro for defining regclass decoder #155755
Conversation
|
@llvm/pr-subscribers-backend-amdgpu Author: Matt Arsenault (arsenm) ChangesUse a template function for the implementation, and use the macro This will help avoid another copy-paste version of this function Full diff: https://github.com/llvm/llvm-project/pull/155755.diff 1 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
index 6a2beeed41dfd..8651ddc89dce2 100644
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
@@ -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>;
#define DECODE_SrcOp(Name, EncSize, OpWidth, EncImm) \
static DecodeStatus Name(MCInst &Inst, unsigned Imm, uint64_t /*Addr*/, \
|
| static const constexpr RegClassDecoder Decode##RegClass##RegisterClass = \ | ||
| decodeRegisterClassImpl<AMDGPU::RegClass##RegClassID>; |
There was a problem hiding this comment.
| 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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
| 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...); } |
?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
#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
Use a template function for the implementation, and use the macro to define a constant function pointer with the expected name. Not sure if there's a cleaner way to do this. This worked out to less code using variadic templates to forward the arguments, but it added a noticable ~10 seconds to compilation time on this file. This will help avoid another copy-paste version of this function in a future change.
This seems to cost 4-5 seconds in build time in the file
This reverts commit 79d03ff80ed3ac9d8f072955c2bae5d7567ff56a.
0f88c29 to
e6cd6ef
Compare
|
ping, due to the compile time issue I think this should avoid the template. I'm thinking of opening a clang bug for it |
Use a template function for the implementation, and use the macro
to define a constant function pointer with the expected name. Not
sure if there's a cleaner way to do this. This worked out to less
code using variadic templates to forward the arguments, but it added
a noticable ~10 seconds to compilation time on this file.
This will help avoid another copy-paste version of this function
in a future change.