Skip to content

Commit 58953b8

Browse files
authored
Revert "[MLIR] Fix duplicated attribute nodes in MLIR bytecode deserialization (#151267) (#155214)
This reverts commit c075fb8. This commit introduces a caching bug that causes undesired collisions.
1 parent c6bcc74 commit 58953b8

File tree

5 files changed

+4
-45
lines changed

5 files changed

+4
-45
lines changed

mlir/include/mlir/AsmParser/AsmParser.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,7 @@ parseAsmSourceFile(const llvm::SourceMgr &sourceMgr, Block *block,
5353
/// null terminated.
5454
Attribute parseAttribute(llvm::StringRef attrStr, MLIRContext *context,
5555
Type type = {}, size_t *numRead = nullptr,
56-
bool isKnownNullTerminated = false,
57-
llvm::StringMap<Attribute> *attributesCache = nullptr);
56+
bool isKnownNullTerminated = false);
5857

5958
/// This parses a single MLIR type to an MLIR context if it was valid. If not,
6059
/// an error diagnostic is emitted to the context.

mlir/lib/AsmParser/DialectSymbolParser.cpp

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -245,15 +245,6 @@ static Symbol parseExtendedSymbol(Parser &p, AsmParserState *asmState,
245245
return nullptr;
246246
}
247247

248-
if constexpr (std::is_same_v<Symbol, Attribute>) {
249-
auto &cache = p.getState().symbols.attributesCache;
250-
auto cacheIt = cache.find(symbolData);
251-
// Skip cached attribute if it has type.
252-
if (cacheIt != cache.end() && !p.getToken().is(Token::colon))
253-
return cacheIt->second;
254-
255-
return cache[symbolData] = createSymbol(dialectName, symbolData, loc);
256-
}
257248
return createSymbol(dialectName, symbolData, loc);
258249
}
259250

@@ -346,7 +337,6 @@ Type Parser::parseExtendedType() {
346337
template <typename T, typename ParserFn>
347338
static T parseSymbol(StringRef inputStr, MLIRContext *context,
348339
size_t *numReadOut, bool isKnownNullTerminated,
349-
llvm::StringMap<Attribute> *attributesCache,
350340
ParserFn &&parserFn) {
351341
// Set the buffer name to the string being parsed, so that it appears in error
352342
// diagnostics.
@@ -358,9 +348,6 @@ static T parseSymbol(StringRef inputStr, MLIRContext *context,
358348
SourceMgr sourceMgr;
359349
sourceMgr.AddNewSourceBuffer(std::move(memBuffer), SMLoc());
360350
SymbolState aliasState;
361-
if (attributesCache)
362-
aliasState.attributesCache = *attributesCache;
363-
364351
ParserConfig config(context);
365352
ParserState state(sourceMgr, config, aliasState, /*asmState=*/nullptr,
366353
/*codeCompleteContext=*/nullptr);
@@ -371,11 +358,6 @@ static T parseSymbol(StringRef inputStr, MLIRContext *context,
371358
if (!symbol)
372359
return T();
373360

374-
if constexpr (std::is_same_v<T, Attribute>) {
375-
if (attributesCache)
376-
*attributesCache = state.symbols.attributesCache;
377-
}
378-
379361
// Provide the number of bytes that were read.
380362
Token endTok = parser.getToken();
381363
size_t numRead =
@@ -392,15 +374,13 @@ static T parseSymbol(StringRef inputStr, MLIRContext *context,
392374

393375
Attribute mlir::parseAttribute(StringRef attrStr, MLIRContext *context,
394376
Type type, size_t *numRead,
395-
bool isKnownNullTerminated,
396-
llvm::StringMap<Attribute> *attributesCache) {
377+
bool isKnownNullTerminated) {
397378
return parseSymbol<Attribute>(
398-
attrStr, context, numRead, isKnownNullTerminated, attributesCache,
379+
attrStr, context, numRead, isKnownNullTerminated,
399380
[type](Parser &parser) { return parser.parseAttribute(type); });
400381
}
401382
Type mlir::parseType(StringRef typeStr, MLIRContext *context, size_t *numRead,
402383
bool isKnownNullTerminated) {
403384
return parseSymbol<Type>(typeStr, context, numRead, isKnownNullTerminated,
404-
/*attributesCache=*/nullptr,
405385
[](Parser &parser) { return parser.parseType(); });
406386
}

mlir/lib/AsmParser/ParserState.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,6 @@ struct SymbolState {
4040

4141
/// A map from unique integer identifier to DistinctAttr.
4242
DenseMap<uint64_t, DistinctAttr> distinctAttributes;
43-
44-
/// A map from unique string identifier to Attribute.
45-
llvm::StringMap<Attribute> attributesCache;
4643
};
4744

4845
//===----------------------------------------------------------------------===//

mlir/lib/Bytecode/Reader/BytecodeReader.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -895,10 +895,6 @@ class AttrTypeReader {
895895
SmallVector<AttrEntry> attributes;
896896
SmallVector<TypeEntry> types;
897897

898-
/// The map of cached attributes, used to avoid re-parsing the same
899-
/// attribute multiple times.
900-
llvm::StringMap<Attribute> attributesCache;
901-
902898
/// A location used for error emission.
903899
Location fileLoc;
904900

@@ -1239,7 +1235,7 @@ LogicalResult AttrTypeReader::parseAsmEntry(T &result, EncodingReader &reader,
12391235
::parseType(asmStr, context, &numRead, /*isKnownNullTerminated=*/true);
12401236
else
12411237
result = ::parseAttribute(asmStr, context, Type(), &numRead,
1242-
/*isKnownNullTerminated=*/true, &attributesCache);
1238+
/*isKnownNullTerminated=*/true);
12431239
if (!result)
12441240
return failure();
12451241

mlir/test/IR/recursive-distinct-attr.mlir

Lines changed: 0 additions & 13 deletions
This file was deleted.

0 commit comments

Comments
 (0)