From 34860550f2bca6c7b927eccc8dd242c4149b24ac Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Wed, 28 Nov 2018 11:43:49 +0000 Subject: [PATCH] [TableGen] Better error checking for TIED_TO constraints. There are quite strong constraints on how you can use the TIED_TO constraint between MC operands, many of which are currently not checked until compiler run time. MachineVerifier enforces that operands can only be tied together in pairs (no three-way ties), and MachineInstr::tieOperands enforces that one of the tied operands must be an output operand (def) and the other must be an input operand (use). Now we check these at TableGen time, so that if you violate any of them in a new instruction definition, you find out immediately, instead of having to wait until you compile something that makes code generation hit one of those assertions. Also in this commit, all the error reports in ParseConstraint now include the name and source location of the def where the problem happened, so that if you do trigger any of these errors, it's easier to find the part of your TableGen input where you made the mistake. The trunk sources already build successfully with this additional error check, so I think no in-tree target has any of these problems. Reviewers: fhahn, lhames, nhaehnle, MatzeB Reviewed By: MatzeB Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D53815 llvm-svn: 347743 --- llvm/test/TableGen/ConstraintChecking.inc | 33 ++++++++ llvm/test/TableGen/ConstraintChecking1.td | 6 ++ llvm/test/TableGen/ConstraintChecking2.td | 6 ++ llvm/test/TableGen/ConstraintChecking3.td | 8 ++ llvm/test/TableGen/ConstraintChecking4.td | 6 ++ llvm/test/TableGen/ConstraintChecking5.td | 6 ++ llvm/test/TableGen/ConstraintChecking6.td | 6 ++ llvm/test/TableGen/ConstraintChecking7.td | 6 ++ llvm/utils/TableGen/CodeGenInstruction.cpp | 97 +++++++++++++++------- llvm/utils/TableGen/CodeGenInstruction.h | 9 ++ 10 files changed, 154 insertions(+), 29 deletions(-) create mode 100644 llvm/test/TableGen/ConstraintChecking.inc create mode 100644 llvm/test/TableGen/ConstraintChecking1.td create mode 100644 llvm/test/TableGen/ConstraintChecking2.td create mode 100644 llvm/test/TableGen/ConstraintChecking3.td create mode 100644 llvm/test/TableGen/ConstraintChecking4.td create mode 100644 llvm/test/TableGen/ConstraintChecking5.td create mode 100644 llvm/test/TableGen/ConstraintChecking6.td create mode 100644 llvm/test/TableGen/ConstraintChecking7.td diff --git a/llvm/test/TableGen/ConstraintChecking.inc b/llvm/test/TableGen/ConstraintChecking.inc new file mode 100644 index 0000000000000..1c5872953be55 --- /dev/null +++ b/llvm/test/TableGen/ConstraintChecking.inc @@ -0,0 +1,33 @@ +include "llvm/Target/Target.td" + +def TestTarget : Target; + +class Encoding : Instruction { + field bits<8> Inst; +} + +class TestReg enc> : Register { + let HWEncoding{15-1} = 0; + let HWEncoding{0} = enc; +} + +def R0 : TestReg<"R0", 0>; +def R1 : TestReg<"R1", 1>; +def Reg : RegisterClass<"TestTarget", [i32], 32, (sequence "R%d", 0, 1)>; + +class TestInstructionWithConstraints : Encoding { + dag OutOperandList = (outs Reg:$dest1, Reg:$dest2); + dag InOperandList = (ins Reg:$src1, Reg:$src2); + string AsmString = "mnemonic $dest1, $dest2, $src1, $src2"; + string AsmVariantName = ""; + let Constraints = cstr; + field bits<1> dest1; + field bits<1> dest2; + field bits<1> src1; + field bits<1> src2; + let Inst{7-4} = 0b1010; + let Inst{3} = dest1{0}; + let Inst{2} = dest2{0}; + let Inst{1} = src1{0}; + let Inst{0} = src2{0}; +} diff --git a/llvm/test/TableGen/ConstraintChecking1.td b/llvm/test/TableGen/ConstraintChecking1.td new file mode 100644 index 0000000000000..2c0c3d9bfabb6 --- /dev/null +++ b/llvm/test/TableGen/ConstraintChecking1.td @@ -0,0 +1,6 @@ +// RUN: not llvm-tblgen -gen-asm-writer -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s + +include "ConstraintChecking.inc" + +// CHECK: [[FILE]]:[[@LINE+1]]:1: error: Unrecognized constraint '$dest1 ~ $src2' in 'Foo' +def Foo : TestInstructionWithConstraints<"$dest1 ~ $src2">; diff --git a/llvm/test/TableGen/ConstraintChecking2.td b/llvm/test/TableGen/ConstraintChecking2.td new file mode 100644 index 0000000000000..65ec882b18f82 --- /dev/null +++ b/llvm/test/TableGen/ConstraintChecking2.td @@ -0,0 +1,6 @@ +// RUN: not llvm-tblgen -gen-asm-writer -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s + +include "ConstraintChecking.inc" + +// CHECK: [[FILE]]:[[@LINE+1]]:1: error: Illegal format for @earlyclobber constraint in 'Foo' +def Foo : TestInstructionWithConstraints<"@earlyclobber ">; diff --git a/llvm/test/TableGen/ConstraintChecking3.td b/llvm/test/TableGen/ConstraintChecking3.td new file mode 100644 index 0000000000000..835ec2e035f7d --- /dev/null +++ b/llvm/test/TableGen/ConstraintChecking3.td @@ -0,0 +1,8 @@ +// RUN: not llvm-tblgen -gen-asm-writer -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s + +include "ConstraintChecking.inc" + +// (This is illegal because the '=' has to be surrounded by whitespace) + +// CHECK: [[FILE]]:[[@LINE+1]]:1: error: Illegal format for tied-to constraint in 'Foo' +def Foo : TestInstructionWithConstraints<"$dest1=$dest2">; diff --git a/llvm/test/TableGen/ConstraintChecking4.td b/llvm/test/TableGen/ConstraintChecking4.td new file mode 100644 index 0000000000000..bd511eb1132df --- /dev/null +++ b/llvm/test/TableGen/ConstraintChecking4.td @@ -0,0 +1,6 @@ +// RUN: not llvm-tblgen -gen-asm-writer -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s + +include "ConstraintChecking.inc" + +// CHECK: [[FILE]]:[[@LINE+1]]:1: error: Output operands '$dest1' and '$dest2' of 'Foo' cannot be tied! +def Foo : TestInstructionWithConstraints<"$dest1 = $dest2">; diff --git a/llvm/test/TableGen/ConstraintChecking5.td b/llvm/test/TableGen/ConstraintChecking5.td new file mode 100644 index 0000000000000..7db354637fa1a --- /dev/null +++ b/llvm/test/TableGen/ConstraintChecking5.td @@ -0,0 +1,6 @@ +// RUN: not llvm-tblgen -gen-asm-writer -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s + +include "ConstraintChecking.inc" + +// CHECK: [[FILE]]:[[@LINE+1]]:1: error: Input operands '$src1' and '$src2' of 'Foo' cannot be tied! +def Foo : TestInstructionWithConstraints<"$src1 = $src2">; diff --git a/llvm/test/TableGen/ConstraintChecking6.td b/llvm/test/TableGen/ConstraintChecking6.td new file mode 100644 index 0000000000000..2d3bdd2adabe7 --- /dev/null +++ b/llvm/test/TableGen/ConstraintChecking6.td @@ -0,0 +1,6 @@ +// RUN: not llvm-tblgen -gen-asm-writer -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s + +include "ConstraintChecking.inc" + +// CHECK: [[FILE]]:[[@LINE+1]]:1: error: Operand '$dest1' of 'Foo' cannot have multiple operands tied to it! +def Foo : TestInstructionWithConstraints<"$dest1 = $src1, $dest1 = $src2">; diff --git a/llvm/test/TableGen/ConstraintChecking7.td b/llvm/test/TableGen/ConstraintChecking7.td new file mode 100644 index 0000000000000..5a44ee4f1d8ae --- /dev/null +++ b/llvm/test/TableGen/ConstraintChecking7.td @@ -0,0 +1,6 @@ +// RUN: not llvm-tblgen -gen-asm-writer -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s + +include "ConstraintChecking.inc" + +// CHECK: [[FILE]]:[[@LINE+1]]:1: error: Operand '$src1' of 'Foo' cannot have multiple constraints! +def Foo : TestInstructionWithConstraints<"$dest1 = $src1, $dest2 = $src1">; diff --git a/llvm/utils/TableGen/CodeGenInstruction.cpp b/llvm/utils/TableGen/CodeGenInstruction.cpp index a6ca400914b49..2a6749a613638 100644 --- a/llvm/utils/TableGen/CodeGenInstruction.cpp +++ b/llvm/utils/TableGen/CodeGenInstruction.cpp @@ -202,7 +202,8 @@ CGIOperandList::ParseOperandName(const std::string &Op, bool AllowWholeOp) { return std::make_pair(0U, 0U); } -static void ParseConstraint(const std::string &CStr, CGIOperandList &Ops) { +static void ParseConstraint(const std::string &CStr, CGIOperandList &Ops, + Record *Rec) { // EARLY_CLOBBER: @early $reg std::string::size_type wpos = CStr.find_first_of(" \t"); std::string::size_type start = CStr.find_first_not_of(" \t"); @@ -211,13 +212,17 @@ static void ParseConstraint(const std::string &CStr, CGIOperandList &Ops) { std::string Name = CStr.substr(wpos+1); wpos = Name.find_first_not_of(" \t"); if (wpos == std::string::npos) - PrintFatalError("Illegal format for @earlyclobber constraint: '" + CStr + "'"); + PrintFatalError( + Rec->getLoc(), "Illegal format for @earlyclobber constraint in '" + + Rec->getName() + "': '" + CStr + "'"); Name = Name.substr(wpos); std::pair Op = Ops.ParseOperandName(Name, false); // Build the string for the operand if (!Ops[Op.first].Constraints[Op.second].isNone()) - PrintFatalError("Operand '" + Name + "' cannot have multiple constraints!"); + PrintFatalError( + Rec->getLoc(), "Operand '" + Name + "' of '" + Rec->getName() + + "' cannot have multiple constraints!"); Ops[Op.first].Constraints[Op.second] = CGIOperandList::ConstraintInfo::getEarlyClobber(); return; @@ -225,39 +230,73 @@ static void ParseConstraint(const std::string &CStr, CGIOperandList &Ops) { // Only other constraint is "TIED_TO" for now. std::string::size_type pos = CStr.find_first_of('='); - assert(pos != std::string::npos && "Unrecognized constraint"); + if (pos == std::string::npos) + PrintFatalError( + Rec->getLoc(), "Unrecognized constraint '" + CStr + + "' in '" + Rec->getName() + "'"); start = CStr.find_first_not_of(" \t"); - std::string Name = CStr.substr(start, pos - start); // TIED_TO: $src1 = $dst - wpos = Name.find_first_of(" \t"); + wpos = CStr.find_first_of(" \t", start); + if (wpos == std::string::npos || wpos > pos) + PrintFatalError( + Rec->getLoc(), "Illegal format for tied-to constraint in '" + + Rec->getName() + "': '" + CStr + "'"); + std::string LHSOpName = StringRef(CStr).substr(start, wpos - start); + std::pair LHSOp = Ops.ParseOperandName(LHSOpName, false); + + wpos = CStr.find_first_not_of(" \t", pos + 1); if (wpos == std::string::npos) - PrintFatalError("Illegal format for tied-to constraint: '" + CStr + "'"); - std::string DestOpName = Name.substr(0, wpos); - std::pair DestOp = Ops.ParseOperandName(DestOpName, false); - - Name = CStr.substr(pos+1); - wpos = Name.find_first_not_of(" \t"); - if (wpos == std::string::npos) - PrintFatalError("Illegal format for tied-to constraint: '" + CStr + "'"); + PrintFatalError( + Rec->getLoc(), "Illegal format for tied-to constraint: '" + CStr + "'"); + + std::string RHSOpName = StringRef(CStr).substr(wpos); + std::pair RHSOp = Ops.ParseOperandName(RHSOpName, false); + + // Sort the operands into order, which should put the output one + // first. But keep the original order, for use in diagnostics. + bool FirstIsDest = (LHSOp < RHSOp); + std::pair DestOp = (FirstIsDest ? LHSOp : RHSOp); + StringRef DestOpName = (FirstIsDest ? LHSOpName : RHSOpName); + std::pair SrcOp = (FirstIsDest ? RHSOp : LHSOp); + StringRef SrcOpName = (FirstIsDest ? RHSOpName : LHSOpName); + + // Ensure one operand is a def and the other is a use. + if (DestOp.first >= Ops.NumDefs) + PrintFatalError( + Rec->getLoc(), "Input operands '" + LHSOpName + "' and '" + RHSOpName + + "' of '" + Rec->getName() + "' cannot be tied!"); + if (SrcOp.first < Ops.NumDefs) + PrintFatalError( + Rec->getLoc(), "Output operands '" + LHSOpName + "' and '" + RHSOpName + + "' of '" + Rec->getName() + "' cannot be tied!"); + + // The constraint has to go on the operand with higher index, i.e. + // the source one. Check there isn't another constraint there + // already. + if (!Ops[SrcOp.first].Constraints[SrcOp.second].isNone()) + PrintFatalError( + Rec->getLoc(), "Operand '" + SrcOpName + "' of '" + Rec->getName() + + "' cannot have multiple constraints!"); - std::string SrcOpName = Name.substr(wpos); - std::pair SrcOp = Ops.ParseOperandName(SrcOpName, false); - if (SrcOp > DestOp) { - std::swap(SrcOp, DestOp); - std::swap(SrcOpName, DestOpName); + unsigned DestFlatOpNo = Ops.getFlattenedOperandNumber(DestOp); + auto NewConstraint = CGIOperandList::ConstraintInfo::getTied(DestFlatOpNo); + + // Check that the earlier operand is not the target of another tie + // before making it the target of this one. + for (const CGIOperandList::OperandInfo &Op : Ops) { + for (unsigned i = 0; i < Op.MINumOperands; i++) + if (Op.Constraints[i] == NewConstraint) + PrintFatalError( + Rec->getLoc(), "Operand '" + DestOpName + "' of '" + Rec->getName() + + "' cannot have multiple operands tied to it!"); } - unsigned FlatOpNo = Ops.getFlattenedOperandNumber(SrcOp); - - if (!Ops[DestOp.first].Constraints[DestOp.second].isNone()) - PrintFatalError("Operand '" + DestOpName + - "' cannot have multiple constraints!"); - Ops[DestOp.first].Constraints[DestOp.second] = - CGIOperandList::ConstraintInfo::getTied(FlatOpNo); + Ops[SrcOp.first].Constraints[SrcOp.second] = NewConstraint; } -static void ParseConstraints(const std::string &CStr, CGIOperandList &Ops) { +static void ParseConstraints(const std::string &CStr, CGIOperandList &Ops, + Record *Rec) { if (CStr.empty()) return; const std::string delims(","); @@ -269,7 +308,7 @@ static void ParseConstraints(const std::string &CStr, CGIOperandList &Ops) { if (eidx == std::string::npos) eidx = CStr.length(); - ParseConstraint(CStr.substr(bidx, eidx - bidx), Ops); + ParseConstraint(CStr.substr(bidx, eidx - bidx), Ops, Rec); bidx = CStr.find_first_not_of(delims, eidx); } } @@ -353,7 +392,7 @@ CodeGenInstruction::CodeGenInstruction(Record *R) hasChain_Inferred = false; // Parse Constraints. - ParseConstraints(R->getValueAsString("Constraints"), Operands); + ParseConstraints(R->getValueAsString("Constraints"), Operands, R); // Parse the DisableEncoding field. Operands.ProcessDisableEncoding(R->getValueAsString("DisableEncoding")); diff --git a/llvm/utils/TableGen/CodeGenInstruction.h b/llvm/utils/TableGen/CodeGenInstruction.h index 6f79b4bcc5cd9..04ad6d2c79140 100644 --- a/llvm/utils/TableGen/CodeGenInstruction.h +++ b/llvm/utils/TableGen/CodeGenInstruction.h @@ -57,6 +57,15 @@ template class ArrayRef; assert(isTied()); return OtherTiedOperand; } + + bool operator==(const ConstraintInfo &RHS) const { + if (Kind != RHS.Kind) + return false; + if (Kind == Tied && OtherTiedOperand != RHS.OtherTiedOperand) + return false; + return true; + } + bool operator!=(const ConstraintInfo &RHS) const { return *this != RHS; } }; /// OperandInfo - The information we keep track of for each operand in the