[AIG][NFC] Introduce a helper struct to manage result paths #8651
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a LongestPathCollection helper to encapsulate path storage, sorting, and deduplication, and refactors the existing pass to use it instead of ad-hoc vectors.
- Add
LongestPathCollection(with sort & dedupe methods) and store the MLIR context inLongestPathAnalysis - Refactor
PrintLongestPathAnalysisPassto use the collection for gathering, sorting, and filtering paths - Update headers with necessary includes and declare the new helper struct
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/Dialect/AIG/Analysis/PrintLongestPathAnalysis.cpp | Refactor to use LongestPathCollection, simplify path management and printing logic |
| lib/Dialect/AIG/Analysis/LongestPathAnalysis.cpp | Initialize and store context in LongestPathAnalysis; implement LongestPathCollection methods |
| include/circt/Dialect/AIG/Analysis/LongestPathAnalysis.h | Add context member & accessor, include headers, and declare LongestPathCollection |
Comments suppressed due to low confidence (3)
lib/Dialect/AIG/Analysis/PrintLongestPathAnalysis.cpp:96
- [nitpick] The alias
longestPathForEachFanOutmay be confused with the old vector; consider renaming it to something likecriticalPathsoruniqueFanOutPaths.
auto &longestPathForEachFanOut = collection.paths;
include/circt/Dialect/AIG/Analysis/LongestPathAnalysis.h:247
- The return type
MLIRContextshould be fully qualified asmlir::MLIRContextto match the member declaration and avoid lookup errors.
MLIRContext *getContext() const { return ctx; }
include/circt/Dialect/AIG/Analysis/LongestPathAnalysis.h:27
- Add
#include "llvm/ADT/DenseSet.h"so thatllvm::DenseSetused insortAndDropNonCriticalPathsPerFanOutis declared.
#include "llvm/ADT/SmallVector.h"
Add LongestPathCollection class to manage and own path data
9a8121f to
fde061a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add LongestPathCollection class to manage and own path data