Skip to content

[RISCV][NFC] Adopt DiagnosticString interface #126290

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 4 commits into from
Feb 8, 2025
Merged

[RISCV][NFC] Adopt DiagnosticString interface #126290

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9998ba6
[RISCV][NFC] Adopt DiagnosticString interface
lenary Feb 7, 2025
70498da
[RISCV][NFC] DiagnosticString TableGen Crimes
lenary Feb 7, 2025
7e3d6a6
fixup! [RISCV][NFC] Adopt DiagnosticString interface
lenary Feb 7, 2025
a33d9ab
Revert "[RISCV][NFC] DiagnosticString TableGen Crimes"
lenary Feb 8, 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
107 changes: 5 additions & 102 deletions llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ class RISCVAsmParser : public MCTargetAsmParser {
std::unique_ptr<RISCVOperand> defaultFRMArgLegacyOp() const;

public:
enum RISCVMatchResultTy {
enum RISCVMatchResultTy : unsigned {
Match_Dummy = FIRST_TARGET_MATCH_RESULT_TY,
#define GET_OPERAND_DIAGNOSTIC_TYPES
#include "RISCVGenAsmMatcher.inc"
Expand Down Expand Up @@ -1524,10 +1524,6 @@ bool RISCVAsmParser::matchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
std::numeric_limits<uint32_t>::max(),
"operand either must be a bare symbol name or an immediate integer in "
"the range");
case Match_InvalidImmZero: {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return Error(ErrorLoc, "immediate must be zero");
}
case Match_InvalidUImmLog2XLen:
if (isRV64())
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 6) - 1);
Expand All @@ -1536,45 +1532,15 @@ bool RISCVAsmParser::matchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
if (isRV64())
return generateImmOutOfRangeError(Operands, ErrorInfo, 1, (1 << 6) - 1);
return generateImmOutOfRangeError(Operands, ErrorInfo, 1, (1 << 5) - 1);
case Match_InvalidUImm1:
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 1) - 1);
case Match_InvalidUImm2:
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 2) - 1);
case Match_InvalidUImm2Lsb0:
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, 2,
"immediate must be one of");
case Match_InvalidUImm3:
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 3) - 1);
case Match_InvalidUImm4:
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 4) - 1);
case Match_InvalidUImm5:
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 5) - 1);
case Match_InvalidUImm5NonZero:
return generateImmOutOfRangeError(Operands, ErrorInfo, 1, (1 << 5) - 1);
case Match_InvalidUImm5GT3:
return generateImmOutOfRangeError(Operands, ErrorInfo, 4, (1 << 5) - 1);
case Match_InvalidUImm6:
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 6) - 1);
case Match_InvalidUImm7:
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 7) - 1);
case Match_InvalidUImm8:
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 8) - 1);
case Match_InvalidUImm8GE32:
return generateImmOutOfRangeError(Operands, ErrorInfo, 32, (1 << 8) - 1);
case Match_InvalidSImm5:
return generateImmOutOfRangeError(Operands, ErrorInfo, -(1 << 4),
(1 << 4) - 1);
case Match_InvalidSImm6:
return generateImmOutOfRangeError(Operands, ErrorInfo, -(1 << 5),
(1 << 5) - 1);
case Match_InvalidSImm6NonZero:
return generateImmOutOfRangeError(
Operands, ErrorInfo, -(1 << 5), (1 << 5) - 1,
"immediate must be non-zero in the range");
case Match_InvalidCLUIImm:
return generateImmOutOfRangeError(
Operands, ErrorInfo, 1, (1 << 5) - 1,
"immediate must be in [0xfffe0, 0xfffff] or");
case Match_InvalidUImm5Lsb0:
return generateImmOutOfRangeError(
Operands, ErrorInfo, 0, (1 << 5) - 2,
Expand Down Expand Up @@ -1611,10 +1577,6 @@ bool RISCVAsmParser::matchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
return generateImmOutOfRangeError(
Operands, ErrorInfo, -(1 << 9), (1 << 9) - 16,
"immediate must be a multiple of 16 bytes and non-zero in the range");
case Match_InvalidUImm10:
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 10) - 1);
case Match_InvalidUImm11:
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 11) - 1);
case Match_InvalidSImm12:
return generateImmOutOfRangeError(
Operands, ErrorInfo, -(1 << 11), (1 << 11) - 1,
Expand All @@ -1637,8 +1599,6 @@ bool RISCVAsmParser::matchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
"operand must be a symbol with "
"%hi/%tprel_hi modifier or an integer in "
"the range");
case Match_InvalidUImm20:
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 20) - 1);
case Match_InvalidUImm20AUIPC:
return generateImmOutOfRangeError(
Operands, ErrorInfo, 0, (1 << 20) - 1,
Expand All @@ -1649,80 +1609,23 @@ bool RISCVAsmParser::matchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
return generateImmOutOfRangeError(
Operands, ErrorInfo, -(1 << 20), (1 << 20) - 2,
"immediate must be a multiple of 2 bytes in the range");
case Match_InvalidCSRSystemRegister: {
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 12) - 1,
"operand must be a valid system register "
"name or an integer in the range");
}
case Match_InvalidLoadFPImm: {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return Error(ErrorLoc, "operand must be a valid floating-point constant");
}
case Match_InvalidBareSymbol: {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return Error(ErrorLoc, "operand must be a bare symbol name");
}
case Match_InvalidPseudoJumpSymbol: {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return Error(ErrorLoc, "operand must be a valid jump target");
}
case Match_InvalidCallSymbol: {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return Error(ErrorLoc, "operand must be a bare symbol name");
}
case Match_InvalidTPRelAddSymbol: {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return Error(ErrorLoc, "operand must be a symbol with %tprel_add modifier");
}
case Match_InvalidTLSDESCCallSymbol: {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return Error(ErrorLoc,
"operand must be a symbol with %tlsdesc_call modifier");
}
case Match_InvalidRTZArg: {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return Error(ErrorLoc, "operand must be 'rtz' floating-point rounding mode");
}
case Match_InvalidVTypeI: {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return generateVTypeError(ErrorLoc);
}
case Match_InvalidVMaskRegister: {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return Error(ErrorLoc, "operand must be v0.t");
}
case Match_InvalidVMaskCarryInRegister: {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return Error(ErrorLoc, "operand must be v0");
}
case Match_InvalidSImm5Plus1: {
return generateImmOutOfRangeError(Operands, ErrorInfo, -(1 << 4) + 1,
(1 << 4),
"immediate must be in the range");
}
case Match_InvalidSImm26:
return generateImmOutOfRangeError(Operands, ErrorInfo, -(1 << 25),
(1 << 25) - 1);
case Match_InvalidRlist: {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return Error(
ErrorLoc,
"operand must be {ra [, s0[-sN]]} or {x1 [, x8[-x9][, x18[-xN]]]}");
}
case Match_InvalidStackAdj: {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return Error(
ErrorLoc,
"stack adjustment is invalid for this instruction and register list; "
"refer to Zc spec for a detailed range of stack adjustment");
}
case Match_InvalidRnumArg: {
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, 10);
}
case Match_InvalidRegReg: {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return Error(ErrorLoc, "operands must be register and register");
}

if (const char *MatchDiag = getMatchKindDiag((RISCVMatchResultTy)Result)) {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return Error(ErrorLoc, MatchDiag);
}

llvm_unreachable("Unknown match type detected!");
Expand Down
26 changes: 24 additions & 2 deletions llvm/lib/Target/RISCV/RISCVInstrInfo.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ def ImmZeroAsmOperand : AsmOperandClass {
let Name = "ImmZero";
let RenderMethod = "addImmOperands";
let DiagnosticType = !strconcat("Invalid", Name);
let DiagnosticString = "immediate must be zero";
}

// A parse method for (${gpr}) or 0(${gpr}), where the 0 is be silently ignored.
Expand All @@ -150,12 +151,26 @@ def SPMem : MemOperand<SP>;

def GPRCMem : MemOperand<GPRC>;

Copy link
Collaborator

@topperc topperc Feb 7, 2025

Choose a reason for hiding this comment

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

I guess this makes a bunch of similar strings in the final binary that can no longer be shared?

Copy link
Member Author

@lenary lenary Feb 7, 2025

Choose a reason for hiding this comment

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

That's my expectation, yes. TableGen crimes have consequences, it turns out.

Copy link
Collaborator

@topperc topperc Feb 7, 2025

Choose a reason for hiding this comment

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

I wonder how much work it be to introduce "DiagnosticCode" to allow us to programmatically generate the string like we do now

Copy link
Member

@mshockwave mshockwave Feb 7, 2025

Choose a reason for hiding this comment

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

I thought these (materialized) strings from TableGen code will eventually be put as normal C strings, so in the final compiler binaries similar string will still be deduplicated. Though these strings wouldn't be deduplicated in the *.inc files generated from TableGen I believe (though I think we can somehow solve this by changing how TableGen generates this part of the code)

Copy link
Collaborator

@topperc topperc Feb 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@mshockwave the range strings will all have a similar beginning and end but the lower and upper bounds are part of the string and will be different values. Without tablegen, the prefix and suffix are separate strings and the lower and upper bounds are converted to strings at runtime.

Copy link
Member

@mshockwave mshockwave Feb 7, 2025

Choose a reason for hiding this comment

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

range strings will all have a similar beginning and end

oh right, I thought we're only talking about fixed strings.

Copy link
Member Author

@lenary lenary Feb 8, 2025

Choose a reason for hiding this comment

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

I wonder how much work it be to introduce "DiagnosticCode" to allow us to programmatically generate the string like we do now

This feels like overenginnering. I'm like down three side-rabbit-holes now, and going to retreat a bit.

I will undo the tablegen crimes, the C++ is better for craig's reasons, but where we have fixed strings this should make things easier.

class SImmAsmOperand<int width, string suffix = "">
class RISCVRangedImmDiagnosticString<int lo, int hi, string prefix = "immediate must be an integer in the range"> {
string Message = prefix # " [" # lo # ", " # hi # "]";
}

class RISCVSImmDiagnosticString<int bitsNum, string prefix = "immediate must be an integer in the range"> :
RISCVRangedImmDiagnosticString<!sub(0, !shl(1, !sub(bitsNum, 1))), !sub(!shl(1, !sub(bitsNum, 1)), 1), prefix> {
}

class RISCVUImmDiagnosticString<int bitsNum, string prefix = "immediate must be an integer in the range"> :
RISCVRangedImmDiagnosticString<0, !sub(!shl(1, bitsNum), 1), prefix> {
}

class SImmAsmOperand<int width, string suffix = "", string messagePrefix = "immediate must be an integer in the range">
: ImmAsmOperand<"S", width, suffix> {
let DiagnosticString = RISCVSImmDiagnosticString<width, messagePrefix>.Message;
}

class UImmAsmOperand<int width, string suffix = "">
class UImmAsmOperand<int width, string suffix = "", string messagePrefix = "immediate must be an integer in the range">
: ImmAsmOperand<"U", width, suffix> {
let DiagnosticString = RISCVUImmDiagnosticString<width, messagePrefix>.Message;
}

class RISCVOp<ValueType vt = XLenVT> : Operand<vt> {
Expand Down Expand Up @@ -323,6 +338,7 @@ def BareSymbol : AsmOperandClass {
let Name = "BareSymbol";
let RenderMethod = "addImmOperands";
let DiagnosticType = "InvalidBareSymbol";
let DiagnosticString = "operand must be a bare symbol name";
let ParserMethod = "parseBareSymbol";
}

Expand All @@ -335,6 +351,7 @@ def CallSymbol : AsmOperandClass {
let Name = "CallSymbol";
let RenderMethod = "addImmOperands";
let DiagnosticType = "InvalidCallSymbol";
let DiagnosticString = "operand must be a bare symbol name";
let ParserMethod = "parseCallSymbol";
}

Expand All @@ -347,6 +364,7 @@ def PseudoJumpSymbol : AsmOperandClass {
let Name = "PseudoJumpSymbol";
let RenderMethod = "addImmOperands";
let DiagnosticType = "InvalidPseudoJumpSymbol";
let DiagnosticString = "operand must be a valid jump target";
let ParserMethod = "parsePseudoJumpSymbol";
}

Expand All @@ -359,6 +377,7 @@ def TPRelAddSymbol : AsmOperandClass {
let Name = "TPRelAddSymbol";
let RenderMethod = "addImmOperands";
let DiagnosticType = "InvalidTPRelAddSymbol";
let DiagnosticString = "operand must be a symbol with %tprel_add modifier";
let ParserMethod = "parseOperandWithModifier";
}

Expand All @@ -371,6 +390,8 @@ def CSRSystemRegister : AsmOperandClass {
let Name = "CSRSystemRegister";
let ParserMethod = "parseCSRSystemRegister";
let DiagnosticType = "InvalidCSRSystemRegister";
let DiagnosticString = RISCVUImmDiagnosticString<12, "operand must be a valid system register "
"name or an integer in the range">.Message;
}

def csr_sysreg : RISCVOp, TImmLeaf<XLenVT, "return isUInt<12>(Imm);"> {
Expand Down Expand Up @@ -1779,6 +1800,7 @@ def TLSDESCCallSymbol : AsmOperandClass {
let Name = "TLSDESCCallSymbol";
let RenderMethod = "addImmOperands";
let DiagnosticType = "InvalidTLSDESCCallSymbol";
let DiagnosticString = "operand must be a symbol with %tlsdesc_call modifier";
let ParserMethod = "parseOperandWithModifier";
}

Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/RISCV/RISCVInstrInfoC.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def simm6 : RISCVSImmLeafOp<6> {

def simm6nonzero : RISCVOp,
ImmLeaf<XLenVT, [{return (Imm != 0) && isInt<6>(Imm);}]> {
let ParserMatchClass = SImmAsmOperand<6, "NonZero">;
let ParserMatchClass = SImmAsmOperand<6, "NonZero", "immediate must be non-zero in the range">;
let EncoderMethod = "getImmOpValue";
let DecoderMethod = "decodeSImmNonZeroOperand<6>";
let OperandType = "OPERAND_SIMM6_NONZERO";
Expand All @@ -69,6 +69,7 @@ def CLUIImmAsmOperand : AsmOperandClass {
let Name = "CLUIImm";
let RenderMethod = "addImmOperands";
let DiagnosticType = !strconcat("Invalid", Name);
let DiagnosticString = "immediate must be in [0xfffe0, 0xfffff] or [1, 31]";
}


Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Target/RISCV/RISCVInstrInfoV.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,15 @@ def VMaskAsmOperand : AsmOperandClass {
let IsOptional = 1;
let DefaultMethod = "defaultMaskRegOp";
let DiagnosticType = "InvalidVMaskRegister";
let DiagnosticString = "operand must be v0.t";
}

def VMaskCarryInAsmOperand : AsmOperandClass {
let Name = "RVVMaskCarryInRegOpOperand";
let RenderMethod = "addRegOperands";
let PredicateMethod = "isV0Reg";
let DiagnosticType = "InvalidVMaskCarryInRegister";
let DiagnosticString = "operand must be v0";
}

def VMaskOp : RegisterOperand<VMV0> {
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Target/RISCV/RISCVInstrInfoXCV.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,7 @@ def CVrrAsmOperand : AsmOperandClass {
let Name = "RegReg";
let ParserMethod = "parseRegReg";
let DiagnosticType = "InvalidRegReg";
let DiagnosticString = "operands must be register and register";
}

def CVrr : Operand<i32>,
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,15 @@ def RlistAsmOperand : AsmOperandClass {
let Name = "Rlist";
let ParserMethod = "parseReglist";
let DiagnosticType = "InvalidRlist";
let DiagnosticString = "operand must be {ra [, s0[-sN]]} or {x1 [, x8[-x9][, x18[-xN]]]}";
}

def StackAdjAsmOperand : AsmOperandClass {
let Name = "StackAdj";
let ParserMethod = "parseZcmpStackAdj";
let DiagnosticType = "InvalidStackAdj";
let DiagnosticString = "stack adjustment is invalid for this instruction and register list; "
"refer to Zc spec for a detailed range of stack adjustment";
let PredicateMethod = "isSpimm";
let RenderMethod = "addSpimmOperands";
}
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Target/RISCV/RISCVInstrInfoZfa.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def LoadFPImmOperand : AsmOperandClass {
let ParserMethod = "parseFPImm";
let RenderMethod = "addFPImmOperands";
let DiagnosticType = "InvalidLoadFPImm";
let DiagnosticString = "operand must be a valid floating-point constant";
}

def loadfpimm : Operand<XLenVT> {
Expand All @@ -43,6 +44,7 @@ def RTZArg : AsmOperandClass {
let Name = "RTZArg";
let RenderMethod = "addFRMArgOperands";
let DiagnosticType = "InvalidRTZArg";
let DiagnosticString = "operand must be 'rtz' floating-point rounding mode";
let ParserMethod = "parseFRMArg";
}

Expand Down