[TableGen] Slightly improve error location for a fatal error #170790
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Created using spr 1.3.8-beta.1
Member
|
@llvm/pr-subscribers-tablegen Author: Alexander Richardson (arichardson) ChangesI was hitting this error and the error location was pointing to the Happens with Full diff: https://github.com/llvm/llvm-project/pull/170790.diff 9 Files Affected:
diff --git a/llvm/test/TableGen/Common/RegClassByHwModeCommon.td b/llvm/test/TableGen/Common/RegClassByHwModeCommon.td
new file mode 100644
index 0000000000000..613fd2623ced6
--- /dev/null
+++ b/llvm/test/TableGen/Common/RegClassByHwModeCommon.td
@@ -0,0 +1,38 @@
+include "llvm/Target/Target.td"
+
+class MyReg<string n> : Register<n> {
+ let Namespace = "MyTarget";
+}
+
+class MyClass<int size, list<ValueType> types, dag registers>
+ : RegisterClass<"MyTarget", types, size, registers> {
+ let Size = size;
+}
+
+def X0 : MyReg<"x0">;
+def X1 : MyReg<"x1">;
+def X2 : MyReg<"x2">;
+def X3 : MyReg<"x3">;
+def XRegs : RegisterClass<"MyTarget", [i32], 32, (add X0, X1, X2, X3)>;
+def Y0 : MyReg<"y0">;
+def Y1 : MyReg<"y1">;
+def Y2 : MyReg<"y2">;
+def Y3 : MyReg<"y3">;
+def YRegs : RegisterClass<"MyTarget", [i64], 64, (add Y0, Y1, X2, X3)>;
+
+class TestInstruction : Instruction {
+ let Size = 2;
+ let Namespace = "MyTarget";
+ let hasSideEffects = false;
+ let hasExtraSrcRegAllocReq = false;
+ let hasExtraDefRegAllocReq = false;
+
+ field bits<16> Inst;
+ bits<3> dst;
+ bits<3> src;
+ bits<3> opcode;
+
+ let Inst{2-0} = dst;
+ let Inst{5-3} = src;
+ let Inst{7-5} = opcode;
+}
diff --git a/llvm/test/TableGen/RegClassByHwMode.td b/llvm/test/TableGen/RegClassByHwMode.td
index ec723f8b70478..187f98c0d01de 100644
--- a/llvm/test/TableGen/RegClassByHwMode.td
+++ b/llvm/test/TableGen/RegClassByHwMode.td
@@ -1,10 +1,10 @@
-// RUN: llvm-tblgen -gen-instr-info -I %p/../../include %s -o - | FileCheck -check-prefix=INSTRINFO %s
-// RUN: llvm-tblgen -gen-asm-matcher -I %p/../../include %s -o - | FileCheck -check-prefix=ASMMATCHER %s
-// RUN: llvm-tblgen -gen-disassembler -I %p/../../include %s -o - | FileCheck -check-prefix=DISASM %s
-// RUN: llvm-tblgen -gen-dag-isel -I %p/../../include %s -o - | FileCheck -check-prefix=ISEL-SDAG %s
-// RUN: llvm-tblgen -gen-global-isel -I %p/../../include %s -o - | FileCheck -check-prefix=ISEL-GISEL %s
+// RUN: llvm-tblgen -gen-instr-info -I %S -I %p/../../include %s -o - | FileCheck -check-prefix=INSTRINFO %s
+// RUN: llvm-tblgen -gen-asm-matcher -I %S -I %p/../../include %s -o - | FileCheck -check-prefix=ASMMATCHER %s
+// RUN: llvm-tblgen -gen-disassembler -I %S -I %p/../../include %s -o - | FileCheck -check-prefix=DISASM %s
+// RUN: llvm-tblgen -gen-dag-isel -I %S -I %p/../../include %s -o - | FileCheck -check-prefix=ISEL-SDAG %s
+// RUN: llvm-tblgen -gen-global-isel -I %S -I %p/../../include %s -o - | FileCheck -check-prefix=ISEL-GISEL %s
-include "llvm/Target/Target.td"
+include "Common/RegClassByHwModeCommon.td"
// INSTRINFO: #ifdef GET_INSTRINFO_ENUM
// INSTRINFO-NEXT: #undef GET_INSTRINFO_ENUM
@@ -302,8 +302,6 @@ include "llvm/Target/Target.td"
// ISEL-GISEL-NEXT: GIR_MutateOpcode, /*InsnID*/0, /*RecycleInsnID*/0, /*Opcode*/GIMT_Encode2(MyTarget::MY_STORE),
// ISEL-GISEL-NEXT: GIR_RootConstrainSelectedInstOperands,
-
-
def HasAlignedRegisters : Predicate<"Subtarget->hasAlignedRegisters()">;
def HasUnalignedRegisters : Predicate<"Subtarget->hasUnalignedRegisters()">;
def IsPtr64 : Predicate<"Subtarget->isPtr64()">;
@@ -317,34 +315,6 @@ def EvenMode : HwMode<[HasAlignedRegisters]>;
def OddMode : HwMode<[HasUnalignedRegisters]>;
def Ptr64 : HwMode<[IsPtr64]>;
-class MyReg<string n>
- : Register<n> {
- let Namespace = "MyTarget";
-}
-
-class MyClass<int size, list<ValueType> types, dag registers>
- : RegisterClass<"MyTarget", types, size, registers> {
- let Size = size;
-}
-
-def X0 : MyReg<"x0">;
-def X1 : MyReg<"x1">;
-def X2 : MyReg<"x2">;
-def X3 : MyReg<"x3">;
-def X4 : MyReg<"x4">;
-def X5 : MyReg<"x5">;
-def X6 : MyReg<"x6">;
-
-def Y0 : MyReg<"y0">;
-def Y1 : MyReg<"y1">;
-def Y2 : MyReg<"y2">;
-def Y3 : MyReg<"y3">;
-def Y4 : MyReg<"y4">;
-def Y5 : MyReg<"y5">;
-def Y6 : MyReg<"y6">;
-
-
-
def P0_32 : MyReg<"p0">;
def P1_32 : MyReg<"p1">;
def P2_32 : MyReg<"p2">;
@@ -385,23 +355,6 @@ def CustomDecodeYEvenIfRequired : RegisterOperand<YRegs_EvenIfRequired> {
let DecoderMethod = "YEvenIfRequiredCustomDecoder";
}
-class TestInstruction : Instruction {
- let Size = 2;
- let Namespace = "MyTarget";
- let hasSideEffects = false;
- let hasExtraSrcRegAllocReq = false;
- let hasExtraDefRegAllocReq = false;
-
- field bits<16> Inst;
- bits<3> dst;
- bits<3> src;
- bits<3> opcode;
-
- let Inst{2-0} = dst;
- let Inst{5-3} = src;
- let Inst{7-5} = opcode;
-}
-
def SpecialOperand : RegisterOperand<XRegs_EvenIfRequired>;
def MY_MOV : TestInstruction {
diff --git a/llvm/test/TableGen/RegClassByHwModeErrors.td b/llvm/test/TableGen/RegClassByHwModeErrors.td
new file mode 100644
index 0000000000000..d7d4258b918eb
--- /dev/null
+++ b/llvm/test/TableGen/RegClassByHwModeErrors.td
@@ -0,0 +1,47 @@
+// RUN: rm -rf %t && split-file %s %t
+// RUN: not llvm-tblgen --gen-asm-matcher -I %p/../../include -I %t -I %S \
+// RUN: %t/inst-alias-bad-reg.td -o /dev/null 2>&1 | FileCheck %t/inst-alias-bad-reg.td
+// RUN: not llvm-tblgen --gen-compress-inst-emitter -I %p/../../include -I %t -I %S \
+// RUN: %t/compress-regclass-by-hwmode.td -o /dev/null 2>&1 | FileCheck %t/compress-regclass-by-hwmode.td
+
+//--- Common.td
+include "Common/RegClassByHwModeCommon.td"
+
+def IsPtr64 : Predicate<"Subtarget->isPtr64()">;
+defvar Ptr32 = DefaultMode;
+def Ptr64 : HwMode<[IsPtr64]>;
+def PtrRC : RegClassByHwMode<[Ptr32, Ptr64], [XRegs, YRegs]>;
+
+def PTR_MOV : TestInstruction {
+ let OutOperandList = (outs PtrRC:$dst);
+ let InOperandList = (ins PtrRC:$src);
+ let AsmString = "ptr_mov $dst, $src";
+ let opcode = 0;
+}
+
+
+//--- inst-alias-bad-reg.td
+include "Common.td"
+// This should fail since X0 is not necessarily part of PtrRC.
+def BAD_REG : InstAlias<"ptr_zero $rd", (PTR_MOV PtrRC:$dst, X0)>;
+// CHECK: [[#@LINE-1]]:5: error: cannot resolve HwMode for PtrRC
+// CHECK: Common.td:6:5: note: PtrRC defined here
+def MyTargetISA : InstrInfo;
+def MyTarget : Target { let InstructionSet = MyTargetISA; }
+
+//--- compress-regclass-by-hwmode.td
+include "Common.td"
+def PTR_ZERO_SMALL : TestInstruction {
+ let OutOperandList = (outs PtrRC:$dst);
+ let InOperandList = (ins);
+ let AsmString = "ptr_zero $dst";
+ let opcode = 1;
+ let Size = 1;
+}
+// This should fail since X0 is not necessarily part of PtrRC.
+def : CompressPat<(PTR_MOV PtrRC:$dst, X0),
+ (PTR_ZERO_SMALL PtrRC:$dst)>;
+// CHECK: [[#@LINE-2]]:1: error: cannot resolve HwMode for PtrRC
+// CHECK: Common.td:6:5: note: PtrRC defined here
+def MyTargetISA : InstrInfo;
+def MyTarget : Target { let InstructionSet = MyTargetISA; }
diff --git a/llvm/utils/TableGen/Common/CodeGenInstAlias.cpp b/llvm/utils/TableGen/Common/CodeGenInstAlias.cpp
index 04d4855b81d3b..26d0d468108cf 100644
--- a/llvm/utils/TableGen/Common/CodeGenInstAlias.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenInstAlias.cpp
@@ -43,7 +43,8 @@ using ResultOperand = CodeGenInstAlias::ResultOperand;
static Expected<ResultOperand> matchSimpleOperand(const Init *Arg,
const StringInit *ArgName,
const Record *Op,
- const CodeGenTarget &T) {
+ const CodeGenTarget &T,
+ ArrayRef<SMLoc> Loc) {
if (const Record *OpRC = T.getAsRegClassLike(Op)) {
if (const auto *ArgDef = dyn_cast<DefInit>(Arg)) {
const Record *ArgRec = ArgDef->getDef();
@@ -52,7 +53,8 @@ static Expected<ResultOperand> matchSimpleOperand(const Init *Arg,
if (const Record *ArgRC = T.getInitValueAsRegClassLike(Arg)) {
if (ArgRC->isSubClassOf("RegisterClass")) {
if (!OpRC->isSubClassOf("RegisterClass") ||
- !T.getRegisterClass(OpRC).hasSubClass(&T.getRegisterClass(ArgRC)))
+ !T.getRegisterClass(OpRC, Loc).hasSubClass(
+ &T.getRegisterClass(ArgRC, Loc)))
return createStringError(
"argument register class " + ArgRC->getName() +
" is not a subclass of operand register class " +
@@ -70,7 +72,8 @@ static Expected<ResultOperand> matchSimpleOperand(const Init *Arg,
// Match 'Reg'.
if (ArgRec->isSubClassOf("Register")) {
- if (!T.getRegisterClass(OpRC).contains(T.getRegBank().getReg(ArgRec)))
+ if (!T.getRegisterClass(OpRC, Loc).contains(
+ T.getRegBank().getReg(ArgRec)))
return createStringError(
"register argument " + ArgRec->getName() +
" is not a member of operand register class " + OpRC->getName());
@@ -199,7 +202,8 @@ CodeGenInstAlias::CodeGenInstAlias(const Record *R, const CodeGenTarget &T)
const Record *SubOp =
cast<DefInit>(OpInfo.MIOperandInfo->getArg(SubOpIdx))->getDef();
Expected<ResultOperand> ResOpOrErr = matchSimpleOperand(
- ArgDag->getArg(SubOpIdx), ArgDag->getArgName(SubOpIdx), SubOp, T);
+ ArgDag->getArg(SubOpIdx), ArgDag->getArgName(SubOpIdx), SubOp, T,
+ R->getLoc());
if (!ResOpOrErr)
PrintFatalError(R, "in argument #" + Twine(ArgIdx) + "." +
Twine(SubOpIdx) + ": " +
@@ -220,8 +224,9 @@ CodeGenInstAlias::CodeGenInstAlias(const Record *R, const CodeGenTarget &T)
} else {
// Simple operand (RegisterClass, RegisterOperand or Operand with empty
// MIOperandInfo).
- Expected<ResultOperand> ResOpOrErr = matchSimpleOperand(
- Result->getArg(ArgIdx), Result->getArgName(ArgIdx), Op, T);
+ Expected<ResultOperand> ResOpOrErr =
+ matchSimpleOperand(Result->getArg(ArgIdx), Result->getArgName(ArgIdx),
+ Op, T, R->getLoc());
if (!ResOpOrErr)
PrintFatalError(R, "in argument #" + Twine(ArgIdx) + ": " +
toString(ResOpOrErr.takeError()));
diff --git a/llvm/utils/TableGen/Common/CodeGenRegisters.cpp b/llvm/utils/TableGen/Common/CodeGenRegisters.cpp
index e853303c37aff..e6d750bd9ca4c 100644
--- a/llvm/utils/TableGen/Common/CodeGenRegisters.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenRegisters.cpp
@@ -1316,11 +1316,18 @@ CodeGenRegBank::getOrCreateSubClass(const CodeGenRegisterClass *RC,
return {&RegClasses.back(), true};
}
-CodeGenRegisterClass *CodeGenRegBank::getRegClass(const Record *Def) const {
+CodeGenRegisterClass *CodeGenRegBank::getRegClass(const Record *Def,
+ ArrayRef<SMLoc> Loc) const {
if (CodeGenRegisterClass *RC = Def2RC.lookup(Def))
return RC;
- PrintFatalError(Def->getLoc(), "Not a known RegisterClass!");
+ ArrayRef<SMLoc> DiagLoc = Loc.empty() ? Def->getLoc() : Loc;
+ // TODO: Ideally we should update the API to allow resolving HwMode.
+ if (Def->isSubClassOf("RegClassByHwMode"))
+ PrintError(DiagLoc, "cannot resolve HwMode for " + Def->getName());
+ else
+ PrintError(DiagLoc, Def->getName() + " is not a known RegisterClass!");
+ PrintFatalNote(Def->getLoc(), Def->getName() + " defined here");
}
CodeGenSubRegIndex *
diff --git a/llvm/utils/TableGen/Common/CodeGenRegisters.h b/llvm/utils/TableGen/Common/CodeGenRegisters.h
index c02d04b648534..a3ad0b797a704 100644
--- a/llvm/utils/TableGen/Common/CodeGenRegisters.h
+++ b/llvm/utils/TableGen/Common/CodeGenRegisters.h
@@ -815,7 +815,8 @@ class CodeGenRegBank {
}
// Find a register class from its def.
- CodeGenRegisterClass *getRegClass(const Record *) const;
+ CodeGenRegisterClass *getRegClass(const Record *,
+ ArrayRef<SMLoc> Loc = {}) const;
/// getRegisterClassForRegister - Find the register class that contains the
/// specified physical register. If the register is not in a register
diff --git a/llvm/utils/TableGen/Common/CodeGenTarget.cpp b/llvm/utils/TableGen/Common/CodeGenTarget.cpp
index f093a61e5a6f6..cca318a6e2047 100644
--- a/llvm/utils/TableGen/Common/CodeGenTarget.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenTarget.cpp
@@ -173,8 +173,8 @@ const CodeGenRegister *CodeGenTarget::getRegisterByName(StringRef Name) const {
}
const CodeGenRegisterClass &
-CodeGenTarget::getRegisterClass(const Record *R) const {
- return *getRegBank().getRegClass(R);
+CodeGenTarget::getRegisterClass(const Record *R, ArrayRef<SMLoc> Loc) const {
+ return *getRegBank().getRegClass(R, Loc);
}
std::vector<ValueTypeByHwMode>
diff --git a/llvm/utils/TableGen/Common/CodeGenTarget.h b/llvm/utils/TableGen/Common/CodeGenTarget.h
index d7390c72ea352..dc1740b174148 100644
--- a/llvm/utils/TableGen/Common/CodeGenTarget.h
+++ b/llvm/utils/TableGen/Common/CodeGenTarget.h
@@ -131,7 +131,8 @@ class CodeGenTarget {
return RegAltNameIndices;
}
- const CodeGenRegisterClass &getRegisterClass(const Record *R) const;
+ const CodeGenRegisterClass &getRegisterClass(const Record *R,
+ ArrayRef<SMLoc> Loc = {}) const;
/// Convenience wrapper to avoid hardcoding the name of RegClassByHwMode
/// everywhere. This is here instead of CodeGenRegBank to avoid the fatal
diff --git a/llvm/utils/TableGen/CompressInstEmitter.cpp b/llvm/utils/TableGen/CompressInstEmitter.cpp
index d8c5ca7c1e1a3..94fe3823fae22 100644
--- a/llvm/utils/TableGen/CompressInstEmitter.cpp
+++ b/llvm/utils/TableGen/CompressInstEmitter.cpp
@@ -142,7 +142,8 @@ class CompressInstEmitter {
void emitCompressInstEmitter(raw_ostream &OS, EmitterType EType);
bool validateTypes(const Record *DagOpType, const Record *InstOpType,
bool IsSourceInst);
- bool validateRegister(const Record *Reg, const Record *RegClass);
+ bool validateRegister(const Record *Reg, const Record *RegClass,
+ ArrayRef<SMLoc> Loc);
void checkDagOperandMapping(const Record *Rec,
const StringMap<ArgData> &DestOperands,
const DagInit *SourceDag, const DagInit *DestDag);
@@ -162,11 +163,12 @@ class CompressInstEmitter {
} // End anonymous namespace.
bool CompressInstEmitter::validateRegister(const Record *Reg,
- const Record *RegClass) {
+ const Record *RegClass,
+ ArrayRef<SMLoc> Loc) {
assert(Reg->isSubClassOf("Register") && "Reg record should be a Register");
- assert(RegClass->isSubClassOf("RegisterClass") &&
- "RegClass record should be a RegisterClass");
- const CodeGenRegisterClass &RC = Target.getRegisterClass(RegClass);
+ assert(RegClass->isSubClassOf("RegisterClassLike") &&
+ "RegClass record should be RegisterClassLike");
+ const CodeGenRegisterClass &RC = Target.getRegisterClass(RegClass, Loc);
const CodeGenRegister *R = Target.getRegBank().getReg(Reg);
assert(R != nullptr && "Register not defined!!");
return RC.contains(R);
@@ -255,7 +257,7 @@ void CompressInstEmitter::addDagOperandMapping(const Record *Rec,
if (const auto *DI = dyn_cast<DefInit>(Dag->getArg(DAGOpNo))) {
if (DI->getDef()->isSubClassOf("Register")) {
// Check if the fixed register belongs to the Register class.
- if (!validateRegister(DI->getDef(), OpndRec))
+ if (!validateRegister(DI->getDef(), OpndRec, Rec->getLoc()))
PrintFatalError(Rec->getLoc(),
"Error in Dag '" + Dag->getAsString() +
"'Register: '" + DI->getDef()->getName() +
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
topperc
reviewed
Dec 5, 2025
| assert(RegClass->isSubClassOf("RegisterClass") && | ||
| "RegClass record should be a RegisterClass"); | ||
| const CodeGenRegisterClass &RC = Target.getRegisterClass(RegClass); | ||
| assert(RegClass->isSubClassOf("RegisterClassLike") && |
Member
Author
There was a problem hiding this comment.
Without this change we get an assertion failure instead of an error in the second test that I added
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
Member
Author
|
Will fix formatting and the test failure tomorrow |
arsenm
approved these changes
Dec 5, 2025
s-barannikov
reviewed
Dec 5, 2025
|
|
||
| PrintFatalError(Def->getLoc(), "Not a known RegisterClass!"); | ||
| ArrayRef<SMLoc> DiagLoc = Loc.empty() ? Def->getLoc() : Loc; | ||
| // TODO: Ideally we should update the API to allow resolving HwMode. |
Contributor
There was a problem hiding this comment.
Is that even possible? HwModes are resolved at runtime
Member
Author
There was a problem hiding this comment.
In some cases (e.g. inside a let Predicates = [...] block), we may be able to know exactly what it resolves to but that will require a lot more refactoring. I can drop the comment if you prefer.
Contributor
There was a problem hiding this comment.
Maybe in those cases we should resolve it manually in td?
Contributor
There was a problem hiding this comment.
No, that would not be very ergonomic. The point is to have reusable patterns and not have to spam every possible detail through every deeply nested multi class
Member
Author
There was a problem hiding this comment.
I've added a test case for this. If there are no further comments I'll submit this tomorrow.
Created using spr 1.3.8-beta.1
Created using spr 1.3.8-beta.1
arsenm
reviewed
Dec 8, 2025
| (PTR_MOV PtrRC:$dst, PtrRC:$src)>; | ||
| // CHECK: [[#@LINE-2]]:1: error: Type mismatch between Input and Output Dag operand 'dst' | ||
| def MyTargetISA : InstrInfo; | ||
| def MyTarget : Target { let InstructionSet = MyTargetISA; } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I was hitting this error and the error location was pointing to the
register class definition instead of the incorrect InstAlias. Pass in
the InstAlias location to make it easier to debug.
Happens with
def : InstAlias<"foo", (Inst X0)>, whereInsttakesa RegClassByHwMode operand that is not necessarily satisfied by
register X0. Similar problem with the CompressPat backend.