[myhdl-list] Some comments on the VHDL code that MyHDL generates

[Angel E. <ang...@gm...>]

In my quest to somehow start using MyHDL for my work, I've been
playing around with it, trying to create a small module in MyHDL from
scratch.

Since Christopher Felton suggested that I may be able to generate VHDL
in a way that my colleagues would not even notice, I created a small,
simple module and looked into the VHDL code that MyHDL generates.

My first impression is that the code is in fact quite similar to what
I would have written, which is nice. However I have a few comments:

1. "type" definitions are not properly indented. For example:
    The following MyHDL:

~~~
t_State = enum('INIT', 'WORKING')
~~~

becomes the following VHDL:

~~~
package pck_sync_module is

    type t_enum_t_State_1 is (
    INIT,
    WORKING
);

end package pck_sync_module;
~~~

Here you can see that the lines below the type keyword itself
(starting with "INIT") are not properly indented.
Instead it should be (IMHO):

~~~
package pck_sync_module is

    type t_enum_t_State_1 is (
        INIT,
        WORKING
    );
~~~

2. I prefer to have the declarations between the "architecture" header
and the begin keyword to be indented. I also prefer to indent the
architecture contents (everything between begin/end). This is nothing
major, and obviously open to debate, but I think it looks clearer. For
example, I'd prefer:

~~~
architecture MyHDL of sync_module is

    signal s_2pps_counter: unsigned(2 downto 0);
    signal pulse_sync: std_logic;

begin
~~~

rather than the current:

~~~
architecture MyHDL of sync_module is

signal s_2pps_counter: unsigned(2 downto 0);
signal pulse_sync: std_logic;

begin
~~~

3. For some reason MyHDL added 3 empty lines between the architecture
"begin" and the first line of code in it:

~~~
architecture MyHDL of sync_module is

signal s_2pps_counter: unsigned(2 downto 0);
signal pulse_sync: std_logic;
signal first_pulse: std_logic;
signal pulse_state: t_enum_t_State_1;

begin



-- Main process
SYNC_MODULE_P_PULSE: process (clk, rst) is
    variable pulse_state3: t_enum_t_State_1;
begin
    if (rst = '1') then
~~~

See the 3 blank lines betwen begin and the first comment. I would
think that one single blank would be enough.

4. I tried to use docstrings to add comments to the generated VHDL, so
that if someone where to inspect it it would be easier to understand
without needing to go to my source MyHDL code. This works in most
cases but there are a few things that could be improved (IMHO) and
some things that I was just not able to do:

    4.1. If I add a docstring to a function that contains an @always
block, MyHDL generates a corresponding "top entity". I expected to see
the docstring above the entity declaration. Instead MyHDL places the
comment between the entity declaration and the architecture
declaration, which I think it is a bit weird. I'd rather see the
documentation as close to the top of the file as possible (i.e. right
above the entity keyword, so that I can document it).

    4.2. I am unable to use a docstring to comment an enum declaration
(or any other signal declaration for that matter). This seems to be
due to the fact that this is done outside of the instance generators.
I think this is important, particularly for enums which are used for
state machines. Maybe all MyHDL objects (signals, intbv, etc) could
have a "comment" or "doc" field that you could set to include some
info about them on the generated VHDL / Verilog code?

    4.3. Is does not seem possible to add a "top level" file comment
via a docstring. I guess this is not very important right now since
there is not a one to one mapping between MyHDL .py files and the
generated .vhd file.

Please note that this is in no way a criticism of MyHDL. I know that
the main goal of MyHDL is not to generate human readable VHDL code.
However I think these small details could improve the generated code
substantially. Items 1 to 3 could be fixed on the user's end by
running the generated code through an automated VHDL indenter tool but
unfortunately I don't know of any good, free indenter that is also
easy to automate (perhaps emacs in batch mode?).

I have some more comments which are not related to the actual code
that MyHDL generates but I'll leave that for another email.

Cheers,

Angel