[llvm-branch-commits] [mlir] [MLIR][Remark] Make remark reporting thread-safe and order final remarks by source position (PR #227360)
Henrich Lauko via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Wed Sep 30 04:31:49 PDT 2026
https://github.com/xlauko updated https://github.com/llvm/llvm-project/pull/227360
>From 327d069f02119be445b34dc04c73a7ccee841e2d Mon Sep 17 00:00:00 2001
From: Henrich Lauko <hlauko at nvidia.com>
Date: Tue, 29 Sep 2026 15:51:08 +0000
Subject: [PATCH] [MLIR][Remark] Make remark reporting thread-safe and order
final remarks by source position
Passes nested under the multithreaded pass manager report into the same
RemarkEngine from worker threads, but nothing in the engine or the
policies was synchronized. RemarkEmittingPolicyFinal inserted into its
map from several threads at once, and under RemarkEmittingPolicyAll the
streamer, including the LLVM remark serializer, was called from several
threads at once. ThreadSanitizer reports both races in the new unit tests.
RemarkEngine now holds a lock around every call into the policy. It is a
recursive llvm::sys::SmartMutex, the same type as the DiagnosticEngine's
lock. report() takes it, and so does a new finalizePolicy(), which the
engine destructor and mlir-opt now use instead of calling finalize() on
the policy directly. For the All policy the streamer and the diagnostic
printer also run under the lock, so custom policies and streamers need no
lock of their own.
With the race fixed, the final policy's creation order is still not
deterministic: which thread reports first depends on scheduling.
finalize() now sorts root remarks by source position. The key is the file
positions nested in the location, in walk order, so the first is the one
the diagnostic printer shows and the rest separate one callee inlined at
different call sites. Remark name, category and kind follow, and the
printed location breaks any remaining tie. Remarks without a file position
come last. Identity and deduplication do not change, and linked remarks
still follow their parent.
remark-final.mlir swaps two CHECK lines: its remarks share a location and
a name, so the category now decides their order. New unit tests cover
source order, independence from report order, a linked remark at an
earlier position, and concurrent reporting under both policies. The new
lit test remark-final-parallel.mlir runs test-remark on four functions in
parallel and checks the same output with and without threading.
Assisted-by: Claude Code (Claude Opus 5.5)
---
mlir/docs/Remarks.md | 28 ++-
mlir/include/mlir/IR/Remarks.h | 33 +++-
mlir/lib/IR/Remarks.cpp | 92 ++++++++--
mlir/lib/Tools/mlir-opt/MlirOptMain.cpp | 2 +-
mlir/test/Pass/remark-final-parallel.mlir | 51 ++++++
mlir/test/Pass/remark-final.mlir | 17 +-
mlir/unittests/IR/RemarkTest.cpp | 210 ++++++++++++++++++++--
7 files changed, 391 insertions(+), 42 deletions(-)
create mode 100644 mlir/test/Pass/remark-final-parallel.mlir
diff --git a/mlir/docs/Remarks.md b/mlir/docs/Remarks.md
index e747f7bc8f5958..125fffe5ad87b6 100644
--- a/mlir/docs/Remarks.md
+++ b/mlir/docs/Remarks.md
@@ -205,8 +205,12 @@ Emits **all** remarks unconditionally.
Stores remarks until `finalize()` is called and emits only the **final** remark
for each location. This is useful in multi-pass compilers where an early pass
may report a failure, but a later pass succeeds. `finalize()` drains the stored
-remarks and emits them in the order in which they were created. Calling it
-again emits only remarks reported since.
+remarks. Calling it again emits only remarks reported since.
+
+Remarks are emitted in source order (file, line, column), remarks without a
+file position last, and linked remarks right after the remark that references
+them. The order does not depend on the order in which remarks were reported,
+so it is the same whether or not passes run in parallel.
**Example:** Only the successful remark is emitted:
@@ -222,6 +226,26 @@ remark::passed(loc, opts) << "Loop unrolled successfully";
You can also implement custom policies by inheriting from the policy interface.
+### Thread safety
+
+Passes that run in parallel report into the same `RemarkEngine`. The engine
+takes a lock around each call into the policy, so `reportRemark` and
+`finalize`, and the streamer calls a policy makes from them, never run
+concurrently. Custom policies and streamers therefore need no lock of their
+own. They must not report remarks or wait for threads that report remarks,
+and diagnostic handlers must not report remarks either.
+
+To emit the remarks a final policy holds while other threads may still report,
+call `RemarkEngine::finalizePolicy()` rather than calling `finalize()` on
+`getRemarkEmittingPolicy()` directly. The engine destructor finalizes the
+policy as well.
+
+With several threads, some output still depends on scheduling. Under
+`RemarkEmittingPolicyAll`, the streamer receives remarks in the order in which
+threads reach the lock. `RemarkId` and `RelatedTo` values come from a shared
+counter. If two threads report the same identity, whichever reports last
+decides the content the final policy keeps.
+
***
## Querying Enabled Remarks
diff --git a/mlir/include/mlir/IR/Remarks.h b/mlir/include/mlir/IR/Remarks.h
index 0ecc63717b5aee..97eba18690a136 100644
--- a/mlir/include/mlir/IR/Remarks.h
+++ b/mlir/include/mlir/IR/Remarks.h
@@ -17,6 +17,7 @@
#include "llvm/IR/DiagnosticInfo.h"
#include "llvm/Remarks/Remark.h"
#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/Mutex.h"
#include "llvm/Support/Regex.h"
#include "mlir/IR/Diagnostics.h"
@@ -449,6 +450,10 @@ class InFlightRemark {
/// optimization remarks to the underlying remark streamer. The derived classes
/// should implement the `streamOptimizationRemark` method to provide the
/// actual streaming implementation.
+///
+/// The RemarkEngine calls `streamOptimizationRemark` under its lock, so an
+/// implementation does not need a lock of its own. It must not report remarks
+/// or wait for threads that report remarks.
class MLIRRemarkStreamerBase {
public:
virtual ~MLIRRemarkStreamerBase() = default;
@@ -468,6 +473,10 @@ using ReportFn = llvm::unique_function<void(const Remark &)>;
/// optimization remarks to the underlying remark streamer. The derived classes
/// should implement the `reportRemark` method to provide the actual emitting
/// implementation.
+///
+/// Through the RemarkEngine, `reportRemark` and `finalize` run under the
+/// engine's lock and are never entered concurrently, even when passes report
+/// remarks from several threads.
class RemarkEmittingPolicyBase {
protected:
ReportFn reportImpl;
@@ -514,6 +523,11 @@ class RemarkEngine {
bool printAsEmitRemarks = false;
/// Atomic counter for generating unique remark IDs.
std::atomic<uint64_t> nextRemarkId{1};
+ /// Serializes report() and finalizePolicy(). Passes running in parallel
+ /// report into the same engine, and neither the policies nor the streamers
+ /// are thread-safe. Recursive, like the DiagnosticEngine's lock, so a
+ /// callback that reports on the same thread does not deadlock.
+ llvm::sys::SmartMutex<true> mutex;
/// Emit a remark using the given maker function, which should return
/// a Remark instance. The remark will be emitted using the main
@@ -547,11 +561,17 @@ class RemarkEngine {
std::unique_ptr<RemarkEmittingPolicyBase> remarkEmittingPolicy,
std::string *errMsg);
- /// Get the remark emitting policy.
+ /// Get the remark emitting policy. Calling into the policy directly
+ /// bypasses the engine's lock; use finalizePolicy() to finalize it while
+ /// other threads may still report remarks.
RemarkEmittingPolicyBase *getRemarkEmittingPolicy() const {
return remarkEmittingPolicy.get();
}
+ /// Finalize the emitting policy under the engine's lock, e.g. to emit the
+ /// remarks a RemarkEmittingPolicyFinal holds once a pipeline has finished.
+ void finalizePolicy();
+
/// Generate a unique ID for a new remark.
RemarkId generateRemarkId() {
return RemarkId(nextRemarkId.fetch_add(1, std::memory_order_relaxed));
@@ -607,7 +627,8 @@ class RemarkEngine {
findRemarks(const RemarkOpts &opts,
std::optional<RemarkKind> kind = std::nullopt) const;
- /// Report a remark.
+ /// Report a remark. Thread-safe: reports from several threads are handed to
+ /// the policy one at a time.
void report(const Remark &&remark);
/// Report a successful remark, this will create an InFlightRemark
@@ -667,7 +688,7 @@ class RemarkEmittingPolicyAll : public detail::RemarkEmittingPolicyBase {
/// Policy that emits only the last remark reported for each identity, see
/// DenseMapInfo<Remark>. Remarks are stored until finalize(), which emits them
-/// in creation order, so the output does not depend on hash order.
+/// in source order, see there.
class RemarkEmittingPolicyFinal : public detail::RemarkEmittingPolicyBase {
private:
/// Remarks reported since the last finalize().
@@ -681,7 +702,11 @@ class RemarkEmittingPolicyFinal : public detail::RemarkEmittingPolicyBase {
postponedRemarks.insert(remark);
}
- /// Emits and drains all stored remarks. Related remarks are printed right
+ /// Emits and drains all stored remarks. Root remarks come out sorted by the
+ /// file positions nested in their location (remarks with none last), then
+ /// by remark name, category and kind, so the output does not depend on the
+ /// order in which remarks were reported or on how parallel passes were
+ /// scheduled. Related remarks are printed right
/// after the remark that references them; a link only resolves when both
/// remarks are in the same call. A later call emits only remarks reported
/// since this one.
diff --git a/mlir/lib/IR/Remarks.cpp b/mlir/lib/IR/Remarks.cpp
index 9d0f3c56c2e0c0..9d9a27edb52165 100644
--- a/mlir/lib/IR/Remarks.cpp
+++ b/mlir/lib/IR/Remarks.cpp
@@ -12,6 +12,7 @@
#include "mlir/IR/Diagnostics.h"
#include "mlir/IR/Value.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
@@ -262,13 +263,21 @@ void RemarkEngine::reportImpl(const Remark &remark) {
}
void RemarkEngine::report(const Remark &&remark) {
- if (remarkEmittingPolicy)
- remarkEmittingPolicy->reportRemark(remark);
+ if (!remarkEmittingPolicy)
+ return;
+ llvm::sys::SmartScopedLock<true> lock(mutex);
+ remarkEmittingPolicy->reportRemark(remark);
+}
+
+void RemarkEngine::finalizePolicy() {
+ if (!remarkEmittingPolicy)
+ return;
+ llvm::sys::SmartScopedLock<true> lock(mutex);
+ remarkEmittingPolicy->finalize();
}
RemarkEngine::~RemarkEngine() {
- if (remarkEmittingPolicy)
- remarkEmittingPolicy->finalize();
+ finalizePolicy();
if (remarkStreamer)
remarkStreamer->finalize();
@@ -365,13 +374,62 @@ namespace mlir::remark {
RemarkEmittingPolicyAll::RemarkEmittingPolicyAll() = default;
RemarkEmittingPolicyFinal::RemarkEmittingPolicyFinal() = default;
+namespace {
+/// Where RemarkEmittingPolicyFinal::finalize() places a root remark. The
+/// fields depend only on the remark's identity, never on its ID or arguments,
+/// so the order does not depend on which report of an identity was kept or on
+/// the order in which threads reported.
+struct FinalOrderKey {
+ /// Every file position nested in the location, in pre-order walk order. The
+ /// first is the one the diagnostic printer shows; the others tell apart,
+ /// for example, one callee inlined at two call sites.
+ SmallVector<std::tuple<StringRef, unsigned, unsigned>, 2> positions;
+ StringRef remarkName;
+ StringRef categoryName;
+ RemarkKind kind;
+ Location loc;
+
+ explicit FinalOrderKey(const detail::Remark &remark)
+ : remarkName(remark.getRemarkName()),
+ categoryName(remark.getCombinedCategoryName()),
+ kind(remark.getRemarkKind()), loc(remark.getLocation()) {
+ loc->walk([&](Location nested) {
+ if (auto flc = dyn_cast<FileLineColLoc>(nested))
+ positions.emplace_back(flc.getFilename().getValue(), flc.getLine(),
+ flc.getColumn());
+ return WalkResult::advance();
+ });
+ }
+
+ bool operator<(const FinalOrderKey &other) const {
+ // Remarks without any file position go last.
+ if (positions.empty() != other.positions.empty())
+ return other.positions.empty();
+ auto fields = std::tie(positions, remarkName, categoryName, kind);
+ auto otherFields = std::tie(other.positions, other.remarkName,
+ other.categoryName, other.kind);
+ if (fields != otherFields)
+ return fields < otherFields;
+ // Two different locations with the same file positions, e.g. two NameLocs
+ // around one position: fall back to their printed form.
+ auto printed = [](Location loc) {
+ std::string text;
+ llvm::raw_string_ostream os(text);
+ os << loc;
+ return text;
+ };
+ return printed(loc) < printed(other.loc);
+ }
+};
+} // namespace
+
void RemarkEmittingPolicyFinal::finalize() {
assert(reportImpl && "reportImpl is not set");
// Take the pending remarks so that a second finalize(), e.g. from the engine
// destructor after an explicit call, does not emit them again. IDs are
- // assigned in creation order; sorting by them keeps the output independent
- // of the set's hash layout.
+ // assigned in creation order; the source-position sort below falls back to
+ // it.
std::vector<detail::Remark> remarks(postponedRemarks.begin(),
postponedRemarks.end());
postponedRemarks.clear();
@@ -383,25 +441,31 @@ void RemarkEmittingPolicyFinal::finalize() {
llvm::DenseMap<uint64_t, const detail::Remark *> idMap;
llvm::DenseSet<uint64_t> childIds; // IDs referenced as children
- for (const auto &remark : remarks) {
+ for (const detail::Remark &remark : remarks) {
if (remark.getId())
idMap[remark.getId().getValue()] = &remark;
for (auto relId : remark.getRelatedRemarkIds())
childIds.insert(relId.getValue());
}
- // Emit remarks with related remarks grouped after their parents.
- // Parent remarks are emitted first, followed by their related (child)
- // remarks. Child-only remarks are skipped at the top level to avoid
- // duplication.
- for (const auto &remark : remarks) {
+ // Sort the root remarks, those not printed under a parent, by source
+ // position. Sort a side vector, since idMap points into `remarks`. The
+ // stable sort falls back to creation order only for two different locations
+ // that print the same.
+ SmallVector<std::pair<FinalOrderKey, const detail::Remark *>> roots;
+ for (const detail::Remark &remark : remarks) {
if (remark.getId() && childIds.contains(remark.getId().getValue()))
continue; // will be printed grouped under its parent
+ roots.emplace_back(FinalOrderKey(remark), &remark);
+ }
+ llvm::stable_sort(roots, llvm::less_first());
- reportImpl(remark);
+ // Emit remarks with related remarks grouped after their parents.
+ for (const detail::Remark *remark : llvm::make_second_range(roots)) {
+ reportImpl(*remark);
// Emit related remarks immediately after the parent.
- for (auto relId : remark.getRelatedRemarkIds()) {
+ for (auto relId : remark->getRelatedRemarkIds()) {
if (const auto *related = idMap.lookup(relId.getValue()))
reportImpl(*related);
}
diff --git a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
index 8b49258e135d28..5f0640209a48b4 100644
--- a/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
+++ b/mlir/lib/Tools/mlir-opt/MlirOptMain.cpp
@@ -633,7 +633,7 @@ performActions(raw_ostream &os,
// This is required if the remark policy is final. Otherwise, the remarks are
// not emitted.
if (remark::detail::RemarkEngine *engine = ctx.getRemarkEngine())
- engine->getRemarkEmittingPolicy()->finalize();
+ engine->finalizePolicy();
return success();
}
diff --git a/mlir/test/Pass/remark-final-parallel.mlir b/mlir/test/Pass/remark-final-parallel.mlir
new file mode 100644
index 00000000000000..2507070ad31bbf
--- /dev/null
+++ b/mlir/test/Pass/remark-final-parallel.mlir
@@ -0,0 +1,51 @@
+// RUN: mlir-opt %s --pass-pipeline='builtin.module(func.func(test-remark))' --remarks-filter-passed=category-1-passed --remark-policy=final -o /dev/null 2>&1 | FileCheck %s --implicit-check-not="remark:"
+// RUN: mlir-opt %s --pass-pipeline='builtin.module(func.func(test-remark))' --remarks-filter-passed=category-1-passed --remark-policy=final --mlir-disable-threading -o /dev/null 2>&1 | FileCheck %s --implicit-check-not="remark:"
+// RUN: mlir-opt %s --pass-pipeline='builtin.module(func.func(test-remark))' --remarks-filter-passed=category-1-passed --remark-policy=final --remark-format=yaml --remarks-output-file=%t.yaml -o /dev/null
+// RUN: FileCheck --check-prefix=CHECK-YAML %s --implicit-check-not="--- !" < %t.yaml
+
+// test-remark runs on each function in parallel and reports one passed remark
+// per operation. The final policy prints them sorted by source position, which
+// differs from both the IR order and any order the threads can report in, and
+// is the same with and without threading. Remarks without a file position come
+// last.
+
+func.func @c() {
+ return loc("b.c":1:1)
+} loc("a.c":30:1)
+func.func @a() {
+ return loc("a.c":11:1)
+} loc("a.c":10:1)
+func.func @b() {
+ return loc("n"("a.c":20:1))
+} loc(fused["a.c":10:5, "x"])
+func.func @d() {
+ return loc(unknown)
+} loc("a.c":25:1)
+
+// CHECK: a.c:10:1: remark: [Passed]
+// CHECK-NEXT: a.c:10:5: remark: [Passed]
+// CHECK-NEXT: a.c:11:1: remark: [Passed]
+// CHECK-NEXT: a.c:20:1: remark: [Passed]
+// CHECK-NEXT: a.c:25:1: remark: [Passed]
+// CHECK-NEXT: a.c:30:1: remark: [Passed]
+// CHECK-NEXT: b.c:1:1: remark: [Passed]
+// CHECK-NEXT: <unknown>:0: remark: [Passed]
+
+// The YAML serializer only resolves a plain file location, so the fused and
+// named ones are written as "<unknown file>", but the order is the same.
+// CHECK-YAML: --- !Passed
+// CHECK-YAML: DebugLoc: { File: a.c, Line: 10, Column: 1 }
+// CHECK-YAML: --- !Passed
+// CHECK-YAML: DebugLoc: { File: '<unknown file>', Line: 0, Column: 0 }
+// CHECK-YAML: --- !Passed
+// CHECK-YAML: DebugLoc: { File: a.c, Line: 11, Column: 1 }
+// CHECK-YAML: --- !Passed
+// CHECK-YAML: DebugLoc: { File: '<unknown file>', Line: 0, Column: 0 }
+// CHECK-YAML: --- !Passed
+// CHECK-YAML: DebugLoc: { File: a.c, Line: 25, Column: 1 }
+// CHECK-YAML: --- !Passed
+// CHECK-YAML: DebugLoc: { File: a.c, Line: 30, Column: 1 }
+// CHECK-YAML: --- !Passed
+// CHECK-YAML: DebugLoc: { File: b.c, Line: 1, Column: 1 }
+// CHECK-YAML: --- !Passed
+// CHECK-YAML: DebugLoc: { File: '<unknown file>', Line: 0, Column: 0 }
diff --git a/mlir/test/Pass/remark-final.mlir b/mlir/test/Pass/remark-final.mlir
index d406de295c5513..17fef7275c889c 100644
--- a/mlir/test/Pass/remark-final.mlir
+++ b/mlir/test/Pass/remark-final.mlir
@@ -5,24 +5,25 @@ module @foo {
"test.op"() : () -> ()
}
-// Remarks come out in creation order. The two passed remarks in
-// "category-1-passed" share an identity, so only the second survives. mlir-opt
-// calls finalize() explicitly and the engine destructor calls it again. The
-// second call must not emit the remarks a second time. --implicit-check-not
-// pins the number of "remark:" lines and of YAML records to five.
+// The two passed remarks in "category-1-passed" share an identity, so only the
+// second survives. All remarks share a location and a name, so the category
+// decides their order. mlir-opt calls finalize() explicitly and the engine
+// destructor calls it again. The second call must not emit the remarks a
+// second time. --implicit-check-not pins the number of "remark:" lines and of
+// YAML records to five.
// CHECK: remark: [Passed] test-remark | Category:category-1-passed |{{.*}}Remark="This is a test passed remark",
-// CHECK: remark: [Failure] test-remark | Category:category-2-failed
// CHECK: remark: [Analysis] test-remark | Category:category-2-analysis
+// CHECK: remark: [Failure] test-remark | Category:category-2-failed
// CHECK: remark: [Passed] test-remark | Category:category-link |{{.*}}RelatedTo=
// CHECK: remark: [Analysis] test-remark | Category:category-link
// CHECK-YAML: --- !Passed
// CHECK-YAML-NEXT: Pass:{{.*}}category-1-passed
-// CHECK-YAML: --- !Failure
-// CHECK-YAML-NEXT: Pass:{{.*}}category-2-failed
// CHECK-YAML: --- !Analysis
// CHECK-YAML-NEXT: Pass:{{.*}}category-2-analysis
+// CHECK-YAML: --- !Failure
+// CHECK-YAML-NEXT: Pass:{{.*}}category-2-failed
// CHECK-YAML: --- !Passed
// CHECK-YAML-NEXT: Pass:{{.*}}category-link
// CHECK-YAML: --- !Analysis
diff --git a/mlir/unittests/IR/RemarkTest.cpp b/mlir/unittests/IR/RemarkTest.cpp
index fba3d96b4389f0..b7ee717a74f4a8 100644
--- a/mlir/unittests/IR/RemarkTest.cpp
+++ b/mlir/unittests/IR/RemarkTest.cpp
@@ -14,6 +14,7 @@
#include "mlir/Remark/RemarkStreamer.h"
#include "mlir/Support/TypeID.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Config/llvm-config.h"
#include "llvm/IR/LLVMRemarkStreamer.h"
#include "llvm/Remarks/RemarkFormat.h"
#include "llvm/Support/FileSystem.h"
@@ -21,7 +22,9 @@
#include "llvm/Support/YAMLParser.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <atomic>
#include <optional>
+#include <thread>
#include <vector>
using namespace mlir;
@@ -426,22 +429,31 @@ class RecordingStreamer : public remark::detail::MLIRRemarkStreamerBase {
std::vector<std::string> &out;
};
-static LogicalResult enableFinalPolicy(MLIRContext &context,
- std::vector<std::string> &emitted,
- StringRef category) {
+/// Enables passed and analysis remarks of `category` with the given policy,
+/// recording what the streamer receives into `emitted`.
+static LogicalResult
+enablePolicy(MLIRContext &context, std::vector<std::string> &emitted,
+ StringRef category,
+ std::unique_ptr<remark::detail::RemarkEmittingPolicyBase> policy,
+ bool printAsEmitRemarks = false) {
mlir::remark::RemarkCategories cats{/*all=*/std::nullopt,
/*passed=*/category.str(),
/*missed=*/std::nullopt,
/*analysis=*/category.str(),
/*failed=*/std::nullopt};
return remark::enableOptimizationRemarks(
- context, std::make_unique<RecordingStreamer>(emitted),
- std::make_unique<remark::RemarkEmittingPolicyFinal>(), cats,
- /*printAsEmitRemarks=*/false);
+ context, std::make_unique<RecordingStreamer>(emitted), std::move(policy),
+ cats, printAsEmitRemarks);
+}
+
+static LogicalResult enableFinalPolicy(MLIRContext &context,
+ std::vector<std::string> &emitted,
+ StringRef category) {
+ return enablePolicy(context, emitted, category,
+ std::make_unique<remark::RemarkEmittingPolicyFinal>());
}
-// The final policy emits remarks in creation order. A later report of an
-// identity replaces the earlier one and takes its own position.
+// A later report of an identity replaces the earlier one.
TEST(Remark, TestRemarkFinalOrder) {
std::vector<std::string> emitted;
{
@@ -469,17 +481,17 @@ TEST(Remark, TestRemarkFinalDrains) {
MLIRContext context;
ASSERT_TRUE(succeeded(enableFinalPolicy(context, emitted, "LoopUnroll")));
Location loc = FileLineColLoc::get(&context, "test.cpp", 1, 5);
- auto *policy = context.getRemarkEngine()->getRemarkEmittingPolicy();
+ remark::detail::RemarkEngine *engine = context.getRemarkEngine();
auto first = remark::RemarkOpts::name("First").category("LoopUnroll");
auto second = remark::RemarkOpts::name("Second").category("LoopUnroll");
remark::passed(loc, first) << "first";
remark::passed(loc, second) << "second";
- policy->finalize();
+ engine->finalizePolicy();
EXPECT_THAT(emitted, ElementsAre("First: first", "Second: second"));
// Nothing pending: a repeated call emits nothing.
- policy->finalize();
+ engine->finalizePolicy();
EXPECT_EQ(emitted.size(), 2u);
// A drained identity can be reported again; it waits for the next call,
@@ -500,7 +512,7 @@ TEST(Remark, TestRemarkFinalLinkAcrossDrains) {
MLIRContext context;
ASSERT_TRUE(succeeded(enableFinalPolicy(context, emitted, "LoopUnroll")));
Location loc = FileLineColLoc::get(&context, "test.cpp", 1, 5);
- auto *policy = context.getRemarkEngine()->getRemarkEmittingPolicy();
+ remark::detail::RemarkEngine *engine = context.getRemarkEngine();
remark::RemarkId analysisId;
{
@@ -510,7 +522,7 @@ TEST(Remark, TestRemarkFinalLinkAcrossDrains) {
analysis << "trip count 128";
analysisId = analysis.getId();
}
- policy->finalize();
+ engine->finalizePolicy();
EXPECT_THAT(emitted, ElementsAre("Analysis: trip count 128"));
remark::passed(loc, remark::RemarkOpts::name("Unroller")
@@ -522,6 +534,178 @@ TEST(Remark, TestRemarkFinalLinkAcrossDrains) {
ElementsAre("Analysis: trip count 128", "Unroller: unrolled"));
}
+// Root remarks come out sorted by file, line and column, then by remark name.
+// Remarks whose location holds no file position go last.
+TEST(Remark, TestRemarkFinalSourceOrder) {
+ std::vector<std::string> emitted;
+ {
+ MLIRContext context;
+ ASSERT_TRUE(succeeded(enableFinalPolicy(context, emitted, "LoopUnroll")));
+ auto at = [&](StringRef file, unsigned line, unsigned col) -> Location {
+ return FileLineColLoc::get(&context, file, line, col);
+ };
+ auto named = [](StringRef name) {
+ return remark::RemarkOpts::name(name).category("LoopUnroll");
+ };
+
+ remark::passed(UnknownLoc::get(&context), named("R")) << "unknown";
+ remark::passed(at("b.cpp", 1, 1), named("R")) << "b.cpp:1:1";
+ remark::passed(at("a.cpp", 9, 1), named("R")) << "a.cpp:9:1";
+ remark::passed(
+ NameLoc::get(StringAttr::get(&context, "n"), at("a.cpp", 5, 1)),
+ named("R"))
+ << "a.cpp:5:1 via NameLoc";
+ remark::passed(at("a.cpp", 2, 7), named("R")) << "a.cpp:2:7";
+ remark::passed(at("a.cpp", 2, 3), named("Zeta")) << "a.cpp:2:3";
+ remark::passed(at("a.cpp", 2, 3), named("Alpha")) << "a.cpp:2:3";
+ }
+ EXPECT_THAT(emitted,
+ ElementsAre("Alpha: a.cpp:2:3", "Zeta: a.cpp:2:3", "R: a.cpp:2:7",
+ "R: a.cpp:5:1 via NameLoc", "R: a.cpp:9:1",
+ "R: b.cpp:1:1", "R: unknown"));
+}
+
+// The order depends only on the set of remarks, not on the order in which
+// they were reported. Code inlined from one callee at two call sites shares
+// the callee's position and is ordered by the call site.
+TEST(Remark, TestRemarkFinalOrderIndependentOfReportOrder) {
+ auto run = [](ArrayRef<unsigned> reportOrder) {
+ std::vector<std::string> emitted;
+ {
+ MLIRContext context;
+ EXPECT_TRUE(succeeded(enableFinalPolicy(context, emitted, "LoopUnroll")));
+ Location callee = FileLineColLoc::get(&context, "helper.h", 3, 1);
+ SmallVector<Location> locs = {
+ CallSiteLoc::get(callee,
+ FileLineColLoc::get(&context, "a.cpp", 20, 1)),
+ CallSiteLoc::get(callee,
+ FileLineColLoc::get(&context, "a.cpp", 10, 1)),
+ FileLineColLoc::get(&context, "a.cpp", 4, 2),
+ FileLineColLoc::get(&context, "a.cpp", 4, 1)};
+ auto opts = remark::RemarkOpts::name("R").category("LoopUnroll");
+ for (unsigned index : reportOrder)
+ remark::passed(locs[index], opts)
+ << ("remark " + std::to_string(index));
+ }
+ return emitted;
+ };
+ std::vector<std::string> forward = run({0, 1, 2, 3});
+ EXPECT_THAT(forward, ElementsAre("R: remark 3", "R: remark 2", "R: remark 1",
+ "R: remark 0"));
+ EXPECT_EQ(forward, run({3, 1, 0, 2}));
+}
+
+// A related remark is printed right after the remark that references it, not
+// at its own position.
+TEST(Remark, TestRemarkFinalChildFollowsParent) {
+ std::vector<std::string> emitted;
+ {
+ MLIRContext context;
+ ASSERT_TRUE(succeeded(enableFinalPolicy(context, emitted, "LoopUnroll")));
+ Location early = FileLineColLoc::get(&context, "test.cpp", 1, 1);
+ Location late = FileLineColLoc::get(&context, "test.cpp", 9, 1);
+
+ remark::RemarkId childId;
+ {
+ auto child = remark::analysis(
+ early, remark::RemarkOpts::name("Child").category("LoopUnroll"));
+ child << "at line 1";
+ childId = child.getId();
+ }
+ remark::passed(early,
+ remark::RemarkOpts::name("Other").category("LoopUnroll"))
+ << "at line 1";
+ remark::passed(late, remark::RemarkOpts::name("Parent")
+ .category("LoopUnroll")
+ .relatedTo(childId))
+ << "at line 9";
+ }
+ EXPECT_THAT(emitted, ElementsAre("Other: at line 1", "Parent: at line 9",
+ "Child: at line 1"));
+}
+
+#if LLVM_ENABLE_THREADS
+/// Runs `body(threadIndex)` on `numThreads` threads released together.
+static void runOnThreads(unsigned numThreads,
+ llvm::function_ref<void(unsigned)> body) {
+ std::atomic<bool> start{false};
+ std::vector<std::thread> threads;
+ for (unsigned t = 0; t < numThreads; ++t) {
+ threads.emplace_back([&, t] {
+ while (!start.load())
+ std::this_thread::yield();
+ body(t);
+ });
+ }
+ start.store(true);
+ for (std::thread &thread : threads)
+ thread.join();
+}
+
+// Reports from several threads reach the final policy one at a time, and the
+// emitted order does not depend on how the threads were scheduled.
+TEST(Remark, TestRemarkFinalConcurrent) {
+ constexpr unsigned numThreads = 8, perThread = 64;
+ std::vector<std::string> emitted;
+ {
+ MLIRContext context;
+ ASSERT_TRUE(succeeded(enableFinalPolicy(context, emitted, "LoopUnroll")));
+ SmallVector<Location> locs;
+ for (unsigned line = 1; line <= numThreads * perThread; ++line)
+ locs.push_back(FileLineColLoc::get(&context, "test.cpp", line, 1));
+ Location shared = FileLineColLoc::get(&context, "shared.cpp", 1, 1);
+ auto opts = remark::RemarkOpts::name("R").category("LoopUnroll");
+
+ runOnThreads(numThreads, [&](unsigned t) {
+ for (unsigned i = 0; i < perThread; ++i) {
+ unsigned line = i * numThreads + t + 1;
+ remark::passed(locs[line - 1], opts)
+ << ("line " + std::to_string(line));
+ }
+ // Every thread reports the same identity; one report survives.
+ remark::passed(shared, opts) << "shared";
+ });
+ }
+ std::vector<std::string> expected = {"R: shared"};
+ for (unsigned line = 1; line <= numThreads * perThread; ++line)
+ expected.push_back("R: line " + std::to_string(line));
+ EXPECT_EQ(emitted, expected);
+}
+
+// Under the All policy the streamer and the diagnostic printer run for every
+// report; the engine serializes them, so a streamer without a lock of its own
+// sees every remark exactly once.
+TEST(Remark, TestRemarkAllConcurrent) {
+ constexpr unsigned numThreads = 8, perThread = 64;
+ std::vector<std::string> emitted;
+ unsigned printed = 0;
+ {
+ MLIRContext context;
+ ScopedDiagnosticHandler handler(&context, [&](Diagnostic &) {
+ ++printed;
+ return success();
+ });
+ ASSERT_TRUE(succeeded(
+ enablePolicy(context, emitted, "LoopUnroll",
+ std::make_unique<remark::RemarkEmittingPolicyAll>(),
+ /*printAsEmitRemarks=*/true)));
+ Location loc = FileLineColLoc::get(&context, "test.cpp", 1, 1);
+ auto opts = remark::RemarkOpts::name("R").category("LoopUnroll");
+
+ runOnThreads(numThreads, [&](unsigned t) {
+ for (unsigned i = 0; i < perThread; ++i)
+ remark::passed(loc, opts)
+ << ("remark " + std::to_string(t * perThread + i));
+ });
+ }
+ std::vector<std::string> expected;
+ for (unsigned index = 0; index < numThreads * perThread; ++index)
+ expected.push_back("R: remark " + std::to_string(index));
+ EXPECT_THAT(emitted, ::testing::UnorderedElementsAreArray(expected));
+ EXPECT_EQ(printed, numThreads * perThread);
+}
+#endif // LLVM_ENABLE_THREADS
+
TEST(Remark, TestArgWithAttribute) {
MLIRContext context;
More information about the llvm-branch-commits
mailing list