-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[MLIR][Remark] Add support for dropping remarks #159519
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
base: main
Are you sure you want to change the base?
Conversation
By default, `InFlightRemark` are emitted as soon as their scope ends. This PR adds a mode to postpone emission until the end of compilation, so remarks are processed at once. This feature will allow later passes to drop, rewrite, or aggregate remarks before they are printed/streamed. This will be done in follow-up PRs.
Co-authored-by: Mehdi Amini <joker.eph@gmail.com>
Co-authored-by: Mehdi Amini <joker.eph@gmail.com>
Co-authored-by: Mehdi Amini <joker.eph@gmail.com>
Co-authored-by: Mehdi Amini <joker.eph@gmail.com>
This PR adds support for dropping remarks emitted by earlier passes. This is a common use case in optimizing compilers where many passes are involved. For example, consider the following pipeline: ``` pass-early : failed unrolling ... other passes ... pass-late : succeeded unrolling ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for dropping remarks in MLIR's remark system, allowing later passes to remove remarks emitted by earlier passes. This is particularly useful in optimization pipelines where multiple passes operate on the same code and later passes may want to override earlier remarks.
Key changes include:
- Addition of postponed remarks that are deferred until pipeline completion
- API for dropping specific postponed remarks based on location and metadata
- Hash-based storage system for efficient remark lookup and removal
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
mlir/include/mlir/IR/Remarks.h | Adds postponed flag to RemarkOpts, DenseSet storage for postponed remarks, and hash specialization for Remark comparison |
mlir/lib/IR/Remarks.cpp | Implements postponed remark storage, emission deferral, and cleanup logic |
mlir/lib/IR/MLIRContext.cpp | Ensures remark engine finalization before context destruction |
mlir/unittests/IR/RemarkTest.cpp | Adds comprehensive tests for postponed remark functionality and drop behavior |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions cpp,h -- mlir/include/mlir/IR/Remarks.h mlir/lib/IR/MLIRContext.cpp mlir/lib/IR/Remarks.cpp mlir/unittests/IR/RemarkTest.cpp
View the diff from clang-format here.diff --git a/mlir/include/mlir/IR/Remarks.h b/mlir/include/mlir/IR/Remarks.h
index c74070122..0c16e741b 100644
--- a/mlir/include/mlir/IR/Remarks.h
+++ b/mlir/include/mlir/IR/Remarks.h
@@ -453,7 +453,9 @@ public:
InFlightRemark emitOptimizationRemarkAnalysis(Location loc, RemarkOpts opts);
/// Get the postponed remarks.
- const DenseSet<Remark> &getPostponedRemarks() const { return postponedRemarks; }
+ const DenseSet<Remark> &getPostponedRemarks() const {
+ return postponedRemarks;
+ }
/// Clear the postponed remarks.
void clearPostponedRemarks() { postponedRemarks.clear(); }
|
This PR adds support for dropping remarks emitted by earlier passes. It is a common use case in optimizing compilers where many passes are involved.
For example, consider the following pipeline. In that case,
pass-late
can drop thefailed unrolling
remark because it is irrelevant, and emit new remark.To be droppable, a remark must be emitted as a
postponed
remark. Non-postponed remarks are emitted immediately and cannot be dropped later.Requires #157434