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

Add -blackbox option to cutpoint pass #4854

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

KrystalDelusion
Copy link
Member

@KrystalDelusion KrystalDelusion commented Jan 16, 2025

What are the reasons/motivation for this change?
Supersedes #4812.

Explain how this is achieved.
Calling cutpoint -blackbox [options] will replace the contents of all (black)boxes in the design with a formal cut point ($anyseq cell by default).
This is preferable to #4812, which instead calls cutpoint on instances of (black)box modules.
Since the module now has contents, it is also no longer a (black)box so we can remove those attributes.

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

Replace the contents of all blackboxes in the design with a formal cut point.
Includes test script.
Also adds "blackbox" as a valid TYPE.
Modify `cutpoint_blackbox.ys` to check that parameters on blackbox modules are maintained after the cutpoint.
Also adjusts the test to check that each instance gets the `$anyseq` cell.
@povik
Copy link
Member

povik commented Jan 17, 2025

From #4812:

This way we avoid any problems with needing to derive de-blackboxed modules to handle I/O signals with parametrizable widths, which isn't possible when they came from verific.

So I take it this new approach doesn't support parametrizable widths.

@KrystalDelusion
Copy link
Member Author

Uhhh, pass?

module top(input a, b, output o);
    wire c, d, e, f;
    bb #(.SOME_PARAM(1)) bb1 (.a (a), .b (b), .o (c));
    wb wb1 (.a (a), .b (b), .o (d));
    bb #(.SOME_PARAM(2)) bb2 (.a ({a,b}), .b ({c,d}), .o ({e,f}));
    some_mod some_inst (.a (c), .b (d), .c (e), .o (o));
endmodule

(* blackbox *)
module bb #( parameter SOME_PARAM=0 ) (input [SOME_PARAM-1:0] a, b, output [SOME_PARAM-1:0] o);
assign o = a | b;
specify
	(a => o) = 1;
endspecify
endmodule

(* whitebox *)
module wb(input a, b, output o);
assign o = a ^ b;
endmodule

(* keep_hierarchy *)
module some_mod(input a, b, c, output o);
assign o = a & (b | c);
endmodule
read -vlog2k cutpoint_blackbox.v
hierarchy -top top
cutpoint -blackbox
flatten
opt -purge

cut

read -vlog2k cutpoint_blackbox.v
hierarchy -top top
select top
setundef -blackbox -anyseq

set

@KrystalDelusion
Copy link
Member Author

I'm not seeing any change in behaviour there; the $anyseq cells for the bb module are both 2bit wide when coming from verific using both the cutpoint and setundef versions of this.

@povik
Copy link
Member

povik commented Jan 17, 2025

I understood inserting the $anyseq cells on blackbox outputs without going the flatten route was motivated by making parametrizable widths work with Verific. Since this PR now has a different approach I'm wondering if parametrizable widths dropped from the requirements

@KrystalDelusion
Copy link
Member Author

I don't have any examples that demonstrate whatever problems are being encountered with the current options when it comes to blackboxes in verific, but as far as I can tell changing the cell outputs (i.e the setundef version of this) gives the same result as this when it comes to output widths. In my quick testing it looks like verific sets the port width on the module and all instances of it to the widest required.

Rather than it no longer being required, I would say that if this implementation is not meeting that requirement then I don't see how #4812 was.

@KrystalDelusion
Copy link
Member Author

I wasn't there for the original dev JF discussion, but when we discussed #4812 in docs JF about whether calling cutpoint from within setundef was the right approach we said:

  • Using cutpoint to remove an instance should be equivalent to cutting out all contents of the module and then flattening that module into the module containing the instance (wrt generated $scopeinfo and hdlname attributes)
  • Cutpoint alone can handle all current use cases, let's not put it in setundef for now

(or at least that's what is in the minutes)

@KrystalDelusion KrystalDelusion added the discuss to be discussed at next dev jour fixe (see #devel-discuss at https://yosyshq.slack.com/) label Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss to be discussed at next dev jour fixe (see #devel-discuss at https://yosyshq.slack.com/)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants