-
Notifications
You must be signed in to change notification settings - Fork 404
[Synth] Add CutRewriter framework and TechMapper pass #8868
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
Conversation
062ec9e to
6787948
Compare
This commit implements a cut-based rewriting framework for combinational logic optimization and adds the TechMapper pass that uses NPN-equivalence matching with area and delay metrics to map AIG circuits to technology libraries. The TechMapper pass processes HWModuleOp instances with hw.techlib.info attributes as library patterns and maps non-library modules to optimal gate implementations. The framework includes comprehensive test coverage for both functionality and error handling scenarios. The implementation adapts to the current Synth dialect structure and integrates with the circt-synth command-line tool.
6787948 to
18c6b10
Compare
cowardsa
left a comment
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.
Great work - so much interesting stuff in here - my only major concern is around useTruthTableMatcher - where it seems to return false always? (But expect I've misunderstood)
I really like the interface for defining a technology library - very cool! You could even take this further and allow users to define higher-level custom patterns that help them bring their own component library - a bit like the pulp library!
| /// Get the best matched pattern for this cut set. | ||
| std::optional<MatchedPattern> getMatchedPattern() const; |
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.
Nit: getBestMatchedPattern(), could also have a getMatchedPatterns() function - as presumably there will be some area/delay tradeoff to balance?
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.
That's good point. Will change to getBestMatchedPattern. I think keeping few matched patterns are great and will do in the follow-up
| /// Enumerate cuts for all nodes in the given module. | ||
| LogicalResult enumerateCuts(Operation *topOp); |
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.
Q: will the current implementation will preserve module boundaries? Namely, there will be no rewriting across the hierarchy?
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.
Yes, it preserves module boundaries. Rewriting across module boundaries would be very complicated (and also it's hard to parallelize across modules) so currently I don't have a plan. Users are expected to perform inlining or flattening if context-sensitivity is important. I think we can still use global timing analysis results as arrival times of inputs (input ports or instance results).
| // Set delay for binary and inv op to 5 so that others will be prioritized | ||
| hw.module @and_inv(in %a : i1, in %b : i1, out result : i1) attributes {hw.techlib.info = {area = 1.0 : f64, delay = [[5], [5]]}} { | ||
| %0 = aig.and_inv %a, %b : i1 | ||
| hw.output %0 : i1 | ||
| } |
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.
This is really cool - like the idea of specifying the technology library within the IR itself! Maybe its added elsewhere but perhaps a safety mechanism would be useful to check that a pattern's techlib.info is only being set once?
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.
Yeah eventually we would like to introduce a cell operation but using attributes on hw.module would be good enough until then. The hw.techlib.info is an attribute so MLIR verifier should ensure the uniqueness.
| std::unique_ptr<CutRewritePattern> pattern = | ||
| std::make_unique<TechLibraryPattern>(hwModule, area, std::move(delay), | ||
| std::move(*npnClass)); |
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.
Q: do we have any checks on unique names and unique pattern definitions? Does it matter if we have two patterns implementing the same circuit with different costs?
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.
Yes, it's possible that there are multiple patterns that are equivalent circuit with different costs, e.g. pattern A/B has smallest delay from input 0/1 respectively. CutRewriter will try all available patterns and picks a best pattern.
| // expected-error@+1 {{All input ports must be single bit}} | ||
| hw.module @multibit(in %a : i1, in %b: i2, out result : i2) attributes {hw.techlib.info = {area = 1.0 : f64, delay = [[1], [2]]}} { |
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.
Q: are multi-bit outputs supported?
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.
Good point. It's not supported now. I'll add a test. It's possible to support but it will be simply treated as multiple single bits outputs.
| // This is a test that needs area-flow to get an optimal result. | ||
| // It produces sub-optimal mappings since currently area-flow is not implemented. |
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.
Q: What is area-flow?
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.
I added a comment and reference to "Heuristics for Area Minimization in LUT-Based FPGA Technology Mapping" which proposed area-flow.
Good point, this is is not used in the current PR so I removed a handling useTrtuTableMatcher in |
This commit implements a cut-based rewriting framework for combinational logic optimization
and adds the TechMapper pass that uses NPN-equivalence matching with area and delay
metrics to map AIG circuits to technology libraries. The implementation is based on:
The Technology Mapping Pass shows how to use this framework in practice by
mapping generic logic operations to specific hardware gates from a technology
library. This pass also serves as a test for the framework since
testing the cut rewriting algorithms directly would be very difficult without
a real application to exercise them.
The TechMapper pass processes
HWModuleOpinstances withhw.techlib.infoattributes as library patterns for now. This must be replaced with more well-designed representation.
The framework is designed to be easily extended with new optimization patterns
and strategies while maintaining good performance for cut enumeration and
pattern matching.