YosysHQ / yosys

Yosys Open SYnthesis Suite
https://yosyshq.net/yosys/
ISC License
3.31k stars 860 forks source link

EDIF: add support for different flavors of bit indexing #568

Open spakin opened 6 years ago

spakin commented 6 years ago

The EDIF code produced by write_edif is incorrectly renumbering the bits in multi-bit values.

Steps to reproduce the issue

  1. Write Verilog code such as the following with inputs/output containing multiple bits:
module badness (in, out);
   input [1:0] in;
   output [1:0] out;

   assign out[0] = ~in[0];
   assign out[1] = in[1];
endmodule
  1. Generate EDIF code:
    yosys> read_verilog badness.v 
    yosys> write_edif badness.edif

I'm using Yosys 83631555dd5d57e2a3d3b7175f77baf0ad67ccd9 (2018-06-11):

$ yosys -V
Yosys 0.7+596 (git sha1 83631555, clang 6.0.0 -fPIC -Os)

Expected behavior

I would have expected an EDIF net to associate in[0] with port 0 of in and with the input to an inverter and another net to associate in[1] with port 1 of in and with port 1 of out, as in the following EDIF code:

(net (rename id00003 "in[0]") (joined
  (portRef (member in 0))
  (portRef A (instanceRef id00002))
))
(net (rename id00004 "in[1]") (joined
  (portRef (member in 1))
  (portRef (member out 1))
))
(net (rename id00005 "$not$badness.v:5$1_Y") (joined
  (portRef (member out 0))
  (portRef Y (instanceRef id00002))
))

In fact, older versions of Yosys get this right. An example of a good Yosys is the 0.7 release (61f681162754fa170494e567f1a1a9ae2d17eb69):

$ yosys -V
Yosys 0.7 (git sha1 61f6811, gcc 6.2.0-11ubuntu1 -O2 -fdebug-prefix-map=/build/yosys-OIL3SR/yosys-0.7=. -fstack-protector-strong -fPIC -Os)

Actual behavior

What actually happens is that an EDIF net associates port 1 of in with the input to the inverter (and with in[0]). Another EDIF net associates port 0 of in directly with port 0 of out (and with in[1])—but no inverter—as in the following EDIF code:

(net (rename id00003 "in[1]") (joined
  (portRef (member in 0))
  (portRef (member out 0))
))
(net (rename id00004 "in[0]") (joined
  (portRef (member in 1))
  (portRef A (instanceRef id00002))
))
(net (rename id00005 "$not$badness.v:5$1_Y") (joined
  (portRef (member out 1))
  (portRef Y (instanceRef id00002))
))

That is, the comment strings are correct, but the actual portRefs are not. This issue may be similar to #315, but I'm not positive.

cliffordwolf commented 5 years ago

See https://github.com/YosysHQ/yosys/pull/830#issuecomment-468809795 and http://svn.clifford.at/handicraft/2019/vivadoedif: The current behavior is to always use array index 0 for the MSB array element, and to use the name Verilog uses for the bit in the rename clause. This mirrors the exact behavior of Xilinx Vivado.

We can add support for other "EDIF flavors", but for that we would need to know what other tool you want us to emulate and what the exact behavior of that tool is.

spakin commented 5 years ago

We can add support for other "EDIF flavors", but for that we would need to know what other tool you want us to emulate and what the exact behavior of that tool is.

It's my own, custom tool. With some effort, I managed to get it to work with array index 0 for the MSB array element and the name Verilog uses for the bit in the rename clause.

Giving the user the choice of flavors you laid out in https://github.com/YosysHQ/yosys/pull/830#issuecomment-468453251 sounds reasonable to me.

cliffordwolf commented 5 years ago

Re-opening as "add supports for flavors wrt numbering of vector bits".

hoglet67 commented 1 year ago

I'm putting together a custom flow that uses Yosys plus the Microchip/Atmel CPLD fitter to target the Microchip/Atmel ATF15xx CPLD family. I have this working well enough to show this is both possible and useful. However, I have fallen foul of this EDIF bit ordering issue, because the Atmel fitter uses the opposite convention to Vivado. i.e. it assumes port array index 0 is the LSB of the array.

I'm just posting to say that I would find the -lsbidx option very useful indeed.

All the work I'm doing on this is in the following git repository: https://github.com/hoglet67/atf15xx_yosys

A test case for this issue is just a simple two-bit counter: https://github.com/hoglet67/atf15xx_yosys/blob/master/examples/counter/counter.v

The fitter equations show currently show counter bits 0 and 1 reversed:

counter_1.D = !counter_1.Q;
counter_0.D = counter_1.Q $ counter_0.Q;
counter_1.C = clock;
counter_0.C = clock;

i.e. counter_1 acts as the LSB and counter_0 acts as the MSB

( $ is the XOR operator - the equations are in CUPL syntax )

hoglet67 commented 1 year ago

I have one futher comment about this....

I'm using an eval copy of the Mentor RTL synthesis tool to understand what the Atmel fitter expects in the EDIF file. What I see in the above two-bit ounter example is two things:

1, A bit reveral in the port ref, just like Yosys does.

     (net (rename counter_1_ "counter(1)")
      (joined
       (portRef (member counter 0))
       (portRef Q (instanceRef io_counter_1_ ))))
  1. Another bit reveral in the interface (via the rename), which yosys does't do
    (interface
     (port clock (direction INPUT))
     (port (array (rename counter "counter(1:0)") 2 )(direction OUTPUT)))

    I think these are acting together to cancel each other out.

peterzieba commented 1 year ago

The ATF150x series devices are very interesting parts for anyone that wants to use a reasonably capable 5V CPLD device, especially considering they are still active / haven't been NRND'd. They're also programmable over JTAG in OpenOCD. I'd love to see Yosys have better support for generating the EDIF necessary as that's the largest bottleneck to using these outside of the vendor tools.

Reliance on the Vendor fitters (which otherwise accept EDIF from Yosys), is otherwise not a significant problem as the fitter is a command line tool that runs well from Wine.

whitequark commented 1 year ago

@hoglet67 Have you seen prjbureau?

whitequark commented 1 year ago

@peterzieba I think we're better off ingesting RTLIL and generating EDIF ourselves, to be honest.

peterzieba commented 1 year ago

With https://github.com/YosysHQ/yosys/pull/1489 being merged, it is now possible to do write_edif -lsbidx