-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[TableGen] Slightly improve error location for a fatal error #170790
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
Changes from all commits
329a308
5deea85
b6da362
0870815
8dfb67d
51e5de8
780b0fe
abe7ec4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| 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 X4 : MyReg<"x4">; | ||
| def X5 : MyReg<"x5">; | ||
| def X6 : MyReg<"x6">; | ||
| def XRegs : RegisterClass<"MyTarget", [i64], 64, (add X0, X1, X2, X3, X4, X5, 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 YRegs : RegisterClass<"MyTarget", [i64], 64, (add Y0, Y1, Y2, Y3, Y4, Y5, Y6)>; | ||
|
|
||
| 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; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| // 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 --implicit-check-not="error:" | ||
| // RUN: not llvm-tblgen --gen-asm-matcher -I %p/../../include -I %t -I %S \ | ||
| // RUN: %t/inst-alias-static-predicates.td -o /dev/null 2>&1 | FileCheck %t/inst-alias-static-predicates.td --implicit-check-not="error:" | ||
| // 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 --implicit-check-not="error:" | ||
| // RUN: not llvm-tblgen --gen-compress-inst-emitter -I %p/../../include -I %t -I %S \ | ||
| // 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:" | ||
|
|
||
| //--- Common.td | ||
| include "Common/RegClassByHwModeCommon.td" | ||
|
|
||
| def IsPtr64 : Predicate<"Subtarget->isPtr64()">; | ||
| def IsPtr32 : 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:7:5: note: PtrRC defined here | ||
| def MyTargetISA : InstrInfo; | ||
| def MyTarget : Target { let InstructionSet = MyTargetISA; } | ||
|
|
||
|
|
||
| //--- inst-alias-static-predicates.td | ||
| include "Common.td" | ||
| /// In theory we could allow the following code since the predicates statically | ||
| /// resolve to the correct register class, but since this is non-trivial, check | ||
| // that we get a sensible-ish error instead. | ||
| let Predicates = [IsPtr32] in | ||
| def MOV_X0 : InstAlias<"mov_x0 $dst", (PTR_MOV PtrRC:$dst, X0)>; | ||
| // CHECK: [[#@LINE-1]]:5: error: cannot resolve HwMode for PtrRC | ||
| // CHECK: Common.td:7:5: note: PtrRC defined here | ||
| let Predicates = [IsPtr64] in | ||
| def MOV_Y0 : InstAlias<"mov_y0 $dst", (PTR_MOV PtrRC:$dst, Y0)>; | ||
|
|
||
| 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:7:5: note: PtrRC defined here | ||
| def MyTargetISA : InstrInfo; | ||
| def MyTarget : Target { let InstructionSet = MyTargetISA; } | ||
|
|
||
|
|
||
| //--- compress-regclass-by-hwmode-2.td | ||
| include "Common.td" | ||
| def X_MOV_BIG : TestInstruction { | ||
| let OutOperandList = (outs XRegs:$dst); | ||
| let InOperandList = (ins XRegs:$src); | ||
| let AsmString = "x_mov $dst, $src"; | ||
| let opcode = 1; | ||
| let Size = 4; | ||
| } | ||
| /// This should fail since PtrRC is not necessarily part of XRegs. | ||
| /// In theory, this could be resolved depending on the Predicates but | ||
| /// for not we should just always emit an error. | ||
| let Predicates = [IsPtr32] in | ||
| def : CompressPat<(X_MOV_BIG XRegs:$dst, XRegs:$src), | ||
| (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; } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1316,11 +1316,19 @@ 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 { | ||
| assert(Def->isSubClassOf("RegisterClassLike")); | ||
| 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. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is that even possible? HwModes are resolved at runtime
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In some cases (e.g. inside a
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe in those cases we should resolve it manually in td?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added a test case for this. If there are no further comments I'll submit this tomorrow. |
||
| 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 * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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") && | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this change related?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without this change we get an assertion failure instead of an error in the second test that I added |
||
| "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() + | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.