[Mlir-commits] [mlir] 5520e07 - [mlir:Parser][NFC] Replace SSAUseInfo with OpAsmParser::UnresolvedOperand
River Riddle
llvmlistbot at llvm.org
Wed Apr 6 18:25:28 PDT 2022
Author: River Riddle
Date: 2022-04-06T18:25:08-07:00
New Revision: 5520e07f46630fe58d459e66154550ea4a7c9674
URL: https://github.com/llvm/llvm-project/commit/5520e07f46630fe58d459e66154550ea4a7c9674
DIFF: https://github.com/llvm/llvm-project/commit/5520e07f46630fe58d459e66154550ea4a7c9674.diff
LOG: [mlir:Parser][NFC] Replace SSAUseInfo with OpAsmParser::UnresolvedOperand
These are functionally identical, and merging the two removes the number of
redundant conversions within the parser.
Added:
Modified:
mlir/lib/Parser/Parser.cpp
Removed:
################################################################################
diff --git a/mlir/lib/Parser/Parser.cpp b/mlir/lib/Parser/Parser.cpp
index 607d6bbd85f97..22daae93fa211 100644
--- a/mlir/lib/Parser/Parser.cpp
+++ b/mlir/lib/Parser/Parser.cpp
@@ -248,15 +248,8 @@ class OperationParser : public Parser {
// SSA Value Handling
//===--------------------------------------------------------------------===//
- /// This represents a use of an SSA value in the program. The first two
- /// entries in the tuple are the name and result number of a reference. The
- /// third is the location of the reference, which is used in case this ends
- /// up being a use of an undefined value.
- struct SSAUseInfo {
- StringRef name; // Value name, e.g. %42 or %abc
- unsigned number; // Number, specified with #12
- SMLoc loc; // Location of first definition or use.
- };
+ using UnresolvedOperand = OpAsmParser::UnresolvedOperand;
+
struct DeferredLocInfo {
SMLoc loc;
StringRef identifier;
@@ -269,20 +262,21 @@ class OperationParser : public Parser {
ParseResult popSSANameScope();
/// Register a definition of a value with the symbol table.
- ParseResult addDefinition(SSAUseInfo useInfo, Value value);
+ ParseResult addDefinition(UnresolvedOperand useInfo, Value value);
/// Parse an optional list of SSA uses into 'results'.
- ParseResult parseOptionalSSAUseList(SmallVectorImpl<SSAUseInfo> &results);
+ ParseResult
+ parseOptionalSSAUseList(SmallVectorImpl<UnresolvedOperand> &results);
/// Parse a single SSA use into 'result'.
- ParseResult parseSSAUse(SSAUseInfo &result);
+ ParseResult parseSSAUse(UnresolvedOperand &result);
/// Given a reference to an SSA value and its type, return a reference. This
/// returns null on failure.
- Value resolveSSAUse(SSAUseInfo useInfo, Type type);
+ Value resolveSSAUse(UnresolvedOperand useInfo, Type type);
- ParseResult
- parseSSADefOrUseAndType(function_ref<ParseResult(SSAUseInfo, Type)> action);
+ ParseResult parseSSADefOrUseAndType(
+ function_ref<ParseResult(UnresolvedOperand, Type)> action);
ParseResult parseOptionalSSAUseAndTypeList(SmallVectorImpl<Value> &results);
@@ -320,7 +314,7 @@ class OperationParser : public Parser {
/// skip parsing that component.
ParseResult parseGenericOperationAfterOpName(
OperationState &result,
- Optional<ArrayRef<SSAUseInfo>> parsedOperandUseInfo = llvm::None,
+ Optional<ArrayRef<UnresolvedOperand>> parsedOperandUseInfo = llvm::None,
Optional<ArrayRef<Block *>> parsedSuccessors = llvm::None,
Optional<MutableArrayRef<std::unique_ptr<Region>>> parsedRegions =
llvm::None,
@@ -369,15 +363,16 @@ class OperationParser : public Parser {
/// If non-empty, 'argLocations' contains an optional locations for each
/// argument. 'isIsolatedNameScope' indicates if the naming scope of this
/// region is isolated from those above.
- ParseResult parseRegion(Region ®ion,
- ArrayRef<std::pair<SSAUseInfo, Type>> entryArguments,
- ArrayRef<Location> argLocations,
- bool isIsolatedNameScope = false);
+ ParseResult
+ parseRegion(Region ®ion,
+ ArrayRef<std::pair<UnresolvedOperand, Type>> entryArguments,
+ ArrayRef<Location> argLocations,
+ bool isIsolatedNameScope = false);
/// Parse a region body into 'region'.
ParseResult
parseRegionBody(Region ®ion, SMLoc startLoc,
- ArrayRef<std::pair<SSAUseInfo, Type>> entryArguments,
+ ArrayRef<std::pair<UnresolvedOperand, Type>> entryArguments,
ArrayRef<Location> argLocations, bool isIsolatedNameScope);
//===--------------------------------------------------------------------===//
@@ -639,7 +634,8 @@ ParseResult OperationParser::popSSANameScope() {
}
/// Register a definition of a value with the symbol table.
-ParseResult OperationParser::addDefinition(SSAUseInfo useInfo, Value value) {
+ParseResult OperationParser::addDefinition(UnresolvedOperand useInfo,
+ Value value) {
auto &entries = getSSAValueEntry(useInfo.name);
// Make sure there is a slot for this value.
@@ -650,14 +646,14 @@ ParseResult OperationParser::addDefinition(SSAUseInfo useInfo, Value value) {
// or a forward reference.
if (auto existing = entries[useInfo.number].value) {
if (!isForwardRefPlaceholder(existing)) {
- return emitError(useInfo.loc)
+ return emitError(useInfo.location)
.append("redefinition of SSA value '", useInfo.name, "'")
.attachNote(getEncodedSourceLocation(entries[useInfo.number].loc))
.append("previously defined here");
}
if (existing.getType() != value.getType()) {
- return emitError(useInfo.loc)
+ return emitError(useInfo.location)
.append("definition of SSA value '", useInfo.name, "#",
useInfo.number, "' has type ", value.getType())
.attachNote(getEncodedSourceLocation(entries[useInfo.number].loc))
@@ -678,7 +674,7 @@ ParseResult OperationParser::addDefinition(SSAUseInfo useInfo, Value value) {
}
/// Record this definition for the current scope.
- entries[useInfo.number] = {value, useInfo.loc};
+ entries[useInfo.number] = {value, useInfo.location};
recordDefinition(useInfo.name);
return success();
}
@@ -688,12 +684,12 @@ ParseResult OperationParser::addDefinition(SSAUseInfo useInfo, Value value) {
/// ssa-use-list ::= ssa-use (`,` ssa-use)*
/// ssa-use-list-opt ::= ssa-use-list?
///
-ParseResult
-OperationParser::parseOptionalSSAUseList(SmallVectorImpl<SSAUseInfo> &results) {
+ParseResult OperationParser::parseOptionalSSAUseList(
+ SmallVectorImpl<UnresolvedOperand> &results) {
if (getToken().isNot(Token::percent_identifier))
return success();
return parseCommaSeparatedList([&]() -> ParseResult {
- SSAUseInfo result;
+ UnresolvedOperand result;
if (parseSSAUse(result))
return failure();
results.push_back(result);
@@ -705,10 +701,10 @@ OperationParser::parseOptionalSSAUseList(SmallVectorImpl<SSAUseInfo> &results) {
///
/// ssa-use ::= ssa-id
///
-ParseResult OperationParser::parseSSAUse(SSAUseInfo &result) {
+ParseResult OperationParser::parseSSAUse(UnresolvedOperand &result) {
result.name = getTokenSpelling();
result.number = 0;
- result.loc = getToken().getLoc();
+ result.location = getToken().getLoc();
if (parseToken(Token::percent_identifier, "expected SSA operand"))
return failure();
@@ -726,14 +722,14 @@ ParseResult OperationParser::parseSSAUse(SSAUseInfo &result) {
/// Given an unbound reference to an SSA value and its type, return the value
/// it specifies. This returns null on failure.
-Value OperationParser::resolveSSAUse(SSAUseInfo useInfo, Type type) {
+Value OperationParser::resolveSSAUse(UnresolvedOperand useInfo, Type type) {
auto &entries = getSSAValueEntry(useInfo.name);
// Functor used to record the use of the given value if the assembly state
// field is populated.
auto maybeRecordUse = [&](Value value) {
if (state.asmState)
- state.asmState->addUses(value, useInfo.loc);
+ state.asmState->addUses(value, useInfo.location);
return value;
};
@@ -744,7 +740,7 @@ Value OperationParser::resolveSSAUse(SSAUseInfo useInfo, Type type) {
if (result.getType() == type)
return maybeRecordUse(result);
- emitError(useInfo.loc, "use of value '")
+ emitError(useInfo.location, "use of value '")
.append(useInfo.name,
"' expects
diff erent type than prior uses: ", type, " vs ",
result.getType())
@@ -760,13 +756,13 @@ Value OperationParser::resolveSSAUse(SSAUseInfo useInfo, Type type) {
// If the value has already been defined and this is an overly large result
// number, diagnose that.
if (entries[0].value && !isForwardRefPlaceholder(entries[0].value))
- return (emitError(useInfo.loc, "reference to invalid result number"),
+ return (emitError(useInfo.location, "reference to invalid result number"),
nullptr);
// Otherwise, this is a forward reference. Create a placeholder and remember
// that we did so.
- Value result = createForwardRefPlaceholder(useInfo.loc, type);
- entries[useInfo.number] = {result, useInfo.loc};
+ Value result = createForwardRefPlaceholder(useInfo.location, type);
+ entries[useInfo.number] = {result, useInfo.location};
return maybeRecordUse(result);
}
@@ -774,8 +770,8 @@ Value OperationParser::resolveSSAUse(SSAUseInfo useInfo, Type type) {
///
/// ssa-use-and-type ::= ssa-use `:` type
ParseResult OperationParser::parseSSADefOrUseAndType(
- function_ref<ParseResult(SSAUseInfo, Type)> action) {
- SSAUseInfo useInfo;
+ function_ref<ParseResult(UnresolvedOperand, Type)> action) {
+ UnresolvedOperand useInfo;
if (parseSSAUse(useInfo) ||
parseToken(Token::colon, "expected ':' and type for SSA operand"))
return failure();
@@ -795,7 +791,7 @@ ParseResult OperationParser::parseSSADefOrUseAndType(
///
ParseResult OperationParser::parseOptionalSSAUseAndTypeList(
SmallVectorImpl<Value> &results) {
- SmallVector<SSAUseInfo, 4> valueIDs;
+ SmallVector<UnresolvedOperand, 4> valueIDs;
if (parseOptionalSSAUseList(valueIDs))
return failure();
@@ -947,7 +943,7 @@ ParseResult OperationParser::parseOperation() {
unsigned opResI = 0;
for (ResultRecord &resIt : resultIDs) {
for (unsigned subRes : llvm::seq<unsigned>(0, std::get<1>(resIt))) {
- if (addDefinition({std::get<0>(resIt), subRes, std::get<2>(resIt)},
+ if (addDefinition({std::get<2>(resIt), std::get<0>(resIt), subRes},
op->getResult(opResI++)))
return failure();
}
@@ -1013,14 +1009,15 @@ struct CleanupOpStateRegions {
} // namespace
ParseResult OperationParser::parseGenericOperationAfterOpName(
- OperationState &result, Optional<ArrayRef<SSAUseInfo>> parsedOperandUseInfo,
+ OperationState &result,
+ Optional<ArrayRef<UnresolvedOperand>> parsedOperandUseInfo,
Optional<ArrayRef<Block *>> parsedSuccessors,
Optional<MutableArrayRef<std::unique_ptr<Region>>> parsedRegions,
Optional<ArrayRef<NamedAttribute>> parsedAttributes,
Optional<FunctionType> parsedFnType) {
// Parse the operand list, if not explicitly provided.
- SmallVector<SSAUseInfo, 8> opInfo;
+ SmallVector<UnresolvedOperand, 8> opInfo;
if (!parsedOperandUseInfo) {
if (parseToken(Token::l_paren, "expected '(' to start operand list") ||
parseOptionalSSAUseList(opInfo) ||
@@ -1220,26 +1217,9 @@ class CustomOpAsmParser : public AsmParserImpl<OpAsmParser> {
Optional<MutableArrayRef<std::unique_ptr<Region>>> parsedRegions,
Optional<ArrayRef<NamedAttribute>> parsedAttributes,
Optional<FunctionType> parsedFnType) final {
-
- // TODO: The types, UnresolvedOperand and SSAUseInfo, both share the same
- // members but in
diff erent order. It would be cleaner to make one alias of
- // the other, making the following code redundant.
- SmallVector<OperationParser::SSAUseInfo> parsedOperandUseInfo;
- if (parsedUnresolvedOperands) {
- for (const UnresolvedOperand &parsedUnresolvedOperand :
- *parsedUnresolvedOperands)
- parsedOperandUseInfo.push_back({
- parsedUnresolvedOperand.name,
- parsedUnresolvedOperand.number,
- parsedUnresolvedOperand.location,
- });
- }
-
return parser.parseGenericOperationAfterOpName(
- result,
- parsedUnresolvedOperands ? llvm::makeArrayRef(parsedOperandUseInfo)
- : llvm::None,
- parsedSuccessors, parsedRegions, parsedAttributes, parsedFnType);
+ result, parsedUnresolvedOperands, parsedSuccessors, parsedRegions,
+ parsedAttributes, parsedFnType);
}
//===--------------------------------------------------------------------===//
// Utilities
@@ -1291,11 +1271,11 @@ class CustomOpAsmParser : public AsmParserImpl<OpAsmParser> {
/// Parse a single operand.
ParseResult parseOperand(UnresolvedOperand &result) override {
- OperationParser::SSAUseInfo useInfo;
+ OperationParser::UnresolvedOperand useInfo;
if (parser.parseSSAUse(useInfo))
return failure();
- result = {useInfo.loc, useInfo.name, useInfo.number};
+ result = {useInfo.location, useInfo.name, useInfo.number};
return success();
}
@@ -1381,9 +1361,7 @@ class CustomOpAsmParser : public AsmParserImpl<OpAsmParser> {
/// Resolve an operand to an SSA value, emitting an error on failure.
ParseResult resolveOperand(const UnresolvedOperand &operand, Type type,
SmallVectorImpl<Value> &result) override {
- OperationParser::SSAUseInfo operandInfo = {operand.name, operand.number,
- operand.location};
- if (auto value = parser.resolveSSAUse(operandInfo, type)) {
+ if (auto value = parser.resolveSSAUse(operand, type)) {
result.push_back(value);
return success();
}
@@ -1456,15 +1434,10 @@ class CustomOpAsmParser : public AsmParserImpl<OpAsmParser> {
assert(arguments.size() == argTypes.size() &&
"mismatching number of arguments and types");
- SmallVector<std::pair<OperationParser::SSAUseInfo, Type>, 2>
+ SmallVector<std::pair<OperationParser::UnresolvedOperand, Type>, 2>
regionArguments;
- for (auto pair : llvm::zip(arguments, argTypes)) {
- const UnresolvedOperand &operand = std::get<0>(pair);
- Type type = std::get<1>(pair);
- OperationParser::SSAUseInfo operandInfo = {operand.name, operand.number,
- operand.location};
- regionArguments.emplace_back(operandInfo, type);
- }
+ for (auto pair : llvm::zip(arguments, argTypes))
+ regionArguments.emplace_back(std::get<0>(pair), std::get<1>(pair));
// Try to parse the region.
(void)isIsolatedFromAbove;
@@ -1823,7 +1796,8 @@ OperationParser::parseTrailingLocationSpecifier(OpOrArgument opOrArgument) {
ParseResult OperationParser::parseRegion(
Region ®ion,
- ArrayRef<std::pair<OperationParser::SSAUseInfo, Type>> entryArguments,
+ ArrayRef<std::pair<OperationParser::UnresolvedOperand, Type>>
+ entryArguments,
ArrayRef<Location> argLocations, bool isIsolatedNameScope) {
// Parse the '{'.
Token lBraceTok = getToken();
@@ -1851,7 +1825,8 @@ ParseResult OperationParser::parseRegion(
ParseResult OperationParser::parseRegionBody(
Region ®ion, SMLoc startLoc,
- ArrayRef<std::pair<OperationParser::SSAUseInfo, Type>> entryArguments,
+ ArrayRef<std::pair<OperationParser::UnresolvedOperand, Type>>
+ entryArguments,
ArrayRef<Location> argLocations, bool isIsolatedNameScope) {
assert(argLocations.empty() || argLocations.size() == entryArguments.size());
auto currentPt = opBuilder.saveInsertionPoint();
@@ -1882,20 +1857,21 @@ ParseResult OperationParser::parseRegionBody(
// Ensure that the argument was not already defined.
if (auto defLoc = getReferenceLoc(argInfo.name, argInfo.number)) {
- return emitError(argInfo.loc, "region entry argument '" + argInfo.name +
- "' is already in use")
+ return emitError(argInfo.location, "region entry argument '" +
+ argInfo.name +
+ "' is already in use")
.attachNote(getEncodedSourceLocation(*defLoc))
<< "previously referenced here";
}
BlockArgument arg = block->addArgument(
placeholderArgPair.second,
argLocations.empty()
- ? getEncodedSourceLocation(placeholderArgPair.first.loc)
+ ? getEncodedSourceLocation(placeholderArgPair.first.location)
: argLocations[argIndex]);
// Add a definition of this arg to the assembly state if provided.
if (state.asmState)
- state.asmState->addDefinition(arg, argInfo.loc);
+ state.asmState->addDefinition(arg, argInfo.location);
// Record the definition for this argument.
if (addDefinition(argInfo, arg))
@@ -2045,7 +2021,7 @@ ParseResult OperationParser::parseOptionalBlockArgList(Block *owner) {
return parseCommaSeparatedList(Delimiter::Paren, [&]() -> ParseResult {
return parseSSADefOrUseAndType(
- [&](SSAUseInfo useInfo, Type type) -> ParseResult {
+ [&](UnresolvedOperand useInfo, Type type) -> ParseResult {
BlockArgument arg;
// If we are defining existing arguments, ensure that the argument
@@ -2060,7 +2036,7 @@ ParseResult OperationParser::parseOptionalBlockArgList(Block *owner) {
if (arg.getType() != type)
return emitError("argument and block argument type mismatch");
} else {
- auto loc = getEncodedSourceLocation(useInfo.loc);
+ auto loc = getEncodedSourceLocation(useInfo.location);
arg = owner->addArgument(type, loc);
}
@@ -2072,7 +2048,7 @@ ParseResult OperationParser::parseOptionalBlockArgList(Block *owner) {
// Mark this block argument definition in the parser state if it was
// provided.
if (state.asmState)
- state.asmState->addDefinition(arg, useInfo.loc);
+ state.asmState->addDefinition(arg, useInfo.location);
return addDefinition(useInfo, arg);
});
More information about the Mlir-commits
mailing list