verilog-to-routing / vtr-verilog-to-routing

Verilog to Routing -- Open Source CAD Flow for FPGA Research
https://verilogtorouting.org
Other
1.02k stars 396 forks source link

Improve VPR error message when circuit contains multi-driven nets #330

Open kmurray opened 6 years ago

kmurray commented 6 years ago

Expected Behaviour

For this input netlist:

.model top
.inputs a b
.outputs c

.names a b c
11 1

.names a c
1 1

.end

VPR should provide a useful error message that the net c (incorrectly) has multiple drivers.

Current Behaviour

Currently this fails an internal consistency check within the netlist data structures:

VPR FPGA Placement and Routing.
Version: 8.0.0-dev+51b49fa
Revision: 51b49fa
Compiled: 2018-04-21T12:54:54 (release build)
Compiler: GNU 5.4.1 on Linux-4.4.0-112-generic x86_64
University of Toronto
vtr-users@googlegroups.com
This is free open source code under MIT license.

VPR was run with the following command-line:
vpr /project/trees/vtr/vtr_flow/arch/timing/EArch.xml error.blif

Using up to 1 parallel worker(s)

Architecture file: /project/trees/vtr/vtr_flow/arch/timing/EArch.xml
Circuit name: error

Building complex block graph.
Warning 1: io[0].clock[0] unconnected pin in architecture.
Load circuit
Load circuit took 0.00 seconds
Error 1: 
Type: Unrecognized Error
File: /project/trees/vtr/vpr/src/base/netlist.tpp
Line: 1477
Message: Inconsistent block data sizes

Which doesn't tell the user what the problem is.

Possible Solution

Produce a helpful error message such as:

Error 1: 
Type: Netlist Error
File: error.blif
Line: 8
Message: Net 'c' already has a driver (multi-driven nets are not supported)

The checks for this should occur whenever a new driver is created during atom netlist construction (i.e. in vpr/src/base/read_blif.cpp), not when the netlist data structures are being verified.

Steps to Reproduce (for bugs)

  1. Run the above netlist on any standard VTR architecture (e.g. vtr_flow/timing/EArch.xml)

Context

This is a potential common mistake for which VPR has enough context information to give a useful error message, and should do so.

Your Environment

benreynwar commented 6 years ago

If all the drivers have the option of driving a 'Z' then the blif should be legal shouldn't it? I'll implement a check that complains if there's multiple drivers and they're not all able to drive a 'Z'. Edit: I'm just not going to worry about 'Z'. I don't understand this stuff well yet, so I'm going to assume implementing it the way described in the issue is the correct way.

kmurray commented 6 years ago

You are correct that driving a high-impedance 'Z' (i.e. disconnecting the driver) would be electrically legal.

However internally VPR assumes that a net only ever have a single driver. If a net has multiple drivers it becomes much less clear how it should be analyzed. For instance, its not clear what the delay of a connection on a multi-driver net should be.

qaarah commented 6 years ago

I have kind of same problem where I get the same driver signal of multiple inputs. For example, inputs io_10_17_0 .outputs io_0_5_0 io_0_11_1 io_4_0_1 io_5_0_1 io_5_17_1 io_5_17_0 io_6_0_0 io_7_0_0 io_8_0_0 io_8_17_0 io_8_17_1 io_9_0_1 io_13_1_1 io_13_6_0 io_13_7_1 io_13_11_1 io_13_15_1 .names [0] io_13_6_0 1 1 .names [3] io_13_15_1 1 1 .names io_10_17_0 [1] 1 1 .names io_10_17_0 [1] 1 1 .names io_10_17_0 [1] 1 1 .names io_10_17_0 [1] 1 1 .names [1241] [155] 1 1 .names [1243] [156] 1 1 .names [1245] [157] 1 1 .names [1247] [158] 1 1 .names [1249] [159] 1 1 I am using the VPR 8. Due to this error, I have some errors in abc as well. Thanks