YosysHQ / arachne-pnr

Place and route tool for FPGAs
MIT License
413 stars 73 forks source link

fatal error: $_TBUF_ gate must drive only one top-level output or inout port #112

Open whitequark opened 6 years ago

whitequark commented 6 years ago

Repro: glasgow_of2ssfk4.zip

daveshah1 commented 6 years ago

This a simplification of the given code for analysis purposes which also triggers the bug.

Verilog

module top(inout x, output y);
assign en = 1'b1;
assign x = en ? 1'b0 : 1'bz;
assign y = en ? x : 1'b0;
endmodule

BLIF

.model top
.inputs
.outputs x y
.names $false
.names $true
1
.names $undef
.gate $_TBUF_ A=$false E=$true Y=x
.attr src "tristate.v:3"
.names x y
1 1
.end
daveshah1 commented 6 years ago

I suppose the question is whether connecting a tristate IO directly to another output using a .names in the BLIF is legal or not (I think it probably is, but I can also see problems with that). Do you have any thoughts @cliffordwolf ?

daveshah1 commented 6 years ago

I suppose arguably arachne-pnr should not be removing .names buffers so early perhaps?

cliffordwolf commented 6 years ago

I suppose the question is whether connecting a tristate IO directly to another output using a .names in the BLIF is legal or not (I think it probably is, but I can also see problems with that).

I think it is perfectly legal wrt the BLIF file format.

Re your simplified example @daveshah1: Obviously that tbuf should have been optimized away. :) I've now added that to git head of yosys. But of course that now "breaks" your example.

daveshah1 commented 6 years ago

@whitequark the Verilog in your test case is also "fixed" by the new Yosys optimisation (my example was deliberately chosen to be as close as possible to your code). But the BLIF MCVE still stands.

I think the solution to this will involve slightly different handling of .names, so it can tell that the TBUF is only directly connected to one pin; and also through an input buffer to another output.

whitequark commented 6 years ago

@cliffordwolf Thanks! @daveshah1 Yes. Your reasoning about .names is correct. It's what I alluded to in the "degenerate case" in https://github.com/cseed/arachne-pnr/pull/110#issue-185852514; there are actually some non-degenerate ones that exhibit the same issue.