Skip to content

Commit

Permalink
[mlir][SymbolTable] Use Identifier instead of StringRef when looking …
Browse files Browse the repository at this point in the history
…up symbol name attributes

Using an Identifier is much more efficient for attribute lookups because it uses pointer comparison as opposed to string comparison.

Differential Revision: https://reviews.llvm.org/D89660
  • Loading branch information
River707 committed Oct 27, 2020
1 parent 67f52f3 commit eda450b
Showing 1 changed file with 15 additions and 3 deletions.
18 changes: 15 additions & 3 deletions mlir/lib/IR/SymbolTable.cpp
Expand Up @@ -27,6 +27,11 @@ static Optional<StringRef> getNameIfSymbol(Operation *symbol) {
symbol->getAttrOfType<StringAttr>(SymbolTable::getSymbolAttrName());
return nameAttr ? nameAttr.getValue() : Optional<StringRef>();
}
static Optional<StringRef> getNameIfSymbol(Operation *symbol,
Identifier symbolAttrNameId) {
auto nameAttr = symbol->getAttrOfType<StringAttr>(symbolAttrNameId);
return nameAttr ? nameAttr.getValue() : Optional<StringRef>();
}

/// Computes the nested symbol reference attribute for the symbol 'symbolName'
/// that are usable within the symbol table operations from 'symbol' as far up
Expand All @@ -49,12 +54,15 @@ collectValidReferencesFor(Operation *symbol, StringRef symbolName,

// Collect references until 'symbolTableOp' reaches 'within'.
SmallVector<FlatSymbolRefAttr, 1> nestedRefs(1, leafRef);
Identifier symbolNameId =
Identifier::get(SymbolTable::getSymbolAttrName(), ctx);
do {
// Each parent of 'symbol' should define a symbol table.
if (!symbolTableOp->hasTrait<OpTrait::SymbolTable>())
return failure();
// Each parent of 'symbol' should also be a symbol.
Optional<StringRef> symbolTableName = getNameIfSymbol(symbolTableOp);
Optional<StringRef> symbolTableName =
getNameIfSymbol(symbolTableOp, symbolNameId);
if (!symbolTableName)
return failure();
results.push_back(SymbolRefAttr::get(*symbolTableName, nestedRefs, ctx));
Expand Down Expand Up @@ -106,8 +114,10 @@ SymbolTable::SymbolTable(Operation *symbolTableOp)
assert(llvm::hasSingleElement(symbolTableOp->getRegion(0)) &&
"expected operation to have a single block");

Identifier symbolNameId = Identifier::get(SymbolTable::getSymbolAttrName(),
symbolTableOp->getContext());
for (auto &op : symbolTableOp->getRegion(0).front()) {
Optional<StringRef> name = getNameIfSymbol(&op);
Optional<StringRef> name = getNameIfSymbol(&op, symbolNameId);
if (!name)
continue;

Expand Down Expand Up @@ -269,8 +279,10 @@ Operation *SymbolTable::lookupSymbolIn(Operation *symbolTableOp,
assert(symbolTableOp->hasTrait<OpTrait::SymbolTable>());

// Look for a symbol with the given name.
Identifier symbolNameId = Identifier::get(SymbolTable::getSymbolAttrName(),
symbolTableOp->getContext());
for (auto &op : symbolTableOp->getRegion(0).front().without_terminator())
if (getNameIfSymbol(&op) == symbol)
if (getNameIfSymbol(&op, symbolNameId) == symbol)
return &op;
return nullptr;
}
Expand Down

0 comments on commit eda450b

Please sign in to comment.