Skip to content

[mlir] Allow accessing DialectResourceBlobManager::blobMap #142352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion mlir/include/mlir/IR/DialectResourceBlobManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,14 @@ class DialectResourceBlobManager {
return HandleT(&entry, dialect);
}

/// Provide access to all the registered blobs via a callable. During access
/// the blob map is guaranteed to remain unchanged.
void getBlobMap(llvm::function_ref<void(const llvm::StringMap<BlobEntry> &)>
accessor) const;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: the API itself is a bit weird since we need to keep access protected by a mutex. without a larger redesign of this, it seems to be the only option?

private:
/// A mutex to protect access to the blob map.
llvm::sys::SmartRWMutex<true> blobMapLock;
mutable llvm::sys::SmartRWMutex<true> blobMapLock;

/// The internal map of tracked blobs. StringMap stores entries in distinct
/// allocations, so we can freely take references to the data without fear of
Expand Down
8 changes: 8 additions & 0 deletions mlir/lib/IR/DialectResourceBlobManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,11 @@ auto DialectResourceBlobManager::insert(StringRef name,
nameStorage.resize(name.size() + 1);
} while (true);
}

void DialectResourceBlobManager::getBlobMap(
llvm::function_ref<void(const llvm::StringMap<BlobEntry> &)> accessor)
const {
llvm::sys::SmartScopedReader<true> reader(blobMapLock);

accessor(blobMap);
}
74 changes: 74 additions & 0 deletions mlir/unittests/IR/BlobManagerTest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
//===- mlir/unittest/IR/BlobManagerTest.cpp - Blob management unit tests --===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "../../test/lib/Dialect/Test/TestDialect.h"
#include "mlir/IR/DialectResourceBlobManager.h"
#include "mlir/Parser/Parser.h"

#include "gtest/gtest.h"

using namespace mlir;

namespace {

StringLiteral moduleStr = R"mlir(
"test.use1"() {attr = dense_resource<blob1> : tensor<1xi64> } : () -> ()

{-#
dialect_resources: {
builtin: {
blob1: "0x08000000ABCDABCDABCDABCE"
}
}
#-}
)mlir";

TEST(DialectResourceBlobManagerTest, Lookup) {
MLIRContext context;
context.loadDialect<test::TestDialect>();

OwningOpRef<ModuleOp> m = parseSourceString<ModuleOp>(moduleStr, &context);
ASSERT_TRUE(m);

const auto &dialectManager =
mlir::DenseResourceElementsHandle::getManagerInterface(&context);
ASSERT_NE(dialectManager.getBlobManager().lookup("blob1"), nullptr);
}

TEST(DialectResourceBlobManagerTest, GetBlobMap) {
MLIRContext context;
context.loadDialect<test::TestDialect>();

OwningOpRef<ModuleOp> m = parseSourceString<ModuleOp>(moduleStr, &context);
ASSERT_TRUE(m);

Block *block = m->getBody();
auto &op = block->getOperations().front();
auto resourceAttr = op.getAttrOfType<DenseResourceElementsAttr>("attr");
ASSERT_NE(resourceAttr, nullptr);

const auto &dialectManager =
resourceAttr.getRawHandle().getManagerInterface(&context);

bool blobsArePresent = false;
dialectManager.getBlobManager().getBlobMap(
[&](const llvm::StringMap<DialectResourceBlobManager::BlobEntry>
&blobMap) { blobsArePresent = blobMap.contains("blob1"); });
ASSERT_TRUE(blobsArePresent);

// remove operations that use resources - resources must still be accessible
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really a guarantee of the system? Aren't we allowed to somehow have a reference-count mechanism?

Copy link
Contributor Author

@andrey-golubev andrey-golubev Jun 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

honestly, I'm not sure. but seems to be intended behavior? from what I understand:

  • resources are "raw" (at best unique when with deleter) pointers
  • blob - underlying data behind resources - are stored into a dialect manager (lifetime == MLIR context's lifetime)
  • resources are used by operations but as the resources themselves are attributes (mlir::DenseResourceElementsAttr), they are not "owned" by operations

thus, what we effectively get is: dense_resource<> is never really deleted unless one explicitly does something at the dialect (aka blob) manager level. which is fine by me, to be honest.

side-note: i was experimenting with custom deletion on top of dialect resources before (e.g. via a manual ref-count) but I guess ideal solution is to switch to properties and have operation-driven auto-ref-count semantics (might require some resource redesign as well though).

block->clear();

blobsArePresent = false;
dialectManager.getBlobManager().getBlobMap(
[&](const llvm::StringMap<DialectResourceBlobManager::BlobEntry>
&blobMap) { blobsArePresent = blobMap.contains("blob1"); });
ASSERT_TRUE(blobsArePresent);
}

} // end anonymous namespace
1 change: 1 addition & 0 deletions mlir/unittests/IR/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ add_mlir_unittest(MLIRIRTests
TypeAttrNamesTest.cpp
OpPropertiesTest.cpp
ValueTest.cpp
BlobManagerTest.cpp

DEPENDS
MLIRTestInterfaceIncGen
Expand Down
Loading