Revert "[MLIR] Fix duplicated attribute nodes in MLIR bytecode deserialization (#151267) (#155214)

Dinistro · web-flow · commit 58953b8ef56a · 2025-08-25T13:46:13.000+02:00
This reverts commit c075fb8. This commit introduces a caching bug that causes undesired collisions.
diff --git 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
@@ -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
@@ -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
@@ -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
