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

Unclear error reporting in the presence of multiple conflicting drivers #57

Open
maliberty opened this issue Mar 17, 2024 · 11 comments
Open

Comments

@maliberty
Copy link

With the attached test case, when I run eqy -d run --force --jobs 40 4_eqy_test.eqy I get:

EQY 21:16:12 [run] partition: starting process "cd run; yosys -ql partition.log partition.ys"
EQY 21:16:15 [run] partition: ERROR: Multiple drivers for \_27797_.B2.
EQY 21:16:15 [run] partition: finished (returncode=1)

However I can not see more than one driver for the net attached to pin _27797_.B2. It would be helpful if the message gave the driver names (or at least two of them). It seems like a false error from what I can see but it prevents completion of the check.

bug.zip

@QuantamHD
Copy link

This one is important to Google.

@maliberty
Copy link
Author

By other means I determined that there was a real error in OR where an input and output were being swapped. However the message here was so unclear to me that it provided no help. There may be a real issue in this test case but I can't tell.

@jix
Copy link
Member

jix commented Mar 19, 2024

Note that EQY already outputs the drivers for the net _27797_.B2 earlier when emitting warnings for conflicting drivers:

[run] read_gate: Warning: multiple conflicting drivers for aes_cipher_top.\_27797_.B2:
[run] read_gate: port Y[0] of cell $flatten\_27691_.$not$cells.v:434$111 ($not)
[run] read_gate: port Y[0] of cell $flatten\_28908_.$xor$cells.v:411$101 ($xor)
[run] read_gate: port Y[0] of cell $flatten\_28912_.$xor$cells.v:411$101 ($xor)

I agree that it's not clear from the current output that the reason for the error can be found earlier in the log, and I will look into improving our error reporting here.

Even with the currently produced warnings, though, it's not directly obvious where the conflict is, as the conflicting drivers check is performed when the design is already flattened. Flattening replaced buffer cells with their implementation in the cell library, which consists of a direct connection. This in turn means that when the conflicting drivers are buffer cells in the input netlist, the error message for the flattened design will report the input to those buffers as conflicting drivers. This will persist even with improved error reporting in EQY, as the design is flattened due to the prep -flatten command that is part of the .eqy configuration file.

One solution here is to include additional checks on the hierarchical design as parts of the .eqy configuration:

[gold]
read_verilog -sv 4_before_rsz.v cells.v

hierarchy -top aes_cipher_top
check -assert

prep -top aes_cipher_top -flatten
memory_map

[gate]
read_verilog -sv 4_after_rsz.v cells.v

hierarchy -top aes_cipher_top
check -assert

prep -top aes_cipher_top -flatten
memory_map

Or using the [script] section which appends the same commands to both the [gold] and [gate] scripts:

[gold]
read_verilog -sv 4_before_rsz.v cells.v

[gate]
read_verilog -sv 4_after_rsz.v cells.v

[script]
hierarchy -top aes_cipher_top
check -assert

prep -top aes_cipher_top -flatten
memory_map

This will perform these checks on the design before inlining the cell definitions during flatten. It will also treat any failure as a hard error, making sure the relevant output is at the bottom of EQY's log output and not hidden among other warnings.

This then fails with

[4_eqy_test] read_gold: Warning: Wire DFF_X2.\QN is used but has no driver.
[4_eqy_test] read_gold: ERROR: Found 1 problems in 'check -assert'.

and indeed DFF_X2 contains an assignment to Q that looks like it should have been an assignment to QN:

module DFF_X2 (CK, D, Q, QN);
   input CK;
   input D;
   output Q;
   output QN;
   always @(posedge CK) begin
      Q <= D;
      Q <= ~D; // <-- here
   end
endmodule // DFF_X2

After fixing this, it will continue checking aes_cipher_top producing error messages that are much more helpful, e.g.:

EQY 14:55:30 [4_eqy_test] read_gate: Warning: multiple conflicting drivers for aes_cipher_top.\net1125:
EQY 14:55:30 [4_eqy_test] read_gate: port S[0] of cell _28912_ (HA_X1)
EQY 14:55:30 [4_eqy_test] read_gate: port Z[0] of cell rebuffer467 (BUF_X16)

matching the input netlist:

 HA_X1 _28912_ (.A(_05120_),
    .B(_14860_),
    .CO(_14859_),
    .S(net1125));
 BUF_X16 rebuffer467 (.A(_05103_),
    .Z(net1125));

@jix jix changed the title Seemingly false "ERROR: Multiple drivers" Unclear error reporting in the presence of multiple conflicting drivers Mar 19, 2024
@maliberty
Copy link
Author

Thanks for catching the DFF_X2 error, I'll fix that in ORFS. The revised error you show matches the HA related bug I fixed in OR so that seems correct.

I'll adopt your recommended script. I suggest you publish a recommended script as it is very hard to understand eqy. We are also using the recommendations in #40

@maliberty
Copy link
Author

With

[gold]
read_verilog -sv ./results/nangate45/aes/base/4_before_rsz.v /workspaces/mliberty-ssd/w1/OpenROAD-flow-scripts/flow/platforms/nangate45/work_around_yosys/cells.v

[gate]
read_verilog -sv ./results/nangate45/aes/base/4_after_rsz.v /workspaces/mliberty-ssd/w1/OpenROAD-flow-scripts/flow/platforms/nangate45/work_around_yosys/cells.v

[script]
hierarchy -top aes_cipher_top
check -assert


prep -top aes_cipher_top -flatten
memory_map


[match *]
gate-nomatch _*_.*


[strategy basic]
use sat
depth 10


[strategy sby]
use sby
depth 10
engine smtbmc bitwuzla

I get odd errors like:

EQY  9:36:35 [./logs/nangate45/aes/base/4_eqy_output] run: Running strategy 'basic' on 'aes_cipher_top.clkbuf_leaf_19_clk.Z'..
EQY  9:36:35 [./logs/nangate45/aes/base/4_eqy_output] run: ERROR: Failed to import cell $flatten\__po_clkbuf_3_3__f_clk.Z__assert.$assert$../../../partitions/aes_cipher_top.clkbuf_3_3__f_clk.Z.sv:58$100177 (type $check) to SAT database.

I have no idea what that means.

@jix
Copy link
Member

jix commented Mar 19, 2024

Are you using the current main branch of EQY? I can only see how this error would occur when using a recent yosys with an older EQY. In any case this is an incompatibility with the output yosys produces and the passes that EQY runs, so if you are using the current main branch of EQY this is a new bug, not an error in using EQY, and it would be helpful if you can provide a reproducing example for this.

@maliberty
Copy link
Author

0.39

@maliberty
Copy link
Author

I have yosys 0.39 but I'm not sure of the version of eqy I'm getting. I don't see any option to print out a version number from eqy. Is there a way?

@maliberty
Copy link
Author

bug.zip

@jix
Copy link
Member

jix commented Mar 19, 2024

There is no separate versioning of EQY and currently no way to print a version number. If you get EQY from tabby-cad or the oss-cad-suite you always get matching versions, if you get EQY from this repository, it now has tags matching the compatible yosys version, i.e. the yosys-0.39 tag would be the compatible EQY version. With that, your latest bug.zip passes the equivalence check for me.

@maliberty
Copy link
Author

Thanks for the confirmation that it works for you. I suggest you add a version number to eqy so that it is possible to verify it matches yosys (I expect I have some mix but can't tell). I'm getting eqy from /usr/local/bin and yosys from ORFS.

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

No branches or pull requests

3 participants