ghdl / ghdl-yosys-plugin

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

Handling of various Id_Const_* module IDs missing #47

Closed tmeissner closed 4 years ago

tmeissner commented 5 years ago

There are a bunch of Const_* IDs defined in ghdlsynt_gates.h which aren't handled in ghdlsynth-beta:

tmeissner commented 5 years ago

Handling of Id_Const_Bit (https://github.com/tgingold/ghdlsynth-beta/commit/1af53cad6d6c8b7fd3ad8241ee2537e0a9217276) and Id_Const_Log (https://github.com/tgingold/ghdlsynth-beta/commit/366d22bac43caec422b0628c08b5986b0199e4bf) is now implemented by @tgingold :-)

pepijndevos commented 4 years ago

I've added Id_Const_SB32 in #61.

It would actually be good to inventory what is there and what is missing more broadly. Maybe this issue could be renamed and extended to include all of ghdlsynth_gates.h?

Here is a script that gets the desired output I think:

cd ghdl
grep -o 'Id_[a-zA-Z0-9_]*' src/synth/ghdlsynth_gates.h | sort -u > ghdlids.txt
cd ../ghdlsynth-beta 
grep -o 'Id_[a-zA-Z0-9_]*' src/ghdl.cc | sort -u > ghdlids.txt
cd ..
diff -u  ghdl/ghdlids.txt ghdlsynth-beta/ghdlids.txt

Which currently gives

I think it's important to cover all these that can realistically show up in synthesized code before thinking about possibly upstreaming this plugin into Yosys, which I'd very much like to see happen.

The upside is that some of these are probably pretty easy to add, and some will not be used at all. So there are only a few that actually require some work to do correctly. Maybe @tgingold can give some input on which ones are used and which one require nontrivial work?

tgingold commented 4 years ago

Those related to memories must not reach ghdl.cc Some (like Rol, Ror) have to be expanded before reaching ghdl.cc

So the remaining gates are the Id_Const gates (easy to add), and div/rem/mod. I am not sure that the div/rem/mod of verilog (when it exists) match the one from vhdl...

pepijndevos commented 4 years ago

It seems Id_Const_UB64 and Id_Const_UL64 are unused.

   --  Should we keep them ?
   pragma Unreferenced (Id_Const_UB64, Id_Const_UL64);
pepijndevos commented 4 years ago

It appears there is no true remainder operator in Verilog or Yosys. So we can't add that at this time. It might be worth checking with Yosys people if this can be added. I suspect there is not a huge demand.

So with #72 I think we're covering all the IDs that we can/should?

pepijndevos commented 4 years ago

Filed a Yosys issue: https://github.com/YosysHQ/yosys/issues/1523