[Mlir-commits] [mlir] Revert "[MLIR] Fix duplicated attribute nodes in MLIR bytecode deserialization (#151267) (PR #155214)
Christian Ulmann
llvmlistbot at llvm.org
Sun Aug 24 23:43:36 PDT 2025
https://github.com/Dinistro created https://github.com/llvm/llvm-project/pull/155214
This reverts commit c075fb8c37856365fb76d986ad3aefa2400b3240. This commit introduces a caching bug that causes undesired collisions.
>From 40109c31b3c097c0224fb72d2add774cf1ec7078 Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Mon, 25 Aug 2025 06:40:23 +0000
Subject: [PATCH] Revert "[MLIR] Fix duplicated attribute nodes in MLIR
bytecode deserialization (#151267)"
This reverts commit c075fb8c37856365fb76d986ad3aefa2400b3240. This
commit introduces a caching bug that causes undesired collisions.
---
mlir/include/mlir/AsmParser/AsmParser.h | 3 +--
mlir/lib/AsmParser/DialectSymbolParser.cpp | 24 ++-------------------
mlir/lib/AsmParser/ParserState.h | 3 ---
mlir/lib/Bytecode/Reader/BytecodeReader.cpp | 6 +-----
mlir/test/IR/recursive-distinct-attr.mlir | 13 -----------
5 files changed, 4 insertions(+), 45 deletions(-)
delete mode 100644 mlir/test/IR/recursive-distinct-attr.mlir
diff --git a/mlir/include/mlir/AsmParser/AsmParser.h b/mlir/include/mlir/AsmParser/AsmParser.h
index f39b3bd853a2a..33daf7ca26f49 100644
--- a/mlir/include/mlir/AsmParser/AsmParser.h
+++ b/mlir/include/mlir/AsmParser/AsmParser.h
@@ -53,8 +53,7 @@ parseAsmSourceFile(const llvm::SourceMgr &sourceMgr, Block *block,
/// null terminated.
Attribute parseAttribute(llvm::StringRef attrStr, MLIRContext *context,
Type type = {}, size_t *numRead = nullptr,
- bool isKnownNullTerminated = false,
- llvm::StringMap<Attribute> *attributesCache = nullptr);
+ bool isKnownNullTerminated = false);
/// This parses a single MLIR type to an MLIR context if it was valid. If not,
/// an error diagnostic is emitted to the context.
diff --git a/mlir/lib/AsmParser/DialectSymbolParser.cpp b/mlir/lib/AsmParser/DialectSymbolParser.cpp
index 416d8eb5f40e0..8b14e71118c3a 100644
--- a/mlir/lib/AsmParser/DialectSymbolParser.cpp
+++ b/mlir/lib/AsmParser/DialectSymbolParser.cpp
@@ -245,15 +245,6 @@ static Symbol parseExtendedSymbol(Parser &p, AsmParserState *asmState,
return nullptr;
}
- if constexpr (std::is_same_v<Symbol, Attribute>) {
- auto &cache = p.getState().symbols.attributesCache;
- auto cacheIt = cache.find(symbolData);
- // Skip cached attribute if it has type.
- if (cacheIt != cache.end() && !p.getToken().is(Token::colon))
- return cacheIt->second;
-
- return cache[symbolData] = createSymbol(dialectName, symbolData, loc);
- }
return createSymbol(dialectName, symbolData, loc);
}
@@ -346,7 +337,6 @@ Type Parser::parseExtendedType() {
template <typename T, typename ParserFn>
static T parseSymbol(StringRef inputStr, MLIRContext *context,
size_t *numReadOut, bool isKnownNullTerminated,
- llvm::StringMap<Attribute> *attributesCache,
ParserFn &&parserFn) {
// Set the buffer name to the string being parsed, so that it appears in error
// diagnostics.
@@ -358,9 +348,6 @@ static T parseSymbol(StringRef inputStr, MLIRContext *context,
SourceMgr sourceMgr;
sourceMgr.AddNewSourceBuffer(std::move(memBuffer), SMLoc());
SymbolState aliasState;
- if (attributesCache)
- aliasState.attributesCache = *attributesCache;
-
ParserConfig config(context);
ParserState state(sourceMgr, config, aliasState, /*asmState=*/nullptr,
/*codeCompleteContext=*/nullptr);
@@ -371,11 +358,6 @@ static T parseSymbol(StringRef inputStr, MLIRContext *context,
if (!symbol)
return T();
- if constexpr (std::is_same_v<T, Attribute>) {
- if (attributesCache)
- *attributesCache = state.symbols.attributesCache;
- }
-
// Provide the number of bytes that were read.
Token endTok = parser.getToken();
size_t numRead =
@@ -392,15 +374,13 @@ static T parseSymbol(StringRef inputStr, MLIRContext *context,
Attribute mlir::parseAttribute(StringRef attrStr, MLIRContext *context,
Type type, size_t *numRead,
- bool isKnownNullTerminated,
- llvm::StringMap<Attribute> *attributesCache) {
+ bool isKnownNullTerminated) {
return parseSymbol<Attribute>(
- attrStr, context, numRead, isKnownNullTerminated, attributesCache,
+ attrStr, context, numRead, isKnownNullTerminated,
[type](Parser &parser) { return parser.parseAttribute(type); });
}
Type mlir::parseType(StringRef typeStr, MLIRContext *context, size_t *numRead,
bool isKnownNullTerminated) {
return parseSymbol<Type>(typeStr, context, numRead, isKnownNullTerminated,
- /*attributesCache=*/nullptr,
[](Parser &parser) { return parser.parseType(); });
}
diff --git a/mlir/lib/AsmParser/ParserState.h b/mlir/lib/AsmParser/ParserState.h
index aa53032107cbf..159058a18fa4e 100644
--- a/mlir/lib/AsmParser/ParserState.h
+++ b/mlir/lib/AsmParser/ParserState.h
@@ -40,9 +40,6 @@ struct SymbolState {
/// A map from unique integer identifier to DistinctAttr.
DenseMap<uint64_t, DistinctAttr> distinctAttributes;
-
- /// A map from unique string identifier to Attribute.
- llvm::StringMap<Attribute> attributesCache;
};
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Bytecode/Reader/BytecodeReader.cpp b/mlir/lib/Bytecode/Reader/BytecodeReader.cpp
index 0f97443433774..44458d010c6c8 100644
--- a/mlir/lib/Bytecode/Reader/BytecodeReader.cpp
+++ b/mlir/lib/Bytecode/Reader/BytecodeReader.cpp
@@ -895,10 +895,6 @@ class AttrTypeReader {
SmallVector<AttrEntry> attributes;
SmallVector<TypeEntry> types;
- /// The map of cached attributes, used to avoid re-parsing the same
- /// attribute multiple times.
- llvm::StringMap<Attribute> attributesCache;
-
/// A location used for error emission.
Location fileLoc;
@@ -1239,7 +1235,7 @@ LogicalResult AttrTypeReader::parseAsmEntry(T &result, EncodingReader &reader,
::parseType(asmStr, context, &numRead, /*isKnownNullTerminated=*/true);
else
result = ::parseAttribute(asmStr, context, Type(), &numRead,
- /*isKnownNullTerminated=*/true, &attributesCache);
+ /*isKnownNullTerminated=*/true);
if (!result)
return failure();
diff --git a/mlir/test/IR/recursive-distinct-attr.mlir b/mlir/test/IR/recursive-distinct-attr.mlir
deleted file mode 100644
index 5afb5c59e0fcf..0000000000000
--- a/mlir/test/IR/recursive-distinct-attr.mlir
+++ /dev/null
@@ -1,13 +0,0 @@
-// RUN: mlir-opt -emit-bytecode %s | mlir-opt --mlir-print-debuginfo | FileCheck %s
-
-// Verify that the distinct attribute which is used transitively
-// through two aliases does not end up duplicated when round-tripped
-// through bytecode.
-
-// CHECK: distinct[0]
-// CHECK-NOT: distinct[1]
-#attr_ugly = #test<attr_ugly begin distinct[0]<> end>
-#attr_ugly1 = #test<attr_ugly begin #attr_ugly end>
-
-module attributes {test.alias = #attr_ugly, test.alias1 = #attr_ugly1} {
-}
\ No newline at end of file
More information about the Mlir-commits
mailing list