[AIG] Refactor Longest Path Analysis with hierarchical path support #8630
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
0c57de8 to
a4b1174
Compare
eee46e9 to
a29bea2
Compare
This patch refactors the Longest Path Analysis in the AIG dialect with several enhancements: - Refactor DataflowPath to use a clean variant-based API for different endpoint types - Reorganize the API with purpose-specific methods replacing complex tuple returns - Add support for hierarchical path elaboration with prependPaths methods - Introduce a unified getAllPaths convenience method to collect all timing paths - Simplify the PrintLongestPathAnalysis pass to use the new API The refactoring enables more comprehensive timing path analysis across module boundaries with proper hierarchical instance information while providing a cleaner, more maintainable API.
a29bea2 to
9fc57cb
Compare
fabianschuiki
approved these changes
Jul 1, 2025
Comment on lines
+204
to
+223
| // Return input-to-internal timing paths for the given module. | ||
| // These are open paths from module input ports to internal sequential | ||
| // elements (registers/flip-flops). | ||
| LogicalResult getOpenPathsFromInputPortsToInternal( | ||
| StringAttr moduleName, SmallVectorImpl<DataflowPath> &results) const; | ||
|
|
||
| // Return internal-to-output timing paths for the given module. | ||
| // These are open paths from internal sequential elements to module output | ||
| // ports. | ||
| LogicalResult getOpenPathsFromInternalToOutputPorts( | ||
| StringAttr moduleName, SmallVectorImpl<DataflowPath> &results) const; | ||
|
|
||
| // Get all timing paths in the given module including both closed and open | ||
| // paths. This is a convenience method that combines results from | ||
| // getClosedPaths, getOpenPathsFromInputPortsToInternal, and | ||
| // getOpenPathsFromInternalToOutputPorts. If elaboratedPaths is true, paths | ||
| // include full hierarchical instance information. | ||
| LogicalResult getAllPaths(StringAttr moduleName, | ||
| SmallVectorImpl<DataflowPath> &results, | ||
| bool elaboratePaths = false) const; |
There was a problem hiding this comment.
Nice split! Convenient to have results be the same type everywhere.
| // FanOut can be either an internal circuit object or a module output port | ||
| // This flexibility allows representing both closed paths | ||
| // (register-to-register) and open paths (register-to-output) in a unified way | ||
| using FanOutType = std::variant<Object, std::pair<size_t, size_t>>; |
There was a problem hiding this comment.
Probably not really useful, but maybe a using Port = std::pair<size_t, size_t> could help give the int pair discernible meaning?
There was a problem hiding this comment.
Good point! I created OutputPort alias as input port is represented as blockarg.
Comment on lines
+260
to
+261
| using Info = llvm::DenseMapInfo< | ||
| std::tuple<circt::igraph::InstancePath, mlir::Value, size_t>>; |
Comment on lines
+1401
to
+1403
| if (elaboratePaths) | ||
| return impl->getClosedPaths<true>(moduleName, results); | ||
| return impl->getClosedPaths<false>(moduleName, results); |
There was a problem hiding this comment.
Did you pull the elaborate boolean out into a template argument to get two optimized hot loops in getClosedPaths for the two cases? That's pretty neat 👍
TaoBi22
pushed a commit
to TaoBi22/circt
that referenced
this pull request
Jul 17, 2025
…lvm#8630) This patch refactors the Longest Path Analysis in the AIG dialect with several enhancements: - Refactor DataflowPath to use a clean variant-based API for different endpoint types - Reorganize the API with purpose-specific methods replacing complex tuple returns - Add support for hierarchical path elaboration with prependPaths methods - Introduce a unified getAllPaths convenience method to collect all timing paths - Simplify the PrintLongestPathAnalysis pass to use the new API The refactoring enables more comprehensive timing path analysis across module boundaries with proper hierarchical instance information while providing a cleaner, more maintainable API.
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.
This patch refactors the Longest Path Analysis in the AIG dialect with several enhancements:
The refactoring enables more comprehensive timing path analysis across module boundaries with proper hierarchical instance information while providing a cleaner, more maintainable API.