diff --git a/llvm/include/llvm/Target/Target.td b/llvm/include/llvm/Target/Target.td --- a/llvm/include/llvm/Target/Target.td +++ b/llvm/include/llvm/Target/Target.td @@ -1058,45 +1058,6 @@ // // 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 --- 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 --- a/llvm/test/TableGen/MissingOperandField.td +++ b/llvm/test/TableGen/MissingOperandField.td @@ -15,8 +15,8 @@ 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 --- a/llvm/utils/TableGen/CodeEmitterGen.cpp +++ b/llvm/utils/TableGen/CodeEmitterGen.cpp @@ -51,8 +51,7 @@ std::string getInstructionCaseForEncoding(Record *R, Record *EncodingDef, CodeGenTarget &Target); bool addCodeToMergeInOperand(Record *R, BitsInit *BI, - const std::string &VarName, unsigned &NumberedOp, - std::set &NamedOpIndices, + const std::string &VarName, std::string &Case, CodeGenTarget &Target); void emitInstructionBaseValues( @@ -81,8 +80,6 @@ // Returns true if it succeeds, false if an error. bool CodeEmitterGen::addCodeToMergeInOperand(Record *R, BitsInit *BI, const std::string &VarName, - unsigned &NumberedOp, - std::set &NamedOpIndices, std::string &Case, CodeGenTarget &Target) { CodeGenInstruction &CGI = Target.getInstruction(R); @@ -114,52 +111,8 @@ // 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 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 @@ CodeGenTarget &Target) { std::string Case; BitsInit *BI = EncodingDef->getValueAsBitsInit("Inst"); - unsigned NumberedOp = 0; - std::set 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 @@ 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 --- a/llvm/utils/TableGen/DecoderEmitter.cpp +++ b/llvm/utils/TableGen/DecoderEmitter.cpp @@ -2029,193 +2029,11 @@ if (IsVarLenInst) { parseVarLenInstOperand(EncodingDef, InsnOperands, CGI); } else { - std::map> NumberedInsnOperands; - std::set NumberedInsnOperandsNoTie; - bool SupportPositionalDecoding = - Target.getInstructionSet()->getValueAsBit( - "useDeprecatedPositionallyEncodedOperands") && - Target.getInstructionSet()->getValueAsBit( - "decodePositionallyEncodedOperands"); - if (SupportPositionalDecoding) { - const std::vector &Vals = Def.getValues(); - unsigned NumberedOp = 0; - - std::set 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(Bits.getBit(bi)); - if (BI) - Var = dyn_cast(BI->getBitVar()); - else - Var = dyn_cast(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 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(Bits.getBit(bi)); - if (BI) - Var = dyn_cast(BI->getBitVar()); - else - Var = dyn_cast(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 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(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(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(DecoderString->getValue()) - : nullptr; - if (!isReg && String && String->getValue() != "") - Decoder = std::string(String->getValue()); - - RecordVal *HasCompleteDecoderVal = - TypeRecord->getValue("hasCompleteDecoder"); - BitInit *HasCompleteDecoderBit = - HasCompleteDecoderVal - ? dyn_cast(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 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