[LLVM][TableGen] Minor cleanup in CodeGenInstruction #142721
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Use StringRef instead of std::string for `AsmString`. - Use range for loop in `getTiedRegister`. - Change `hadOperandNamed` to return index as std::optional and rename it to `findOperandNamed`. - Change `SubOperandAlias` to return std::optional and rename it to `findSubOperandAlias`.
|
@llvm/pr-subscribers-tablegen Author: Rahul Joshi (jurahul) Changes
Full diff: https://github.com/llvm/llvm-project/pull/142721.diff 4 Files Affected:
diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index 9792eb41ea5d7..026e3fe7a6161 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -1714,11 +1714,11 @@ void AsmMatcherInfo::buildInstructionOperandReference(MatchableInfo *II,
MatchableInfo::AsmOperand *Op = &II->AsmOperands[AsmOpIdx];
// Map this token to an operand.
- unsigned Idx;
- if (!Operands.hasOperandNamed(OperandName, Idx))
+ std::optional<unsigned> MayBeIdx = Operands.findOperandNamed(OperandName);
+ if (!MayBeIdx)
PrintFatalError(II->TheDef->getLoc(),
"error: unable to find operand: '" + OperandName + "'");
-
+ unsigned Idx = *MayBeIdx;
// If the instruction operand has multiple suboperands, but the parser
// match class for the asm operand is still the default "ImmAsmOperand",
// then handle each suboperand separately.
diff --git a/llvm/utils/TableGen/CodeEmitterGen.cpp b/llvm/utils/TableGen/CodeEmitterGen.cpp
index 2fe40450abfda..14dffb438fcba 100644
--- a/llvm/utils/TableGen/CodeEmitterGen.cpp
+++ b/llvm/utils/TableGen/CodeEmitterGen.cpp
@@ -123,12 +123,11 @@ bool CodeEmitterGen::addCodeToMergeInOperand(const Record *R,
// operand number. Non-matching operands are assumed to be in
// order.
unsigned OpIdx;
- std::pair<unsigned, unsigned> SubOp;
- if (CGI.Operands.hasSubOperandAlias(VarName, SubOp)) {
- OpIdx = CGI.Operands[SubOp.first].MIOperandNo + SubOp.second;
- } else if (CGI.Operands.hasOperandNamed(VarName, OpIdx)) {
+ if (auto SubOp = CGI.Operands.findSubOperandAlias(VarName)) {
+ OpIdx = CGI.Operands[SubOp->first].MIOperandNo + SubOp->second;
+ } else if (auto MayBeOpIdx = CGI.Operands.findOperandNamed(VarName)) {
// Get the machine operand number for the indicated operand.
- OpIdx = CGI.Operands[OpIdx].MIOperandNo;
+ OpIdx = CGI.Operands[*MayBeOpIdx].MIOperandNo;
} else {
PrintError(R, Twine("No operand named ") + VarName + " in record " +
R->getName());
diff --git a/llvm/utils/TableGen/Common/CodeGenInstruction.cpp b/llvm/utils/TableGen/Common/CodeGenInstruction.cpp
index 0dfcf200d7e4b..3dbf2b9b446e8 100644
--- a/llvm/utils/TableGen/Common/CodeGenInstruction.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenInstruction.cpp
@@ -183,8 +183,8 @@ CGIOperandList::CGIOperandList(const Record *R) : TheDef(R) {
// If we have no explicit sub-op dag, but have an top-level encoder
// method, the single encoder will multiple sub-ops, itself.
OpInfo.EncoderMethodNames[0] = EncoderMethod;
- for (unsigned j = 1; j < NumOps; ++j)
- OpInfo.DoNotEncode[j] = true;
+ OpInfo.DoNotEncode.set();
+ OpInfo.DoNotEncode[0] = false;
}
MIOperandNo += NumOps;
@@ -199,36 +199,31 @@ CGIOperandList::CGIOperandList(const Record *R) : TheDef(R) {
/// specified name, abort.
///
unsigned CGIOperandList::getOperandNamed(StringRef Name) const {
- unsigned OpIdx;
- if (hasOperandNamed(Name, OpIdx))
- return OpIdx;
+ std::optional<unsigned> OpIdx = findOperandNamed(Name);
+ if (OpIdx)
+ return *OpIdx;
PrintFatalError(TheDef->getLoc(), "'" + TheDef->getName() +
"' does not have an operand named '$" +
Name + "'!");
}
-/// hasOperandNamed - Query whether the instruction has an operand of the
-/// given name. If so, return true and set OpIdx to the index of the
-/// operand. Otherwise, return false.
-bool CGIOperandList::hasOperandNamed(StringRef Name, unsigned &OpIdx) const {
+/// findOperandNamed - Query whether the instruction has an operand of the
+/// given name. If so, the index of the operand. Otherwise, return std::nullopt.
+std::optional<unsigned> CGIOperandList::findOperandNamed(StringRef Name) const {
assert(!Name.empty() && "Cannot search for operand with no name!");
- for (unsigned i = 0, e = OperandList.size(); i != e; ++i)
- if (OperandList[i].Name == Name) {
- OpIdx = i;
- return true;
- }
- return false;
+ for (const auto &[Index, Opnd] : enumerate(OperandList))
+ if (Opnd.Name == Name)
+ return Index;
+ return std::nullopt;
}
-bool CGIOperandList::hasSubOperandAlias(
- StringRef Name, std::pair<unsigned, unsigned> &SubOp) const {
+std::optional<std::pair<unsigned, unsigned>>
+CGIOperandList::findSubOperandAlias(StringRef Name) const {
assert(!Name.empty() && "Cannot search for operand with no name!");
auto SubOpIter = SubOpAliases.find(Name);
- if (SubOpIter != SubOpAliases.end()) {
- SubOp = SubOpIter->second;
- return true;
- }
- return false;
+ if (SubOpIter != SubOpAliases.end())
+ return SubOpIter->second;
+ return std::nullopt;
}
std::pair<unsigned, unsigned>
@@ -251,9 +246,7 @@ CGIOperandList::ParseOperandName(StringRef Op, bool AllowWholeOp) {
OpName = OpName.substr(0, DotIdx);
}
- unsigned OpIdx;
-
- if (std::pair<unsigned, unsigned> SubOp; hasSubOperandAlias(OpName, SubOp)) {
+ if (auto SubOp = findSubOperandAlias(OpName)) {
// Found a name for a piece of an operand, just return it directly.
if (!SubOpName.empty()) {
PrintFatalError(
@@ -262,10 +255,10 @@ CGIOperandList::ParseOperandName(StringRef Op, bool AllowWholeOp) {
": Cannot use dotted suboperand name within suboperand '" +
OpName + "'");
}
- return SubOp;
+ return *SubOp;
}
- OpIdx = getOperandNamed(OpName);
+ unsigned OpIdx = getOperandNamed(OpName);
if (SubOpName.empty()) { // If no suboperand name was specified:
// If one was needed, throw.
@@ -435,7 +428,7 @@ void CGIOperandList::ProcessDisableEncoding(StringRef DisableEncoding) {
CodeGenInstruction::CodeGenInstruction(const Record *R)
: TheDef(R), Operands(R), InferredFrom(nullptr) {
Namespace = R->getValueAsString("Namespace");
- AsmString = R->getValueAsString("AsmString").str();
+ AsmString = R->getValueAsString("AsmString");
isPreISelOpcode = R->getValueAsBit("isPreISelOpcode");
isReturn = R->getValueAsBit("isReturn");
diff --git a/llvm/utils/TableGen/Common/CodeGenInstruction.h b/llvm/utils/TableGen/Common/CodeGenInstruction.h
index e38979af3909d..1ae913c262b2e 100644
--- a/llvm/utils/TableGen/Common/CodeGenInstruction.h
+++ b/llvm/utils/TableGen/Common/CodeGenInstruction.h
@@ -127,11 +127,9 @@ class CGIOperandList {
/// getTiedOperand - If this operand is tied to another one, return the
/// other operand number. Otherwise, return -1.
int getTiedRegister() const {
- for (unsigned j = 0, e = Constraints.size(); j != e; ++j) {
- const CGIOperandList::ConstraintInfo &CI = Constraints[j];
+ for (const CGIOperandList::ConstraintInfo &CI : Constraints)
if (CI.isTied())
return CI.getTiedOperand();
- }
return -1;
}
};
@@ -177,13 +175,13 @@ class CGIOperandList {
/// specified name, abort.
unsigned getOperandNamed(StringRef Name) const;
- /// hasOperandNamed - Query whether the instruction has an operand of the
- /// given name. If so, return true and set OpIdx to the index of the
- /// operand. Otherwise, return false.
- bool hasOperandNamed(StringRef Name, unsigned &OpIdx) const;
+ /// findOperandNamed - Query whether the instruction has an operand of the
+ /// given name. If so, the index of the operand. Otherwise, return
+ /// std::nullopt.
+ std::optional<unsigned> findOperandNamed(StringRef Name) const;
- bool hasSubOperandAlias(StringRef Name,
- std::pair<unsigned, unsigned> &SubOp) const;
+ std::optional<std::pair<unsigned, unsigned>>
+ findSubOperandAlias(StringRef Name) const;
/// ParseOperandName - Parse an operand name like "$foo" or "$foo.bar",
/// where $foo is a whole operand and $foo.bar refers to a suboperand.
@@ -227,7 +225,7 @@ class CodeGenInstruction {
/// AsmString - The format string used to emit a .s file for the
/// instruction.
- std::string AsmString;
+ StringRef AsmString;
/// Operands - This is information about the (ins) and (outs) list specified
/// to the instruction.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
AsmString.getTiedRegister.hadOperandNamedto return index as std::optional and rename it tofindOperandNamed.SubOperandAliasto return std::optional and rename it tofindSubOperandAlias.