-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[AMDGPU][NFCI] Decouple actual register encodings from HWEncoding values. #69452
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
Conversation
@llvm/pr-subscribers-backend-amdgpu Author: Ivan Kosarev (kosarev) ChangesThe HWEncoding values currently form a strange mix of actual register codes for some subtargets and types of operands and informational flags. This patch removes the dependency allowing arbitrary changes in the structure of HWEncoding values without breaking register encodings. Such changes, in turn, would make it possible to speed up and simplify getAVOperandEncoding() testing for AGPRs as well as other functions dealing with register codes downstream. They would also allow to maintain the same format of HWEncoding values across our downstream code bases, thus simplifying merging in mainline changes. Full diff: https://github.com/llvm/llvm-project/pull/69452.diff 5 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
index d74fd0b3a9ea74e..5bfc354a815608c 100644
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
@@ -38,9 +38,16 @@ using namespace llvm;
#define DEBUG_TYPE "amdgpu-disassembler"
-#define SGPR_MAX \
- (isGFX10Plus() ? AMDGPU::EncValues::SGPR_MAX_GFX10 \
- : AMDGPU::EncValues::SGPR_MAX_SI)
+enum : unsigned {
+ INLINE_INTEGER_C_MIN = 128,
+ INLINE_INTEGER_C_POSITIVE_MAX = 192, // 64
+ INLINE_INTEGER_C_MAX = 208,
+ INLINE_FLOATING_C_MIN = 240,
+ INLINE_FLOATING_C_MAX = 248,
+ LITERAL_CONST = 255,
+};
+
+#define SGPR_MAX (isGFX10Plus() ? 105 : 101)
using DecodeStatus = llvm::MCDisassembler::DecodeStatus;
@@ -143,11 +150,11 @@ static DecodeStatus decodeBoolReg(MCInst &Inst, unsigned Val, uint64_t Addr,
// Decoder for registers. Imm(10-bit): Imm{7-0} is number of register,
// Imm{9} is acc(agpr or vgpr) Imm{8} should be 0 (see VOP3Pe_SMFMAC).
-// Set Imm{8} to 1 (IS_VGPR) to decode using 'enum10' from decodeSrcOp.
+// Set Imm{8} to 1 (is-VGPR) to decode using 'enum10' from decodeSrcOp.
// Used by AV_ register classes (AGPR or VGPR only register operands).
#define DECODE_OPERAND_REG_AV10(RegClass, OpWidth) \
- DECODE_SrcOp(Decode##RegClass##RegisterClass, 10, OpWidth, \
- Imm | AMDGPU::EncValues::IS_VGPR, false, 0)
+ DECODE_SrcOp(Decode##RegClass##RegisterClass, 10, OpWidth, Imm | 0x100, \
+ false, 0)
// Decoder for Src(9-bit encoding) registers only.
#define DECODE_OPERAND_SRC_REG_9(RegClass, OpWidth) \
@@ -1121,8 +1128,7 @@ DecodeStatus AMDGPUDisassembler::convertFMAanyK(MCInst &MI,
auto OpType = Desc.operands()[I].OperandType;
bool IsDeferredOp = (OpType == AMDGPU::OPERAND_REG_IMM_FP32_DEFERRED ||
OpType == AMDGPU::OPERAND_REG_IMM_FP16_DEFERRED);
- if (Op.isImm() && Op.getImm() == AMDGPU::EncValues::LITERAL_CONST &&
- IsDeferredOp)
+ if (Op.isImm() && Op.getImm() == LITERAL_CONST && IsDeferredOp)
Op.setImm(Literal);
}
return MCDisassembler::Success;
@@ -1246,8 +1252,6 @@ MCOperand AMDGPUDisassembler::decodeLiteralConstant(bool ExtendFP64) const {
}
MCOperand AMDGPUDisassembler::decodeIntImmed(unsigned Imm) {
- using namespace AMDGPU::EncValues;
-
assert(Imm >= INLINE_INTEGER_C_MIN && Imm <= INLINE_INTEGER_C_MAX);
return MCOperand::createImm((Imm <= INLINE_INTEGER_C_POSITIVE_MAX) ?
(static_cast<int64_t>(Imm) - INLINE_INTEGER_C_MIN) :
@@ -1331,8 +1335,7 @@ static int64_t getInlineImmVal16(unsigned Imm) {
}
MCOperand AMDGPUDisassembler::decodeFPImmed(unsigned ImmWidth, unsigned Imm) {
- assert(Imm >= AMDGPU::EncValues::INLINE_FLOATING_C_MIN
- && Imm <= AMDGPU::EncValues::INLINE_FLOATING_C_MAX);
+ assert(Imm >= INLINE_FLOATING_C_MIN && Imm <= INLINE_FLOATING_C_MAX);
// ToDo: case 248: 1/(2*PI) - is allowed only on VI
// ImmWidth 0 is a default case where operand should not allow immediates.
@@ -1449,10 +1452,8 @@ unsigned AMDGPUDisassembler::getTtmpClassId(const OpWidthTy Width) const {
}
int AMDGPUDisassembler::getTTmpIdx(unsigned Val) const {
- using namespace AMDGPU::EncValues;
-
- unsigned TTmpMin = isGFX9Plus() ? TTMP_GFX9PLUS_MIN : TTMP_VI_MIN;
- unsigned TTmpMax = isGFX9Plus() ? TTMP_GFX9PLUS_MAX : TTMP_VI_MAX;
+ unsigned TTmpMin = isGFX9Plus() ? 108 : 112;
+ unsigned TTmpMax = isGFX9Plus() ? 123 : 123;
return (TTmpMin <= Val && Val <= TTmpMax)? Val - TTmpMin : -1;
}
@@ -1460,16 +1461,14 @@ int AMDGPUDisassembler::getTTmpIdx(unsigned Val) const {
MCOperand AMDGPUDisassembler::decodeSrcOp(const OpWidthTy Width, unsigned Val,
bool MandatoryLiteral,
unsigned ImmWidth, bool IsFP) const {
- using namespace AMDGPU::EncValues;
-
assert(Val < 1024); // enum10
bool IsAGPR = Val & 512;
Val &= 511;
- if (VGPR_MIN <= Val && Val <= VGPR_MAX) {
- return createRegOperand(IsAGPR ? getAgprClassId(Width)
- : getVgprClassId(Width), Val - VGPR_MIN);
+ if (Val >= 256) {
+ return createRegOperand(
+ IsAGPR ? getAgprClassId(Width) : getVgprClassId(Width), Val - 256);
}
return decodeNonVGPRSrcOp(Width, Val & 0xFF, MandatoryLiteral, ImmWidth,
IsFP);
@@ -1483,13 +1482,9 @@ MCOperand AMDGPUDisassembler::decodeNonVGPRSrcOp(const OpWidthTy Width,
// Cases when Val{8} is 1 (vgpr, agpr or true 16 vgpr) should have been
// decoded earlier.
assert(Val < (1 << 8) && "9-bit Src encoding when Val{8} is 0");
- using namespace AMDGPU::EncValues;
- if (Val <= SGPR_MAX) {
- // "SGPR_MIN <= Val" is always true and causes compilation warning.
- static_assert(SGPR_MIN == 0);
- return createSRegOperand(getSgprClassId(Width), Val - SGPR_MIN);
- }
+ if (Val <= SGPR_MAX)
+ return createSRegOperand(getSgprClassId(Width), Val);
int TTmpIdx = getTTmpIdx(Val);
if (TTmpIdx >= 0) {
@@ -1608,7 +1603,6 @@ MCOperand AMDGPUDisassembler::decodeSDWASrc(const OpWidthTy Width,
const unsigned Val,
unsigned ImmWidth) const {
using namespace AMDGPU::SDWA;
- using namespace AMDGPU::EncValues;
if (STI.hasFeature(AMDGPU::FeatureGFX9) ||
STI.hasFeature(AMDGPU::FeatureGFX10)) {
diff --git a/llvm/lib/Target/AMDGPU/FLATInstructions.td b/llvm/lib/Target/AMDGPU/FLATInstructions.td
index 533013a3130c05f..143e65bc8d89c96 100644
--- a/llvm/lib/Target/AMDGPU/FLATInstructions.td
+++ b/llvm/lib/Target/AMDGPU/FLATInstructions.td
@@ -1982,7 +1982,7 @@ multiclass FLAT_Real_SADDR_RTN_gfx10<bits<7> op> {
multiclass FLAT_Real_ST_gfx10<bits<7> op> {
def _ST_gfx10 :
FLAT_Real_gfx10<op, !cast<FLAT_Pseudo>(NAME#"_ST")> {
- let Inst{54-48} = !cast<int>(EXEC_HI.HWEncoding);
+ let Inst{54-48} = EXEC_HI.Index;
let OtherPredicates = [HasFlatScratchSTMode];
}
}
@@ -2203,13 +2203,13 @@ multiclass FLAT_Aliases_gfx11<string ps, string opName, int renamed> {
multiclass FLAT_Real_Base_gfx11<bits<7> op, string ps, string opName, int renamed = false> :
FLAT_Aliases_gfx11<ps, opName, renamed> {
def _gfx11 : FLAT_Real_gfx11<op, !cast<FLAT_Pseudo>(ps), opName> {
- let Inst{54-48} = !cast<int>(SGPR_NULL_gfx11plus.HWEncoding);
+ let Inst{54-48} = SGPR_NULL_gfx11plus.Index;
}
}
multiclass FLAT_Real_RTN_gfx11<bits<7> op, string ps, string opName> {
def _RTN_gfx11 : FLAT_Real_gfx11<op, !cast<FLAT_Pseudo>(ps#"_RTN"), opName> {
- let Inst{54-48} = !cast<int>(SGPR_NULL_gfx11plus.HWEncoding);
+ let Inst{54-48} = SGPR_NULL_gfx11plus.Index;
}
}
@@ -2223,7 +2223,7 @@ multiclass FLAT_Real_SADDR_RTN_gfx11<bits<7> op, string ps, string opName> {
multiclass FLAT_Real_ST_gfx11<bits<7> op, string ps, string opName> {
def _ST_gfx11 : FLAT_Real_gfx11<op, !cast<FLAT_Pseudo>(ps#"_ST"), opName> {
- let Inst{54-48} = !cast<int>(SGPR_NULL_gfx11plus.HWEncoding);
+ let Inst{54-48} = SGPR_NULL_gfx11plus.Index;
let OtherPredicates = [HasFlatScratchSTMode];
}
}
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp
index 88c1668f62800aa..55d249898110286 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp
@@ -353,7 +353,8 @@ void AMDGPUMCCodeEmitter::encodeInstruction(const MCInst &MI,
// However, dst is encoded as EXEC for compatibility with SP3.
if (AMDGPU::isGFX10Plus(STI) && isVCMPX64(Desc)) {
assert((Encoding & 0xFF) == 0);
- Encoding |= MRI.getEncodingValue(AMDGPU::EXEC_LO);
+ Encoding |=
+ MRI.getEncodingValue(AMDGPU::EXEC_LO) & AMDGPU::EncValues::REG_IDX_MASK;
}
for (unsigned i = 0; i < bytes; i++) {
@@ -499,11 +500,14 @@ void AMDGPUMCCodeEmitter::getAVOperandEncoding(
const MCInst &MI, unsigned OpNo, APInt &Op,
SmallVectorImpl<MCFixup> &Fixups, const MCSubtargetInfo &STI) const {
unsigned Reg = MI.getOperand(OpNo).getReg();
- uint64_t Enc = MRI.getEncodingValue(Reg);
+ unsigned Enc = MRI.getEncodingValue(Reg);
+ unsigned Idx = Enc & AMDGPU::EncValues::REG_IDX_MASK;
+ bool IsVGPR = Enc & AMDGPU::EncValues::IS_VGPR;
// VGPR and AGPR have the same encoding, but SrcA and SrcB operands of mfma
// instructions use acc[0:1] modifier bits to distinguish. These bits are
// encoded as a virtual 9th bit of the register for these operands.
+ bool IsAGPR = false;
if (MRI.getRegClass(AMDGPU::AGPR_32RegClassID).contains(Reg) ||
MRI.getRegClass(AMDGPU::AReg_64RegClassID).contains(Reg) ||
MRI.getRegClass(AMDGPU::AReg_96RegClassID).contains(Reg) ||
@@ -518,9 +522,9 @@ void AMDGPUMCCodeEmitter::getAVOperandEncoding(
MRI.getRegClass(AMDGPU::AReg_384RegClassID).contains(Reg) ||
MRI.getRegClass(AMDGPU::AReg_512RegClassID).contains(Reg) ||
MRI.getRegClass(AMDGPU::AGPR_LO16RegClassID).contains(Reg))
- Enc |= 512;
+ IsAGPR = true;
- Op = Enc;
+ Op = Idx | (IsVGPR << 8) | (IsAGPR << 9);
}
static bool needsPCRel(const MCExpr *Expr) {
@@ -551,7 +555,10 @@ void AMDGPUMCCodeEmitter::getMachineOpValue(const MCInst &MI,
SmallVectorImpl<MCFixup> &Fixups,
const MCSubtargetInfo &STI) const {
if (MO.isReg()){
- Op = MRI.getEncodingValue(MO.getReg());
+ unsigned Enc = MRI.getEncodingValue(MO.getReg());
+ unsigned Idx = Enc & AMDGPU::EncValues::REG_IDX_MASK;
+ bool IsVGPR = Enc & AMDGPU::EncValues::IS_VGPR;
+ Op = Idx | (IsVGPR << 8);
return;
}
unsigned OpNo = &MO - MI.begin();
diff --git a/llvm/lib/Target/AMDGPU/SIDefines.h b/llvm/lib/Target/AMDGPU/SIDefines.h
index bb38c0c9be00ff0..d2ddc89ba784cbb 100644
--- a/llvm/lib/Target/AMDGPU/SIDefines.h
+++ b/llvm/lib/Target/AMDGPU/SIDefines.h
@@ -311,25 +311,11 @@ namespace AMDGPUAsmVariants {
} // namespace AMDGPUAsmVariants
namespace AMDGPU {
-namespace EncValues { // Encoding values of enum9/8/7 operands
+namespace EncValues {
+// Register codes as defined in the TableGen's HWEncoding field.
enum : unsigned {
REG_IDX_MASK = 255,
- SGPR_MIN = 0,
- SGPR_MAX_SI = 101,
- SGPR_MAX_GFX10 = 105,
- TTMP_VI_MIN = 112,
- TTMP_VI_MAX = 123,
- TTMP_GFX9PLUS_MIN = 108,
- TTMP_GFX9PLUS_MAX = 123,
- INLINE_INTEGER_C_MIN = 128,
- INLINE_INTEGER_C_POSITIVE_MAX = 192, // 64
- INLINE_INTEGER_C_MAX = 208,
- INLINE_FLOATING_C_MIN = 240,
- INLINE_FLOATING_C_MAX = 248,
- LITERAL_CONST = 255,
- VGPR_MIN = 256,
- VGPR_MAX = 511,
IS_VGPR = 256, // Indicates VGPR or AGPR
IS_HI = 512, // High 16-bit register.
};
diff --git a/llvm/lib/Target/AMDGPU/SIRegisterInfo.td b/llvm/lib/Target/AMDGPU/SIRegisterInfo.td
index ea06e85fb400c1b..4c16e9692c66af8 100644
--- a/llvm/lib/Target/AMDGPU/SIRegisterInfo.td
+++ b/llvm/lib/Target/AMDGPU/SIRegisterInfo.td
@@ -128,6 +128,8 @@ class SIReg <string n, bits<8> regIdx = 0, bit isAGPROrVGPR = 0,
let HWEncoding{7-0} = regIdx;
let HWEncoding{8} = isAGPROrVGPR;
let HWEncoding{9} = isHi;
+
+ int Index = !cast<int>(regIdx);
}
// For register classes that use TSFlags.
@@ -164,6 +166,8 @@ multiclass SIRegLoHi16 <string n, bits<8> regIdx, bit ArtificialHigh = 1,
let CoveredBySubRegs = !not(ArtificialHigh);
let HWEncoding{7-0} = regIdx;
let HWEncoding{8} = isAGPROrVGPR;
+
+ int Index = !cast<int>(regIdx);
}
}
|
#define SGPR_MAX \ | ||
(isGFX10Plus() ? AMDGPU::EncValues::SGPR_MAX_GFX10 \ | ||
: AMDGPU::EncValues::SGPR_MAX_SI) | ||
enum : unsigned { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the point of moving it here? Technically we should use it in the AMDGPUMCCodeEmitter.cpp too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the encoding side isn't interested in the range boundaries much (which makes sense?). LITERAL_CONST
could be used there, but currently isn't, so maybe it can wait for when we need it in a wider scope? (I wouldn't mind it either way.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was specifically surprised that LITERAL_CONST isn't used. I do not see moving it to decoding side a good thing, I would rather replace uses of 255 on the encoding side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. What namespace do we want LITERAL_CONST
to be declared in? Would it make sense to rename EncValues
to HWEncoding
and introduce another EncValues
namespace for that kind of constants?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not have strong preference here, but why don't we start with replacing uses of literals with symbolic names in the encoder and see what are we really using?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this goes well beyond the scope of this patch and I personally wouldn't propose such a change, because as I already mentioned elsewhere introducing and maintaining this kind of named constants is not an obvious improvement to me (but wouldn't mind terribly either if someone else is going to propose that).
So I can give give LITERAL_CONST
some more use here, if that's seen critical, but would prefer to not deviate from the purposes of this patch trying to refine the encoder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the purpose of this patch I would just live the enum where it is now. The rest seems good.
Updated as suggested. |
You can test this locally with the following command:git-clang-format --diff 783b4d91c73c992fad32e045ce3265b01028fc99 81dfb156c003859e9502bbd4fb94cb9d77b242be -- llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCCodeEmitter.cpp llvm/lib/Target/AMDGPU/SIDefines.h llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp View the diff from clang-format here.diff --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
index ab50e98483ee..f2666b6dceb4 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
@@ -1162,11 +1162,10 @@ amdhsa::kernel_descriptor_t getDefaultAmdhsaKernelDescriptor(
AMDHSA_BITS_SET(KD.kernel_code_properties,
amdhsa::KERNEL_CODE_PROPERTY_ENABLE_WAVEFRONT_SIZE32,
STI->getFeatureBits().test(FeatureWavefrontSize32) ? 1 : 0);
- AMDHSA_BITS_SET(KD.compute_pgm_rsrc1,
- amdhsa::COMPUTE_PGM_RSRC1_WGP_MODE,
+ AMDHSA_BITS_SET(KD.compute_pgm_rsrc1, amdhsa::COMPUTE_PGM_RSRC1_WGP_MODE,
STI->getFeatureBits().test(FeatureCuMode) ? 0 : 1);
- AMDHSA_BITS_SET(KD.compute_pgm_rsrc1,
- amdhsa::COMPUTE_PGM_RSRC1_MEM_ORDERED, 1);
+ AMDHSA_BITS_SET(KD.compute_pgm_rsrc1, amdhsa::COMPUTE_PGM_RSRC1_MEM_ORDERED,
+ 1);
}
if (AMDGPU::isGFX90A(*STI)) {
AMDHSA_BITS_SET(KD.compute_pgm_rsrc3,
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please improve the documentation about what does HWEncoding actually mean on the AMDGPU target? And also document the meaning of EncValues?
// Used by AV_ register classes (AGPR or VGPR only register operands). | ||
#define DECODE_OPERAND_REG_AV10(RegClass, OpWidth) \ | ||
DECODE_SrcOp(Decode##RegClass##RegisterClass, 10, OpWidth, \ | ||
Imm | AMDGPU::EncValues::IS_VGPR, false, 0) | ||
DECODE_SrcOp(Decode##RegClass##RegisterClass, 10, OpWidth, Imm | 0x100, \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If AMDGPU::EncValues::IS_VGPR is misleading, we should define a new named value, not use a raw number.
if (VGPR_MIN <= Val && Val <= VGPR_MAX) { | ||
return createRegOperand(IsAGPR ? getAgprClassId(Width) | ||
: getVgprClassId(Width), Val - VGPR_MIN); | ||
if (Val >= 256) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If VGPR_MIN/ VGPR_MAX is misleading, we should define a new named value or refactor it, not use a raw number.
…ues. The HWEncoding values currently form a strange mix of actual register codes for some subtargets and types of operands and informational flags. This patch removes the dependency allowing arbitrary changes in the structure of HWEncoding values without breaking register encodings. Such changes, in turn, would make it possible to speed up and simplify getAVOperandEncoding() testing for AGPRs as well as other functions dealing with register codes downstream. They would also allow to maintain the same format of HWEncoding values across our downstream code bases, thus simplifying merging in mainline changes.
6126bea
to
81dfb15
Compare
As same questions about naming numbers keep coming up, I thought maybe proceeding by smaller steps will get us farther with this. I excluded changes related to |
uint64_t Enc = MRI.getEncodingValue(Reg); | ||
unsigned Enc = MRI.getEncodingValue(Reg); | ||
unsigned Idx = Enc & AMDGPU::HWEncoding::REG_IDX_MASK; | ||
bool IsVGPROrAGPR = Enc & AMDGPU::HWEncoding::IS_VGPR_OR_AGPR; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this is the only such place where an AGPR could actually encounter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is probably something in the decoder...
uint64_t Enc = MRI.getEncodingValue(Reg); | ||
unsigned Enc = MRI.getEncodingValue(Reg); | ||
unsigned Idx = Enc & AMDGPU::HWEncoding::REG_IDX_MASK; | ||
bool IsVGPROrAGPR = Enc & AMDGPU::HWEncoding::IS_VGPR_OR_AGPR; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is probably something in the decoder...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks.
The HWEncoding values currently form a strange mix of actual register codes for some subtargets and types of operands and informational flags. This patch removes the dependency allowing arbitrary changes in the structure of HWEncoding values without breaking register encodings.
Such changes, in turn, would make it possible to speed up and simplify getAVOperandEncoding() testing for AGPRs as well as other functions dealing with register codes downstream. They would also allow to maintain the same format of HWEncoding values across our downstream code bases, thus simplifying merging in mainline changes.