ghdl / ghdl-yosys-plugin

VHDL synthesis (based on ghdl)
GNU General Public License v3.0
295 stars 32 forks source link

Added proper components.vhdl with uppercase symbols #156

Closed hackfin closed 2 years ago

hackfin commented 2 years ago

This being the PR related to #155.

SiliconWizard commented 2 years ago

There's an additional modification I did to make the 'DP16KD' primitive work in Yosys/Nextpnr: comment out the 'INIT_DATA' generic parameter for the 'DP16KD' component, as with it, Yosys would yield an error.

I haven't tried the 'PDPW16KD' primitive yet, but I would expect the same.

There may be other things to adjust for full compatibility with Yosys, it would require an exhaustive testing...

Now I made the modification for the time being, but maybe it would be best if Yosys supported this parameter?

tgingold commented 2 years ago

Thanks!

hackfin commented 2 years ago

Now I made the modification for the time being, but maybe it would be best if Yosys supported this parameter?

There should be no issue with string parameters in yosys in general. Do you have a test case to be run online in a container?

SiliconWizard commented 2 years ago

It's not a general issue with string parameters (although there was one with ghdl synth, but I think it has been fixed here: https://github.com/ghdl/ghdl/commit/f75191ef6e013c4efaa8b68eee93e4de47254db9 )

Unrelated here though - it's just that Yosys's own definition for ECP5 block RAM doesn't include this specific 'INIT_DATA' parameter, so Yosys complains about it.

You can check Yosys definition here: https://github.com/YosysHQ/yosys/blob/master/techlibs/ecp5/brams_map.v

It includes the bram_init_9_18_36.vh file, which in turn defines the 'INITVAL_xx' parameters. (The file: https://github.com/YosysHQ/yosys/blob/master/techlibs/ecp5/brams_init.py generates this include file...)

But the 'INIT_DATA' parameter is not defined in Yosys Verilog module definition. It is in 'components.vhdl'. Thus the issue.

Instantiating a DP16KD primitive in VHDL code is all that it takes to reproduce the problem with Yosys.

umarcor commented 2 years ago

FTR, when working on NEORV32, @tmeissner did also find mismatches between the components to be used with GHDL+Yosys or Radiant. Precisely, these are the components we use for GHDL+Yosys: https://github.com/stnolting/neorv32/blob/master/setups/osflow/devices/ice40/sb_ice40_components.vhd. When using Radiant, components are not explicitly required, however, it can be seen that the 256K SPRAM component is not the same:

tmeissner commented 2 years ago

Yep, AFAIK Yosys' ice40 target was developed before Radiant's release, so it uses the component definitions coming with the "old" IceCube2 software, the predecessor of Radiant.