Skip to content
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

cost: Add $mem_v2, $macc_v2 estimates #4946

Merged
merged 2 commits into from
Apr 14, 2025
Merged

Conversation

povik
Copy link
Member

@povik povik commented Mar 18, 2025

What are the reasons/motivation for this change?

Fill in blind spots of the keep_hierarchy command.

Explain how this is achieved.

Add $mem, $macc estimation consistent with existing $mul, $dff formulas.

If applicable, please suggest to reviewers how they can test the change.

This script can exercise the new support:

design -reset
read_verilog <<EOF
module top(input clk, input [10:0] a, input [2:0] w, output [2:0] y);
	reg [3:0] m[60:0];

	always @(posedge clk) begin
		y <= m[a];
		m[a] <= w;
	end
endmodule
EOF
prep
keep_hierarchy -min_cost 20

design -reset
read_verilog <<EOF
module top(input [2:0] a, input [2:0] b, output [2:0] y);
	assign y = a * b;
endmodule
EOF
design -save r
keep_hierarchy -min_cost 20

design -load r
alumacc
opt_clean
keep_hierarchy -min_cost 20

Copy link
Collaborator

@widlarizer widlarizer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but the terminology change should be reflected in docs/source/cell/word_arith.rst as well

Copy link
Collaborator

@widlarizer widlarizer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the "port" concept that has been renamed to "term" is code-only and isn't referred to by the docs. The docs only refer to "port" as in cell port. The docs refer to the now-"term" as "summand" which is more explicit as to its role than "term" but longer. I would slightly prefer "term" being replaced with "summand" in the codebase for consistency but it's not a big deal

@povik povik merged commit 38beae1 into YosysHQ:main Apr 14, 2025
25 checks passed
@povik povik deleted the cost-cc-enhance branch April 14, 2025 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants