[TableGen] Delete support for deprecated positional matching.

After the work in a538d1f13a 5351878ba1 372240dfe3, 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
This commit is contained in:
James Y Knight
2023-02-08 10:47:11 -05:00
parent 3ea4c5014d
commit b87dc35669
5 changed files with 7 additions and 326 deletions

View File

@@ -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.

View File

@@ -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);
}

View File

@@ -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;

View File

@@ -51,8 +51,7 @@ private:
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) {

View File

@@ -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