[llvm] b87dc35 - [TableGen] Delete support for deprecated positional matching.
James Y Knight via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 7 12:04:25 PST 2023
Author: James Y Knight
Date: 2023-03-07T15:04:09-05:00
New Revision: b87dc35669929ed29838cc7006c25ef9fa84e6f6
URL: https://github.com/llvm/llvm-project/commit/b87dc35669929ed29838cc7006c25ef9fa84e6f6
DIFF: https://github.com/llvm/llvm-project/commit/b87dc35669929ed29838cc7006c25ef9fa84e6f6.diff
LOG: [TableGen] Delete support for deprecated positional matching.
After the work in a538d1f13a13 5351878ba196 372240dfe3d5, and
subsequently cleanup of all the in-tree targets, we can now delete the
support for positional operand matching!
This removes three options which could previously be set in a
Target's "InstrInfo" tablegen definition:
- useDeprecatedPositionallyEncodedOperands
- decodePositionallyEncodedOperands
- noNamedPositionallyEncodedOperands
(Also announced at https://discourse.llvm.org/t/tablegen-deleting-deprecated-positional-instruction-operand-matching-support/68524)
Differential Revision: https://reviews.llvm.org/D144210
Added:
Modified:
llvm/include/llvm/Target/Target.td
llvm/test/TableGen/MissingOperandField.td
llvm/utils/TableGen/CodeEmitterGen.cpp
llvm/utils/TableGen/DecoderEmitter.cpp
Removed:
llvm/test/TableGen/InsufficientPositionalOperands.td
################################################################################
diff --git a/llvm/include/llvm/Target/Target.td b/llvm/include/llvm/Target/Target.td
index 181c8eb17511b..6e68663f33c18 100644
--- a/llvm/include/llvm/Target/Target.td
+++ b/llvm/include/llvm/Target/Target.td
@@ -1058,45 +1058,6 @@ class InstrInfo {
//
// This option is a temporary migration help. It will go away.
bit guessInstructionProperties = true;
-
- // TableGen's instruction encoder generator has support for matching operands
- // to bit-field variables both by name and by position. Support for matching
- // by position is DEPRECATED, and WILL BE REMOVED. Positional matching is
- // confusing to use, and makes it very easy to accidentally write buggy
- // instruction definitions.
- //
- // In previous versions of LLVM, the ability to match operands by position was
- // enabled unconditionally. It is now controllable by this option -- and
- // disabled by default. The previous behavior can be restored by setting this
- // option to true.
- //
- // This option is temporary, and will go away once all in-tree targets have
- // migrated.
- //
- // TODO: clean up and remove these options.
- bit useDeprecatedPositionallyEncodedOperands = false;
-
- // If positional encoding rules are used for the encoder generator, they may
- // also need to be used by the decoder generator -- if so, enable this
- // variable.
- //
- // This option is a no-op unless useDeprecatedPositionallyEncodedOperands is
- // true.
- //
- // This option is temporary, and will go away once all in-tree targets have
- // migrated.
- bit decodePositionallyEncodedOperands = false;
-
- // When set, this indicates that there will be no overlap between those
- // operands that are matched by ordering (positional operands) and those
- // matched by name.
- //
- // This is a no-op unless useDeprecatedPositionallyEncodedOperands is true
- // (though it does modify the "would've used positional operand XXX" error.)
- //
- // This option is temporary, and will go away once all in-tree targets have
- // migrated.
- bit noNamedPositionallyEncodedOperands = false;
}
// Standard Pseudo Instructions.
diff --git a/llvm/test/TableGen/InsufficientPositionalOperands.td b/llvm/test/TableGen/InsufficientPositionalOperands.td
deleted file mode 100644
index 131c0861ad3c5..0000000000000
--- a/llvm/test/TableGen/InsufficientPositionalOperands.td
+++ /dev/null
@@ -1,35 +0,0 @@
-// RUN: not llvm-tblgen -gen-emitter -I %p/../../include %s 2>&1 | FileCheck %s --implicit-check-not=error:
-
-// Check that TableGen doesn't crash on insufficient positional
-// instruction operands.
-
-// TODO: remove this test when removing useDeprecatedPositionallyEncodedOperands
-
-include "llvm/Target/Target.td"
-
-def ArchInstrInfo : InstrInfo {
- let useDeprecatedPositionallyEncodedOperands = 1;
-}
-
-def Arch : Target {
- let InstructionSet = ArchInstrInfo;
-}
-
-def Reg : Register<"reg">;
-
-def Regs : RegisterClass<"foo", [i32], 0, (add Reg)>;
-
-// CHECK: error: Too few operands in record foo (no match for variable rs)
-// CHECK: note: Dumping record for previous error:
-def foo : Instruction {
- bits<3> rd;
- bits<3> rs;
-
- bits<8> Inst;
- let Inst{1-0} = 0;
- let Inst{4-2} = rd;
- let Inst{7-5} = rs;
-
- let OutOperandList = (outs Regs:$xd);
- let InOperandList = (ins);
-}
diff --git a/llvm/test/TableGen/MissingOperandField.td b/llvm/test/TableGen/MissingOperandField.td
index 3b550813540a6..37d6695ad4d59 100644
--- a/llvm/test/TableGen/MissingOperandField.td
+++ b/llvm/test/TableGen/MissingOperandField.td
@@ -15,8 +15,8 @@ def Reg : Register<"reg">;
def Regs : RegisterClass<"foo", [i32], 0, (add Reg)>;
-// CHECK: error: No operand named rd in record foo (would've used positional operand #0 ('xd') sub-op #0 with useDeprecatedPositionallyEncodedOperands=true)
-// CHECK: error: No operand named rs in record foo (would've given 'too few operands' error with useDeprecatedPositionallyEncodedOperands=true)
+// CHECK: error: No operand named rd in record foo
+// CHECK: error: No operand named rs in record foo
// CHECK: note: Dumping record for previous error:
def foo : Instruction {
bits<3> rd;
diff --git a/llvm/utils/TableGen/CodeEmitterGen.cpp b/llvm/utils/TableGen/CodeEmitterGen.cpp
index 1dd076a437766..11a57bb1bc310 100644
--- a/llvm/utils/TableGen/CodeEmitterGen.cpp
+++ b/llvm/utils/TableGen/CodeEmitterGen.cpp
@@ -51,8 +51,7 @@ class CodeEmitterGen {
std::string getInstructionCaseForEncoding(Record *R, Record *EncodingDef,
CodeGenTarget &Target);
bool addCodeToMergeInOperand(Record *R, BitsInit *BI,
- const std::string &VarName, unsigned &NumberedOp,
- std::set<unsigned> &NamedOpIndices,
+ const std::string &VarName,
std::string &Case, CodeGenTarget &Target);
void emitInstructionBaseValues(
@@ -81,8 +80,6 @@ int CodeEmitterGen::getVariableBit(const std::string &VarName,
// Returns true if it succeeds, false if an error.
bool CodeEmitterGen::addCodeToMergeInOperand(Record *R, BitsInit *BI,
const std::string &VarName,
- unsigned &NumberedOp,
- std::set<unsigned> &NamedOpIndices,
std::string &Case,
CodeGenTarget &Target) {
CodeGenInstruction &CGI = Target.getInstruction(R);
@@ -114,52 +111,8 @@ bool CodeEmitterGen::addCodeToMergeInOperand(Record *R, BitsInit *BI,
// Get the machine operand number for the indicated operand.
OpIdx = CGI.Operands[OpIdx].MIOperandNo;
} else {
- // Fall back to positional lookup. By default, we now disable positional
- // lookup (and print an error, below), but even so, we'll do the lookup to
- // help print a helpful diagnostic message.
- //
- // TODO: When we remove useDeprecatedPositionallyEncodedOperands, delete all
- // this code, just leaving a "no operand named X in record Y" error.
-
- unsigned NumberOps = CGI.Operands.size();
- /// If this operand is not supposed to be emitted by the
- /// generated emitter, skip it.
- while (NumberedOp < NumberOps &&
- (CGI.Operands.isFlatOperandNotEmitted(NumberedOp) ||
- (!NamedOpIndices.empty() && NamedOpIndices.count(
- CGI.Operands.getSubOperandNumber(NumberedOp).first)))) {
- ++NumberedOp;
- }
-
- if (NumberedOp >=
- CGI.Operands.back().MIOperandNo + CGI.Operands.back().MINumOperands) {
- if (!Target.getInstructionSet()->getValueAsBit(
- "useDeprecatedPositionallyEncodedOperands")) {
- PrintError(R, Twine("No operand named ") + VarName + " in record " +
- R->getName() +
- " (would've given 'too few operands' error with "
- "useDeprecatedPositionallyEncodedOperands=true)");
- } else {
- PrintError(R, "Too few operands in record " + R->getName() +
- " (no match for variable " + VarName + ")");
- }
- return false;
- }
-
- OpIdx = NumberedOp++;
-
- if (!Target.getInstructionSet()->getValueAsBit(
- "useDeprecatedPositionallyEncodedOperands")) {
- std::pair<unsigned, unsigned> SO =
- CGI.Operands.getSubOperandNumber(OpIdx);
- std::string OpName = CGI.Operands[SO.first].Name;
- PrintError(R, Twine("No operand named ") + VarName + " in record " +
- R->getName() + " (would've used positional operand #" +
- Twine(SO.first) + " ('" + OpName + "') sub-op #" +
- Twine(SO.second) +
- " with useDeprecatedPositionallyEncodedOperands=true)");
- return false;
- }
+ PrintError(R, Twine("No operand named ") + VarName + " in record " + R->getName());
+ return false;
}
if (CGI.Operands.isFlatOperandNotEmitted(OpIdx)) {
@@ -320,22 +273,6 @@ std::string CodeEmitterGen::getInstructionCaseForEncoding(Record *R, Record *Enc
CodeGenTarget &Target) {
std::string Case;
BitsInit *BI = EncodingDef->getValueAsBitsInit("Inst");
- unsigned NumberedOp = 0;
- std::set<unsigned> NamedOpIndices;
-
- // Collect the set of operand indices that might correspond to named
- // operand, and skip these when assigning operands based on position.
- if (Target.getInstructionSet()->
- getValueAsBit("noNamedPositionallyEncodedOperands")) {
- CodeGenInstruction &CGI = Target.getInstruction(R);
- for (const RecordVal &RV : R->getValues()) {
- unsigned OpIdx;
- if (!CGI.Operands.hasOperandNamed(RV.getName(), OpIdx))
- continue;
-
- NamedOpIndices.insert(OpIdx);
- }
- }
// Loop over all of the fields in the instruction, determining which are the
// operands to the instruction.
@@ -347,8 +284,8 @@ std::string CodeEmitterGen::getInstructionCaseForEncoding(Record *R, Record *Enc
continue;
Success &=
- addCodeToMergeInOperand(R, BI, std::string(RV.getName()), NumberedOp,
- NamedOpIndices, Case, Target);
+ addCodeToMergeInOperand(R, BI, std::string(RV.getName()),
+ Case, Target);
}
if (!Success) {
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index eabc158ab91ed..b3f696654fc5f 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -2029,193 +2029,11 @@ populateInstruction(CodeGenTarget &Target, const Record &EncodingDef,
if (IsVarLenInst) {
parseVarLenInstOperand(EncodingDef, InsnOperands, CGI);
} else {
- std::map<std::string, std::vector<OperandInfo>> NumberedInsnOperands;
- std::set<std::string> NumberedInsnOperandsNoTie;
- bool SupportPositionalDecoding =
- Target.getInstructionSet()->getValueAsBit(
- "useDeprecatedPositionallyEncodedOperands") &&
- Target.getInstructionSet()->getValueAsBit(
- "decodePositionallyEncodedOperands");
- if (SupportPositionalDecoding) {
- const std::vector<RecordVal> &Vals = Def.getValues();
- unsigned NumberedOp = 0;
-
- std::set<unsigned> NamedOpIndices;
- if (Target.getInstructionSet()->getValueAsBit(
- "noNamedPositionallyEncodedOperands"))
- // Collect the set of operand indices that might correspond to named
- // operand, and skip these when assigning operands based on position.
- for (unsigned i = 0, e = Vals.size(); i != e; ++i) {
- unsigned OpIdx;
- if (!CGI.Operands.hasOperandNamed(Vals[i].getName(), OpIdx))
- continue;
-
- NamedOpIndices.insert(OpIdx);
- }
-
- for (unsigned i = 0, e = Vals.size(); i != e; ++i) {
- // Ignore fixed fields in the record, we're looking for values like:
- // bits<5> RST = { ?, ?, ?, ?, ? };
- if (Vals[i].isNonconcreteOK() || Vals[i].getValue()->isComplete())
- continue;
-
- // Determine if Vals[i] actually contributes to the Inst encoding.
- unsigned bi = 0;
- for (; bi < Bits.getNumBits(); ++bi) {
- VarInit *Var = nullptr;
- VarBitInit *BI = dyn_cast<VarBitInit>(Bits.getBit(bi));
- if (BI)
- Var = dyn_cast<VarInit>(BI->getBitVar());
- else
- Var = dyn_cast<VarInit>(Bits.getBit(bi));
-
- if (Var && Var->getName() == Vals[i].getName())
- break;
- }
-
- if (bi == Bits.getNumBits())
- continue;
-
- // Skip variables that correspond to explicitly-named operands.
- unsigned OpIdx;
- std::pair<unsigned, unsigned> SubOp;
- if (CGI.Operands.hasSubOperandAlias(Vals[i].getName(), SubOp) ||
- CGI.Operands.hasOperandNamed(Vals[i].getName(), OpIdx))
- continue;
-
- // Get the bit range for this operand:
- unsigned bitStart = bi++, bitWidth = 1;
- for (; bi < Bits.getNumBits(); ++bi) {
- VarInit *Var = nullptr;
- VarBitInit *BI = dyn_cast<VarBitInit>(Bits.getBit(bi));
- if (BI)
- Var = dyn_cast<VarInit>(BI->getBitVar());
- else
- Var = dyn_cast<VarInit>(Bits.getBit(bi));
-
- if (!Var)
- break;
-
- if (Var->getName() != Vals[i].getName())
- break;
-
- ++bitWidth;
- }
-
- unsigned NumberOps = CGI.Operands.size();
- while (NumberedOp < NumberOps &&
- (CGI.Operands.isFlatOperandNotEmitted(NumberedOp) ||
- (!NamedOpIndices.empty() &&
- NamedOpIndices.count(
- CGI.Operands.getSubOperandNumber(NumberedOp).first))))
- ++NumberedOp;
-
- OpIdx = NumberedOp++;
-
- // OpIdx now holds the ordered operand number of Vals[i].
- std::pair<unsigned, unsigned> SO =
- CGI.Operands.getSubOperandNumber(OpIdx);
- const std::string &Name = CGI.Operands[SO.first].Name;
-
- LLVM_DEBUG(dbgs() << "Numbered operand mapping for " << Def.getName()
- << ": " << Name << "(" << SO.first << ", "
- << SO.second << ") => " << Vals[i].getName() << "\n");
-
- std::string Decoder;
- Record *TypeRecord = CGI.Operands[SO.first].Rec;
-
- RecordVal *DecoderString = TypeRecord->getValue("DecoderMethod");
- StringInit *String =
- DecoderString ? dyn_cast<StringInit>(DecoderString->getValue())
- : nullptr;
- if (String && String->getValue() != "")
- Decoder = std::string(String->getValue());
-
- if (Decoder == "" && CGI.Operands[SO.first].MIOperandInfo &&
- CGI.Operands[SO.first].MIOperandInfo->getNumArgs()) {
- Init *Arg = CGI.Operands[SO.first].MIOperandInfo->getArg(SO.second);
- if (DefInit *DI = cast<DefInit>(Arg))
- TypeRecord = DI->getDef();
- }
-
- bool isReg = false;
- if (TypeRecord->isSubClassOf("RegisterOperand"))
- TypeRecord = TypeRecord->getValueAsDef("RegClass");
- if (TypeRecord->isSubClassOf("RegisterClass")) {
- Decoder = "Decode" + TypeRecord->getName().str() + "RegisterClass";
- isReg = true;
- } else if (TypeRecord->isSubClassOf("PointerLikeRegClass")) {
- Decoder = "DecodePointerLikeRegClass" +
- utostr(TypeRecord->getValueAsInt("RegClassKind"));
- isReg = true;
- }
-
- DecoderString = TypeRecord->getValue("DecoderMethod");
- String = DecoderString ? dyn_cast<StringInit>(DecoderString->getValue())
- : nullptr;
- if (!isReg && String && String->getValue() != "")
- Decoder = std::string(String->getValue());
-
- RecordVal *HasCompleteDecoderVal =
- TypeRecord->getValue("hasCompleteDecoder");
- BitInit *HasCompleteDecoderBit =
- HasCompleteDecoderVal
- ? dyn_cast<BitInit>(HasCompleteDecoderVal->getValue())
- : nullptr;
- bool HasCompleteDecoder =
- HasCompleteDecoderBit ? HasCompleteDecoderBit->getValue() : true;
-
- OperandInfo OpInfo(Decoder, HasCompleteDecoder);
- OpInfo.addField(bitStart, bitWidth, 0);
-
- NumberedInsnOperands[Name].push_back(OpInfo);
-
- // FIXME: For complex operands with custom decoders we can't handle tied
- // sub-operands automatically. Skip those here and assume that this is
- // fixed up elsewhere.
- if (CGI.Operands[SO.first].MIOperandInfo &&
- CGI.Operands[SO.first].MIOperandInfo->getNumArgs() > 1 && String &&
- String->getValue() != "")
- NumberedInsnOperandsNoTie.insert(Name);
- }
- }
-
// For each operand, see if we can figure out where it is encoded.
for (const auto &Op : InOutOperands) {
Init *OpInit = Op.first;
StringRef OpName = Op.second;
- if (SupportPositionalDecoding) {
- if (!NumberedInsnOperands[std::string(OpName)].empty()) {
- llvm::append_range(InsnOperands,
- NumberedInsnOperands[std::string(OpName)]);
- continue;
- }
- if (!NumberedInsnOperands[TiedNames[std::string(OpName)]].empty()) {
- if (!NumberedInsnOperandsNoTie.count(
- TiedNames[std::string(OpName)])) {
- // Figure out to which (sub)operand we're tied.
- unsigned i =
- CGI.Operands.getOperandNamed(TiedNames[std::string(OpName)]);
- int tiedTo = CGI.Operands[i].getTiedRegister();
- if (tiedTo == -1) {
- i = CGI.Operands.getOperandNamed(OpName);
- tiedTo = CGI.Operands[i].getTiedRegister();
- }
-
- if (tiedTo != -1) {
- std::pair<unsigned, unsigned> SO =
- CGI.Operands.getSubOperandNumber(tiedTo);
-
- InsnOperands.push_back(
- NumberedInsnOperands[TiedNames[std::string(OpName)]]
- [SO.second]);
- }
- }
- continue;
- }
- }
-
// We're ready to find the instruction encoding locations for this operand.
// First, find the operand type ("OpInit"), and sub-op names
More information about the llvm-commits
mailing list