Skip to content

Commit 5079260

Browse files
arichardsonarichardson
authored
[TableGen] Slightly improve error location for a fatal error
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)>`, where `Inst` takes a RegClassByHwMode operand that is not necessarily satisfied by register X0. Similar problem with the CompressPat backend. Reviewed By: arsenm Pull Request: #170790
1 parent 9baf76a commit 5079260

File tree

9 files changed

+175
-78
lines changed

9 files changed

+175
-78
lines changed

‎llvm/test/TableGen/Common/RegClassByHwModeCommon.td‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
include "llvm/Target/Target.td"
2+
3+
class MyReg<string n> : Register<n> {
4+
let Namespace = "MyTarget";
5+
}
6+
7+
class MyClass<int size, list<ValueType> types, dag registers>
8+
: RegisterClass<"MyTarget", types, size, registers> {
9+
let Size = size;
10+
}
11+
12+
def X0 : MyReg<"x0">;
13+
def X1 : MyReg<"x1">;
14+
def X2 : MyReg<"x2">;
15+
def X3 : MyReg<"x3">;
16+
def X4 : MyReg<"x4">;
17+
def X5 : MyReg<"x5">;
18+
def X6 : MyReg<"x6">;
19+
def XRegs : RegisterClass<"MyTarget", [i64], 64, (add X0, X1, X2, X3, X4, X5, X6)>;
20+
def Y0 : MyReg<"y0">;
21+
def Y1 : MyReg<"y1">;
22+
def Y2 : MyReg<"y2">;
23+
def Y3 : MyReg<"y3">;
24+
def Y4 : MyReg<"y4">;
25+
def Y5 : MyReg<"y5">;
26+
def Y6 : MyReg<"y6">;
27+
def YRegs : RegisterClass<"MyTarget", [i64], 64, (add Y0, Y1, Y2, Y3, Y4, Y5, Y6)>;
28+
29+
class TestInstruction : Instruction {
30+
let Size = 2;
31+
let Namespace = "MyTarget";
32+
let hasSideEffects = false;
33+
let hasExtraSrcRegAllocReq = false;
34+
let hasExtraDefRegAllocReq = false;
35+
36+
field bits<16> Inst;
37+
bits<3> dst;
38+
bits<3> src;
39+
bits<3> opcode;
40+
41+
let Inst{2-0} = dst;
42+
let Inst{5-3} = src;
43+
let Inst{7-5} = opcode;
44+
}

‎llvm/test/TableGen/RegClassByHwMode.td‎

Lines changed: 7 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
// RUN: llvm-tblgen -gen-instr-info -I %p/../../include %s -o - | FileCheck -check-prefix=INSTRINFO %s
2-
// RUN: llvm-tblgen -gen-asm-matcher -I %p/../../include %s -o - | FileCheck -check-prefix=ASMMATCHER %s
3-
// RUN: llvm-tblgen -gen-disassembler -I %p/../../include %s -o - | FileCheck -check-prefix=DISASM %s
4-
// RUN: llvm-tblgen -gen-dag-isel -I %p/../../include %s -o - | FileCheck -check-prefix=ISEL-SDAG %s
5-
// RUN: llvm-tblgen -gen-global-isel -I %p/../../include %s -o - | FileCheck -check-prefix=ISEL-GISEL %s
1+
// RUN: llvm-tblgen -gen-instr-info -I %S -I %p/../../include %s -o - | FileCheck -check-prefix=INSTRINFO %s
2+
// RUN: llvm-tblgen -gen-asm-matcher -I %S -I %p/../../include %s -o - | FileCheck -check-prefix=ASMMATCHER %s
3+
// RUN: llvm-tblgen -gen-disassembler -I %S -I %p/../../include %s -o - | FileCheck -check-prefix=DISASM %s
4+
// RUN: llvm-tblgen -gen-dag-isel -I %S -I %p/../../include %s -o - | FileCheck -check-prefix=ISEL-SDAG %s
5+
// RUN: llvm-tblgen -gen-global-isel -I %S -I %p/../../include %s -o - | FileCheck -check-prefix=ISEL-GISEL %s
66

7-
include "llvm/Target/Target.td"
7+
include "Common/RegClassByHwModeCommon.td"
88

99
// INSTRINFO: #ifdef GET_INSTRINFO_ENUM
1010
// INSTRINFO-NEXT: #undef GET_INSTRINFO_ENUM
@@ -302,8 +302,6 @@ include "llvm/Target/Target.td"
302302
// ISEL-GISEL-NEXT: GIR_MutateOpcode, /*InsnID*/0, /*RecycleInsnID*/0, /*Opcode*/GIMT_Encode2(MyTarget::MY_STORE),
303303
// ISEL-GISEL-NEXT: GIR_RootConstrainSelectedInstOperands,
304304

305-
306-
307305
def HasAlignedRegisters : Predicate<"Subtarget->hasAlignedRegisters()">;
308306
def HasUnalignedRegisters : Predicate<"Subtarget->hasUnalignedRegisters()">;
309307
def IsPtr64 : Predicate<"Subtarget->isPtr64()">;
@@ -317,34 +315,6 @@ def EvenMode : HwMode<[HasAlignedRegisters]>;
317315
def OddMode : HwMode<[HasUnalignedRegisters]>;
318316
def Ptr64 : HwMode<[IsPtr64]>;
319317

320-
class MyReg<string n>
321-
: Register<n> {
322-
let Namespace = "MyTarget";
323-
}
324-
325-
class MyClass<int size, list<ValueType> types, dag registers>
326-
: RegisterClass<"MyTarget", types, size, registers> {
327-
let Size = size;
328-
}
329-
330-
def X0 : MyReg<"x0">;
331-
def X1 : MyReg<"x1">;
332-
def X2 : MyReg<"x2">;
333-
def X3 : MyReg<"x3">;
334-
def X4 : MyReg<"x4">;
335-
def X5 : MyReg<"x5">;
336-
def X6 : MyReg<"x6">;
337-
338-
def Y0 : MyReg<"y0">;
339-
def Y1 : MyReg<"y1">;
340-
def Y2 : MyReg<"y2">;
341-
def Y3 : MyReg<"y3">;
342-
def Y4 : MyReg<"y4">;
343-
def Y5 : MyReg<"y5">;
344-
def Y6 : MyReg<"y6">;
345-
346-
347-
348318
def P0_32 : MyReg<"p0">;
349319
def P1_32 : MyReg<"p1">;
350320
def P2_32 : MyReg<"p2">;
@@ -356,15 +326,12 @@ def P2_64 : MyReg<"p2_64">;
356326
def P3_64 : MyReg<"p3_64">;
357327

358328

359-
360-
def XRegs : RegisterClass<"MyTarget", [i64], 64, (add X0, X1, X2, X3, X4, X5, X6)>;
361329
def XRegs_Odd : RegisterClass<"MyTarget", [i64], 64, (add X1, X3, X5)>;
362330
def XRegs_Even : RegisterClass<"MyTarget", [i64], 64, (add X0, X2, X4, X6)>;
363331

364332
def XRegs_EvenIfRequired : RegClassByHwMode<[DefaultMode, EvenMode, OddMode],
365-
[XRegs, XRegs_Even, XRegs_Odd]>;
333+
[XRegs, XRegs_Even, XRegs_Odd]>;
366334

367-
def YRegs : RegisterClass<"MyTarget", [i64], 64, (add Y0, Y1, Y2, Y3, Y4, Y5, Y6)>;
368335
def YRegs_Even : RegisterClass<"MyTarget", [i64], 64, (add Y0, Y2, Y4, Y6)>;
369336

370337
def YRegs_EvenIfRequired : RegClassByHwMode<[DefaultMode, EvenMode],
@@ -385,23 +352,6 @@ def CustomDecodeYEvenIfRequired : RegisterOperand<YRegs_EvenIfRequired> {
385352
let DecoderMethod = "YEvenIfRequiredCustomDecoder";
386353
}
387354

388-
class TestInstruction : Instruction {
389-
let Size = 2;
390-
let Namespace = "MyTarget";
391-
let hasSideEffects = false;
392-
let hasExtraSrcRegAllocReq = false;
393-
let hasExtraDefRegAllocReq = false;
394-
395-
field bits<16> Inst;
396-
bits<3> dst;
397-
bits<3> src;
398-
bits<3> opcode;
399-
400-
let Inst{2-0} = dst;
401-
let Inst{5-3} = src;
402-
let Inst{7-5} = opcode;
403-
}
404-
405355
def SpecialOperand : RegisterOperand<XRegs_EvenIfRequired>;
406356

407357
def MY_MOV : TestInstruction {

‎llvm/test/TableGen/RegClassByHwModeErrors.td‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// RUN: rm -rf %t && split-file %s %t
2+
// RUN: not llvm-tblgen --gen-asm-matcher -I %p/../../include -I %t -I %S \
3+
// RUN: %t/inst-alias-bad-reg.td -o /dev/null 2>&1 | FileCheck %t/inst-alias-bad-reg.td --implicit-check-not="error:"
4+
// RUN: not llvm-tblgen --gen-asm-matcher -I %p/../../include -I %t -I %S \
5+
// RUN: %t/inst-alias-static-predicates.td -o /dev/null 2>&1 | FileCheck %t/inst-alias-static-predicates.td --implicit-check-not="error:"
6+
// RUN: not llvm-tblgen --gen-compress-inst-emitter -I %p/../../include -I %t -I %S \
7+
// RUN: %t/compress-regclass-by-hwmode.td -o /dev/null 2>&1 | FileCheck %t/compress-regclass-by-hwmode.td --implicit-check-not="error:"
8+
// RUN: not llvm-tblgen --gen-compress-inst-emitter -I %p/../../include -I %t -I %S \
9+
// RUN: %t/compress-regclass-by-hwmode-2.td -o /dev/null 2>&1 | FileCheck %t/compress-regclass-by-hwmode-2.td --implicit-check-not="error:"
10+
11+
//--- Common.td
12+
include "Common/RegClassByHwModeCommon.td"
13+
14+
def IsPtr64 : Predicate<"Subtarget->isPtr64()">;
15+
def IsPtr32 : Predicate<"!Subtarget->isPtr64()">;
16+
defvar Ptr32 = DefaultMode;
17+
def Ptr64 : HwMode<[IsPtr64]>;
18+
def PtrRC : RegClassByHwMode<[Ptr32, Ptr64], [XRegs, YRegs]>;
19+
20+
def PTR_MOV : TestInstruction {
21+
let OutOperandList = (outs PtrRC:$dst);
22+
let InOperandList = (ins PtrRC:$src);
23+
let AsmString = "ptr_mov $dst, $src";
24+
let opcode = 0;
25+
}
26+
27+
28+
//--- inst-alias-bad-reg.td
29+
include "Common.td"
30+
/// This should fail since X0 is not necessarily part of PtrRC.
31+
def BAD_REG : InstAlias<"ptr_zero $rd", (PTR_MOV PtrRC:$dst, X0)>;
32+
// CHECK: [[#@LINE-1]]:5: error: cannot resolve HwMode for PtrRC
33+
// CHECK: Common.td:7:5: note: PtrRC defined here
34+
def MyTargetISA : InstrInfo;
35+
def MyTarget : Target { let InstructionSet = MyTargetISA; }
36+
37+
38+
//--- inst-alias-static-predicates.td
39+
include "Common.td"
40+
/// In theory we could allow the following code since the predicates statically
41+
/// resolve to the correct register class, but since this is non-trivial, check
42+
// that we get a sensible-ish error instead.
43+
let Predicates = [IsPtr32] in
44+
def MOV_X0 : InstAlias<"mov_x0 $dst", (PTR_MOV PtrRC:$dst, X0)>;
45+
// CHECK: [[#@LINE-1]]:5: error: cannot resolve HwMode for PtrRC
46+
// CHECK: Common.td:7:5: note: PtrRC defined here
47+
let Predicates = [IsPtr64] in
48+
def MOV_Y0 : InstAlias<"mov_y0 $dst", (PTR_MOV PtrRC:$dst, Y0)>;
49+
50+
def MyTargetISA : InstrInfo;
51+
def MyTarget : Target { let InstructionSet = MyTargetISA; }
52+
53+
//--- compress-regclass-by-hwmode.td
54+
include "Common.td"
55+
def PTR_ZERO_SMALL : TestInstruction {
56+
let OutOperandList = (outs PtrRC:$dst);
57+
let InOperandList = (ins);
58+
let AsmString = "ptr_zero $dst";
59+
let opcode = 1;
60+
let Size = 1;
61+
}
62+
/// This should fail since X0 is not necessarily part of PtrRC.
63+
def : CompressPat<(PTR_MOV PtrRC:$dst, X0),
64+
(PTR_ZERO_SMALL PtrRC:$dst)>;
65+
// CHECK: [[#@LINE-2]]:1: error: cannot resolve HwMode for PtrRC
66+
// CHECK: Common.td:7:5: note: PtrRC defined here
67+
def MyTargetISA : InstrInfo;
68+
def MyTarget : Target { let InstructionSet = MyTargetISA; }
69+
70+
71+
//--- compress-regclass-by-hwmode-2.td
72+
include "Common.td"
73+
def X_MOV_BIG : TestInstruction {
74+
let OutOperandList = (outs XRegs:$dst);
75+
let InOperandList = (ins XRegs:$src);
76+
let AsmString = "x_mov $dst, $src";
77+
let opcode = 1;
78+
let Size = 4;
79+
}
80+
/// This should fail since PtrRC is not necessarily part of XRegs.
81+
/// In theory, this could be resolved depending on the Predicates but
82+
/// for not we should just always emit an error.
83+
let Predicates = [IsPtr32] in
84+
def : CompressPat<(X_MOV_BIG XRegs:$dst, XRegs:$src),
85+
(PTR_MOV PtrRC:$dst, PtrRC:$src)>;
86+
// CHECK: [[#@LINE-2]]:1: error: Type mismatch between Input and Output Dag operand 'dst'
87+
def MyTargetISA : InstrInfo;
88+
def MyTarget : Target { let InstructionSet = MyTargetISA; }

‎llvm/utils/TableGen/Common/CodeGenInstAlias.cpp‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,9 @@ unsigned CodeGenInstAlias::ResultOperand::getMINumOperands() const {
4040

4141
using ResultOperand = CodeGenInstAlias::ResultOperand;
4242

43-
static Expected<ResultOperand> matchSimpleOperand(const Init *Arg,
44-
const StringInit *ArgName,
45-
const Record *Op,
46-
const CodeGenTarget &T) {
43+
static Expected<ResultOperand>
44+
matchSimpleOperand(const Init *Arg, const StringInit *ArgName, const Record *Op,
45+
const CodeGenTarget &T, ArrayRef<SMLoc> Loc) {
4746
if (const Record *OpRC = T.getAsRegClassLike(Op)) {
4847
if (const auto *ArgDef = dyn_cast<DefInit>(Arg)) {
4948
const Record *ArgRec = ArgDef->getDef();
@@ -52,7 +51,8 @@ static Expected<ResultOperand> matchSimpleOperand(const Init *Arg,
5251
if (const Record *ArgRC = T.getInitValueAsRegClassLike(Arg)) {
5352
if (ArgRC->isSubClassOf("RegisterClass")) {
5453
if (!OpRC->isSubClassOf("RegisterClass") ||
55-
!T.getRegisterClass(OpRC).hasSubClass(&T.getRegisterClass(ArgRC)))
54+
!T.getRegisterClass(OpRC, Loc).hasSubClass(
55+
&T.getRegisterClass(ArgRC, Loc)))
5656
return createStringError(
5757
"argument register class " + ArgRC->getName() +
5858
" is not a subclass of operand register class " +
@@ -70,7 +70,8 @@ static Expected<ResultOperand> matchSimpleOperand(const Init *Arg,
7070

7171
// Match 'Reg'.
7272
if (ArgRec->isSubClassOf("Register")) {
73-
if (!T.getRegisterClass(OpRC).contains(T.getRegBank().getReg(ArgRec)))
73+
if (!T.getRegisterClass(OpRC, Loc).contains(
74+
T.getRegBank().getReg(ArgRec)))
7475
return createStringError(
7576
"register argument " + ArgRec->getName() +
7677
" is not a member of operand register class " + OpRC->getName());
@@ -199,7 +200,8 @@ CodeGenInstAlias::CodeGenInstAlias(const Record *R, const CodeGenTarget &T)
199200
const Record *SubOp =
200201
cast<DefInit>(OpInfo.MIOperandInfo->getArg(SubOpIdx))->getDef();
201202
Expected<ResultOperand> ResOpOrErr = matchSimpleOperand(
202-
ArgDag->getArg(SubOpIdx), ArgDag->getArgName(SubOpIdx), SubOp, T);
203+
ArgDag->getArg(SubOpIdx), ArgDag->getArgName(SubOpIdx), SubOp, T,
204+
R->getLoc());
203205
if (!ResOpOrErr)
204206
PrintFatalError(R, "in argument #" + Twine(ArgIdx) + "." +
205207
Twine(SubOpIdx) + ": " +
@@ -220,8 +222,9 @@ CodeGenInstAlias::CodeGenInstAlias(const Record *R, const CodeGenTarget &T)
220222
} else {
221223
// Simple operand (RegisterClass, RegisterOperand or Operand with empty
222224
// MIOperandInfo).
223-
Expected<ResultOperand> ResOpOrErr = matchSimpleOperand(
224-
Result->getArg(ArgIdx), Result->getArgName(ArgIdx), Op, T);
225+
Expected<ResultOperand> ResOpOrErr =
226+
matchSimpleOperand(Result->getArg(ArgIdx), Result->getArgName(ArgIdx),
227+
Op, T, R->getLoc());
225228
if (!ResOpOrErr)
226229
PrintFatalError(R, "in argument #" + Twine(ArgIdx) + ": " +
227230
toString(ResOpOrErr.takeError()));

‎llvm/utils/TableGen/Common/CodeGenRegisters.cpp‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,11 +1316,19 @@ CodeGenRegBank::getOrCreateSubClass(const CodeGenRegisterClass *RC,
13161316
return {&RegClasses.back(), true};
13171317
}
13181318

1319-
CodeGenRegisterClass *CodeGenRegBank::getRegClass(const Record *Def) const {
1319+
CodeGenRegisterClass *CodeGenRegBank::getRegClass(const Record *Def,
1320+
ArrayRef<SMLoc> Loc) const {
1321+
assert(Def->isSubClassOf("RegisterClassLike"));
13201322
if (CodeGenRegisterClass *RC = Def2RC.lookup(Def))
13211323
return RC;
13221324

1323-
PrintFatalError(Def->getLoc(), "Not a known RegisterClass!");
1325+
ArrayRef<SMLoc> DiagLoc = Loc.empty() ? Def->getLoc() : Loc;
1326+
// TODO: Ideally we should update the API to allow resolving HwMode.
1327+
if (Def->isSubClassOf("RegClassByHwMode"))
1328+
PrintError(DiagLoc, "cannot resolve HwMode for " + Def->getName());
1329+
else
1330+
PrintError(DiagLoc, Def->getName() + " is not a known RegisterClass!");
1331+
PrintFatalNote(Def->getLoc(), Def->getName() + " defined here");
13241332
}
13251333

13261334
CodeGenSubRegIndex *

‎llvm/utils/TableGen/Common/CodeGenRegisters.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,8 @@ class CodeGenRegBank {
815815
}
816816

817817
// Find a register class from its def.
818-
CodeGenRegisterClass *getRegClass(const Record *) const;
818+
CodeGenRegisterClass *getRegClass(const Record *,
819+
ArrayRef<SMLoc> Loc = {}) const;
819820

820821
/// getRegisterClassForRegister - Find the register class that contains the
821822
/// specified physical register. If the register is not in a register

‎llvm/utils/TableGen/Common/CodeGenTarget.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,8 @@ const CodeGenRegister *CodeGenTarget::getRegisterByName(StringRef Name) const {
173173
}
174174

175175
const CodeGenRegisterClass &
176-
CodeGenTarget::getRegisterClass(const Record *R) const {
177-
return *getRegBank().getRegClass(R);
176+
CodeGenTarget::getRegisterClass(const Record *R, ArrayRef<SMLoc> Loc) const {
177+
return *getRegBank().getRegClass(R, Loc);
178178
}
179179

180180
std::vector<ValueTypeByHwMode>

‎llvm/utils/TableGen/Common/CodeGenTarget.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,8 @@ class CodeGenTarget {
131131
return RegAltNameIndices;
132132
}
133133

134-
const CodeGenRegisterClass &getRegisterClass(const Record *R) const;
134+
const CodeGenRegisterClass &getRegisterClass(const Record *R,
135+
ArrayRef<SMLoc> Loc = {}) const;
135136

136137
/// Convenience wrapper to avoid hardcoding the name of RegClassByHwMode
137138
/// everywhere. This is here instead of CodeGenRegBank to avoid the fatal

‎llvm/utils/TableGen/CompressInstEmitter.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,8 @@ class CompressInstEmitter {
142142
void emitCompressInstEmitter(raw_ostream &OS, EmitterType EType);
143143
bool validateTypes(const Record *DagOpType, const Record *InstOpType,
144144
bool IsSourceInst);
145-
bool validateRegister(const Record *Reg, const Record *RegClass);
145+
bool validateRegister(const Record *Reg, const Record *RegClass,
146+
ArrayRef<SMLoc> Loc);
146147
void checkDagOperandMapping(const Record *Rec,
147148
const StringMap<ArgData> &DestOperands,
148149
const DagInit *SourceDag, const DagInit *DestDag);
@@ -162,11 +163,12 @@ class CompressInstEmitter {
162163
} // End anonymous namespace.
163164

164165
bool CompressInstEmitter::validateRegister(const Record *Reg,
165-
const Record *RegClass) {
166+
const Record *RegClass,
167+
ArrayRef<SMLoc> Loc) {
166168
assert(Reg->isSubClassOf("Register") && "Reg record should be a Register");
167-
assert(RegClass->isSubClassOf("RegisterClass") &&
168-
"RegClass record should be a RegisterClass");
169-
const CodeGenRegisterClass &RC = Target.getRegisterClass(RegClass);
169+
assert(RegClass->isSubClassOf("RegisterClassLike") &&
170+
"RegClass record should be RegisterClassLike");
171+
const CodeGenRegisterClass &RC = Target.getRegisterClass(RegClass, Loc);
170172
const CodeGenRegister *R = Target.getRegBank().getReg(Reg);
171173
assert(R != nullptr && "Register not defined!!");
172174
return RC.contains(R);
@@ -255,7 +257,7 @@ void CompressInstEmitter::addDagOperandMapping(const Record *Rec,
255257
if (const auto *DI = dyn_cast<DefInit>(Dag->getArg(DAGOpNo))) {
256258
if (DI->getDef()->isSubClassOf("Register")) {
257259
// Check if the fixed register belongs to the Register class.
258-
if (!validateRegister(DI->getDef(), OpndRec))
260+
if (!validateRegister(DI->getDef(), OpndRec, Rec->getLoc()))
259261
PrintFatalError(Rec->getLoc(),
260262
"Error in Dag '" + Dag->getAsString() +
261263
"'Register: '" + DI->getDef()->getName() +

0 commit comments

Comments
 (0)