llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-tablegen Author: Alexander Richardson (arichardson) <details> <summary>Changes</summary> 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. Not test yet in this commit since I plan to improve this error message in a follow-up. --- Full diff: https://github.com/llvm/llvm-project/pull/170790.diff 9 Files Affected: - (added) llvm/test/TableGen/Common/RegClassByHwModeCommon.td (+38) - (modified) llvm/test/TableGen/RegClassByHwMode.td (+6-53) - (added) llvm/test/TableGen/RegClassByHwModeErrors.td (+47) - (modified) llvm/utils/TableGen/Common/CodeGenInstAlias.cpp (+11-6) - (modified) llvm/utils/TableGen/Common/CodeGenRegisters.cpp (+9-2) - (modified) llvm/utils/TableGen/Common/CodeGenRegisters.h (+2-1) - (modified) llvm/utils/TableGen/Common/CodeGenTarget.cpp (+2-2) - (modified) llvm/utils/TableGen/Common/CodeGenTarget.h (+2-1) - (modified) llvm/utils/TableGen/CompressInstEmitter.cpp (+8-6) ``````````diff 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() + `````````` </details> https://github.com/llvm/llvm-project/pull/170790 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
