Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Implement a VHDL backend #122

Draft
wants to merge 100 commits into
base: master
Choose a base branch
from
Draft

Conversation

rlee287
Copy link
Contributor

@rlee287 rlee287 commented May 22, 2020

This will be a VHDL backend from #112, and I will be using this PR to track what has been finished and what still needs work.

  • Adjust command line options as necessary
  • Ensure identifiers are valid VHDL identifiers
    • Keyword collisions (merely updating the keyword list?)
    • Handling $
    • Extended identifiers (now used when syntactically needed and to avoid collisions with internally generated names)
  • Adjust Makefiles
  • Get GHDL version to put into generated output (see Create a version header to export version strings to C ghdl#1448)
  • Technical debt (as outlined in comment below)
  • Write tests for the backend
  • Figure out how to dump attributes cleanly
  • Better handling of sensitivity lists (helper function to extract these from SigSpecs?)
  • Dump memory cells correctly (not just type declarations in dump_memory)
  • Use qualified expressions when aggregates have ambiguous array type
  • Other high-level issues that may appear

On extended identifiers: #112 suggests always using these, but user-provided names (e.g. port names) should be preserved when possible, and IEEE-1076-2008 15.4.3 states that "Moreover, every extended identifier is distinct from any basic identifier."

Functions that need to be changed to output VHDL syntax (note that this checklist only deals with syntax generation in that function and not any other things that may need adjustment):

  • void reset_auto_counter(RTLIL::Module *module)
  • std::string next_auto_id()
  • std::string id(RTLIL::IdString internal_id, bool may_rename = true)
  • bool is_reg_wire(RTLIL::SigSpec sig, std::string &reg_name)
  • void dump_const(std::ostream &f, const RTLIL::Const &data, int width = -1, int offset = 0, bool no_decimal = false, bool escape_comment = false)
  • void dump_reg_init(std::ostream &f, SigSpec sig)
  • void dump_sigchunk(std::ostream &f, const RTLIL::SigChunk &chunk, bool no_decimal = false)
  • void dump_sigspec(std::ostream &f, const RTLIL::SigSpec &sig)
  • void dump_attributes(std::ostream &f, std::string indent, dict<RTLIL::IdString, RTLIL::Const> &attributes, char term = '\n', bool modattr = false, bool regattr = false, bool as_comment = false)
  • void dump_wire(std::ostream &f, std::string indent, RTLIL::Wire *wire)
  • void dump_memory_types(std::ostream &f, std::string indent, Mem &mem)
  • void dump_memory(????, std::string indent, Mem &mem) (split off on my own, signature to be determined when implemented
  • void dump_cell_expr_port(std::ostream &f, RTLIL::Cell *cell, std::string port, bool gen_signed = true)
  • std::string cellname(RTLIL::Cell *cell)
  • void dump_cell_expr_uniop(std::ostream &f, std::string indent, RTLIL::Cell *cell, std::string op)
  • void dump_cell_expr_binop(std::ostream &f, std::string indent, RTLIL::Cell *cell, std::string op)
  • bool dump_cell_expr(std::ostream &f, std::string indent, RTLIL::Cell *cell)
  • void dump_cell(std::ostream &f, std::string indent, RTLIL::Cell *cell)
  • void dump_conn(std::ostream &f, std::string indent, const RTLIL::SigSpec &left, const RTLIL::SigSpec &right)
  • void dump_proc_switch(std::ostream &f, std::string indent, RTLIL::SwitchRule *sw)
  • void dump_case_body(std::ostream &f, std::string indent, RTLIL::CaseRule *cs, bool omit_trailing_begin = false)
  • void dump_process(std::ostream &f, std::string indent, RTLIL::Process *proc, bool find_regs = false)
  • void dump_module(std::ostream &f, std::string indent, RTLIL::Module *module)
  • void VHDLBackend::execute(std::ostream *&f, std::string filename, std::vector<std::string> args, RTLIL::Design *design) YS_OVERRIDE

@rlee287 rlee287 marked this pull request as draft May 22, 2020 00:39
@rlee287
Copy link
Contributor Author

rlee287 commented May 23, 2020

Documentation of major design decisions I am making (feel free to contest these in followup comments):

  • All signals and variables will be std_logic_vectors, with casts to unsigned and signed as necessary for arithmetic operations. (This mirrors the current behavior of ghdl --synth.)
  • As all signals will be a vector type with fixed width, -decimal (which does not include widths in constants) does not make sense and will be removed.
  • It may be possible to represent RTLIL::Process objects exactly in VHDL, so that warning can be removed once this is confirmed.
  • $specify* type cells ($specify2, $specify3, and $specrule cells to be more specific) are not translated. A warning is printed, and a comment is left behind in the generated code indicating that the cell was not translated.
  • As per a conversation in the Gitter chat, Verilog (and RTLIL by extension) does not differentiate between arrays of 1 element and singular values (i.e. STD_LOGIC_VECTOR (0 downto 0) and STD_LOGIC), so the backend will dump using STD_LOGIC. A side effect of this is that vector ports of width 1 will turn into non-vector ports upon export, causing type mismatches.
  • Include a --std08 option that uses some elements of the VHDL-08 in order to make the generated code more human-readable.

@rlee287
Copy link
Contributor Author

rlee287 commented May 24, 2020

Technical debt to cleanup before finishing:

  • Everything labelled TODO in the code
  • Array types are generated with the name array_type_(width), so the signal renaming logic should be changed to check for collisions with this
  • Check if there is a reason for stringf("string_constant") and remove the wrapping function call if there isn't one
  • Check if is_reg_wire and related are actually required for VHDL (since the reg/wire distinction does not exist in VHDL), and possibly using is_reg_wire for more informative signal names
  • Signal concatenation on the LHS of an expression most likely assumes VHDL-2008 and probably generates invalid output for multi-chunk SigSpecs when -std08 is not specified

dump_cell_expr_binop may need adjustments because of STD_LOGIC_VECTOR vs UNSIGNED/SIGNED
I am not marking this as done until I better understand the context in which this function is called
@Xiretza
Copy link
Contributor

Xiretza commented May 30, 2020

FYI, VlogHammer might come in useful for testing this. The patch below is an attempt at adapting it for ghdl (to be run with make YOSYS_MODE=ghdl GHDL_MODULE=/path/to/ghdl.so syn_yosys), but there's not enough functionality to properly test it yet:

diff --git a/scripts/syn_yosys.sh b/scripts/syn_yosys.sh
index cb2244f..5276c8d 100644
--- a/scripts/syn_yosys.sh
+++ b/scripts/syn_yosys.sh
@@ -55,6 +55,9 @@ case "$YOSYS_MODE" in
        7|minimal)
                yosys -q -l synth.log -b 'verilog -noattr' -o synth.v \
                      -p 'hierarchy; proc' ../../rtl/$job.v ;;
+       8|ghdl)
+               yosys -m "${GHDL_MODULE:-ghdl}" -q -l synth.log -b 'verilog -noattr' -o synth.v \
+                     -p 'write_vhdl -noattr synth.vhd; design -reset; ghdl synth.vhd -e' ../../rtl/$job.v ;;
        *)
                echo "Unsupported mode: $YOSYS_MODE" >&2
                exit 1 ;;

@rlee287
Copy link
Contributor Author

rlee287 commented Jun 4, 2020

It turns out that more of the id related functions need updating, as the autogenerated ids need to be changed to be valid identifiers (without underscores at the beginning or end). I will need to decide if I want to imitate ghdl --synth and also append _o or _q to internal signal names. (The checklist at the top will be updated momentarily.)

@rlee287
Copy link
Contributor Author

rlee287 commented Dec 17, 2020

Finally getting around to resuming development and copying over the Mem refactoring, but the requirement for placing signal declarations at the beginning for VHDL means that the actual memory dumping is going to be more complicated.

src/vhdl_backend.cc Outdated Show resolved Hide resolved
@JimLewis
Copy link

Not quite. "=" is a string operation. "?=" is a hardware operation.
"=" like character matches like character
"?=" 0 matches L, 1 matches H, '-' matches anything. U results in U (except with -), everything else is X.
For more see the LRM.

@JimLewis
Copy link

JimLewis commented Dec 17, 2020

Had a long discussion with a Verilog person about what value to assign in the default assignment within a case statement. It actually helped me understand why I should keep doing what I had always been doing. Which is when assigning to a signal, use 'X" as don't care. For example:

   case slv2 is 
     when "00" => ...
     when "01" => ...
     when "10" => ...
     when "11" => ...
     when others => Y <= "XXXX" ;
  end case ; 

If instead, Y were assigned to "----", then the expression, Y ?= "0000" would be true when the don't care happened.
OTOH, interesting things would happen anyway if the condition "11" were not covered and it actually happened in the real hardware.

rlee287 and others added 19 commits December 20, 2020 14:45
…ation happens

All uses of dump_cell_expr_port are on the RHS of an expression
TODO: concatenation handling may be broken outside of VHDL-2008 mode; handle this later
* In std08 mode, avoid an explicit `='1'` for non-PSL asserts
* Update the log_experiment call to reflect the use of non-PSL asserts
Actually dumping memories though is currently an unresolved thing
Qualified expression part will probably be moved into dump_sigspec
@kammoh
Copy link

kammoh commented Feb 2, 2022

Hi!
Great initiative @rlee287!
I saw there was no activity on this PR so I went a little further with it:

https://github.com/kammoh/ghdl-yosys-plugin/tree/vhdl_backend

I'm now able to run post-synthesis simulation using Yosys synth/synth_xilinx netlist synthesis and GHDL simulation.
I've only tested a couple of rather small designs and it passes verification.
I merged in master and made sure works with the latest yosys.

Please let me know your thoughts about those changes.

Would you still be interested in working on this feature @rlee287? I'd love to provide any help, as this is a feature I direly need at the moment. Otherwise please let me know if I should make a new PR.

Thank you!

@rlee287
Copy link
Contributor Author

rlee287 commented Feb 4, 2022

Unfortunately, I haven't had time to continue working on this, so I appreciate you stepping up to continue where I left off. I hope you'll be able to see this through and merely ask that you continue documenting design choices and similar things (e.g. the last commit of Yosys that this part of the plugin was based on, in order to track whether it accounts for updates to any of the Yosys cell libs, etc.), similar to how I documented such things when I was working on this earlier.

If you do complete this and get a PR of yours merged with this functionality, I'll be happy to close this PR. Thanks again for picking up where I left off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants