Skip to content

Commit 536344c

Browse files
author
git apple-llvm automerger
committed
Merge commit '38be53aa04de' from llvm.org/main into next
2 parents 2a094cd + 38be53a commit 536344c

File tree

5 files changed

+69
-16
lines changed

5 files changed

+69
-16
lines changed

mlir/lib/IR/AttributeDetail.h

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,9 @@
1919
#include "mlir/IR/BuiltinTypes.h"
2020
#include "mlir/IR/IntegerSet.h"
2121
#include "mlir/IR/MLIRContext.h"
22-
#include "mlir/Support/StorageUniquer.h"
23-
#include "mlir/Support/ThreadLocalCache.h"
2422
#include "llvm/ADT/APFloat.h"
25-
#include "llvm/ADT/PointerIntPair.h"
26-
#include "llvm/Support/TrailingObjects.h"
23+
#include "llvm/Support/Allocator.h"
24+
#include <mutex>
2725

2826
namespace mlir {
2927
namespace detail {
@@ -396,27 +394,30 @@ class DistinctAttributeUniquer {
396394
Attribute referencedAttr);
397395
};
398396

399-
/// An allocator for distinct attribute storage instances. It uses thread local
400-
/// bump pointer allocators stored in a thread local cache to ensure the storage
401-
/// is freed after the destruction of the distinct attribute allocator.
402-
class DistinctAttributeAllocator {
397+
/// An allocator for distinct attribute storage instances. Uses a synchronized
398+
/// BumpPtrAllocator to ensure thread-safety. The allocated storage is deleted
399+
/// when the DistinctAttributeAllocator is destroyed.
400+
class DistinctAttributeAllocator final {
403401
public:
404402
DistinctAttributeAllocator() = default;
405-
406403
DistinctAttributeAllocator(DistinctAttributeAllocator &&) = delete;
407404
DistinctAttributeAllocator(const DistinctAttributeAllocator &) = delete;
408405
DistinctAttributeAllocator &
409406
operator=(const DistinctAttributeAllocator &) = delete;
410407

411-
/// Allocates a distinct attribute storage using a thread local bump pointer
412-
/// allocator to enable synchronization free parallel allocations.
413408
DistinctAttrStorage *allocate(Attribute referencedAttr) {
414-
return new (allocatorCache.get().Allocate<DistinctAttrStorage>())
409+
std::scoped_lock<std::mutex> guard(allocatorMutex);
410+
return new (allocator.Allocate<DistinctAttrStorage>())
415411
DistinctAttrStorage(referencedAttr);
416-
}
412+
};
417413

418414
private:
419-
ThreadLocalCache<llvm::BumpPtrAllocator> allocatorCache;
415+
/// Used to allocate distict attribute storages. The managed memory is freed
416+
/// automatically when the allocator instance is destroyed.
417+
llvm::BumpPtrAllocator allocator;
418+
419+
/// Used to lock access to the allocator.
420+
std::mutex allocatorMutex;
420421
};
421422
} // namespace detail
422423
} // namespace mlir

mlir/lib/IR/MLIRContext.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "llvm/Support/CommandLine.h"
3232
#include "llvm/Support/Compiler.h"
3333
#include "llvm/Support/Debug.h"
34+
#include "llvm/Support/ManagedStatic.h"
3435
#include "llvm/Support/Mutex.h"
3536
#include "llvm/Support/RWMutex.h"
3637
#include "llvm/Support/ThreadPool.h"

mlir/lib/Pass/PassCrashRecovery.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -411,14 +411,19 @@ struct FileReproducerStream : public mlir::ReproducerStream {
411411

412412
LogicalResult PassManager::runWithCrashRecovery(Operation *op,
413413
AnalysisManager am) {
414+
const bool threadingEnabled = getContext()->isMultithreadingEnabled();
414415
crashReproGenerator->initialize(getPasses(), op, verifyPasses);
415416

416417
// Safely invoke the passes within a recovery context.
417418
LogicalResult passManagerResult = failure();
418419
llvm::CrashRecoveryContext recoveryContext;
419-
recoveryContext.RunSafelyOnThread(
420-
[&] { passManagerResult = runPasses(op, am); });
420+
const auto runPassesFn = [&] { passManagerResult = runPasses(op, am); };
421+
if (threadingEnabled)
422+
recoveryContext.RunSafelyOnThread(runPassesFn);
423+
else
424+
recoveryContext.RunSafely(runPassesFn);
421425
crashReproGenerator->finalize(op, passManagerResult);
426+
422427
return passManagerResult;
423428
}
424429

mlir/unittests/IR/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ add_mlir_unittest(MLIRIRTests
66
AttrTypeReplacerTest.cpp
77
Diagnostic.cpp
88
DialectTest.cpp
9+
DistinctAttributeAllocatorTest.cpp
910
InterfaceTest.cpp
1011
IRMapping.cpp
1112
InterfaceAttachmentTest.cpp
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
//=== DistinctAttributeAllocatorTest.cpp - DistinctAttr storage alloc test ===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "gtest/gtest.h"
10+
11+
#include "mlir/IR/Builders.h"
12+
#include "mlir/IR/BuiltinAttributes.h"
13+
#include "mlir/IR/MLIRContext.h"
14+
#include "llvm/Support/CrashRecoveryContext.h"
15+
#include <thread>
16+
17+
using namespace mlir;
18+
19+
//
20+
// Test that a DistinctAttr that is created on a separate thread does
21+
// not have its storage deleted when the thread joins.
22+
//
23+
TEST(DistinctAttributeAllocatorTest, TestAttributeWellFormedAfterThreadJoin) {
24+
MLIRContext ctx;
25+
OpBuilder builder(&ctx);
26+
DistinctAttr attr;
27+
28+
std::thread t([&ctx, &attr]() {
29+
attr = DistinctAttr::create(UnitAttr::get(&ctx));
30+
ASSERT_TRUE(attr);
31+
});
32+
t.join();
33+
34+
// If the attribute storage got deleted after the thread joins (which we don't
35+
// want) then trying to access it triggers an assert in Debug mode, and a
36+
// crash otherwise. Run this in a CrashRecoveryContext to avoid bringing down
37+
// the whole test suite if this test fails. Additionally, MSAN and/or TSAN
38+
// should raise failures here if the attribute storage was deleted.
39+
llvm::CrashRecoveryContext crc;
40+
EXPECT_TRUE(crc.RunSafely([attr]() { (void)attr.getAbstractAttribute(); }));
41+
EXPECT_TRUE(
42+
crc.RunSafely([attr]() { (void)*cast<Attribute>(attr).getImpl(); }));
43+
44+
ASSERT_TRUE(attr);
45+
}

0 commit comments

Comments
 (0)