ghdl / ghdl-yosys-plugin

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

GHDL synth internal state inconsistent with Yosys state #137

Open rlee287 opened 3 years ago

rlee287 commented 3 years ago

(The title of this issue refers to my best guess at explaining the behavior below, as I have not dived into how the GHDL synthesis code actually works. Please let me know if some other title would be more appropriate.)

The sections below describe behavior explainable by the GHDL synthesis plugin maintaining too much independent state, but please let me know if any of the below is actually intended behavior.

(This is related to the title of #74, but most of the comments under it appear to be a separate discussion about GPL licensing.)

Files used in below examples

addmodule_verilog.v (not actually used below but a useful Verilog reference that should do the same thing)

`default_nettype none
module addmodule (
        input wire  [7:0] a,
        input wire  [7:0] b,
        output wire [7:0] sum_val
);
        assign sum_val = a + b;
endmodule

addmodule_vhdl.vhd

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;

entity addmodule is
    port (
        a: in UNSIGNED(7 downto 0);
        b: in UNSIGNED(7 downto 0);
        sum_val: out UNSIGNED(7 downto 0)
    );
end entity;

architecture Behavioral of addmodule is
begin
    sum_val <= a + b;
end Behavioral;

addmodule_vhdl_other.vhd

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;

entity addmodule is
    port (
        a: in SIGNED(7 downto 0);
        b: in SIGNED(7 downto 0);
        sum_val: out SIGNED(7 downto 0)
    );
end entity;

architecture Behavioral of addmodule is
begin
    sum_val <= a + b;
end Behavioral;

Broken state after trying to import nonexistent file

The second ghdl command should not result in an error.

$ yosys -Q -m ghdl

yosys> ghdl nonexistent.vhd -e

1. Executing GHDL.
error: cannot open nonexistent.vhd
ERROR: vhdl import failed.

yosys> ghdl addmodule_vhdl.vhd -e

2. Executing GHDL.
ERROR: vhdl import failed.

yosys> exit

End of script. Logfile hash: 849a1f940b, CPU: user 0.02s system 0.01s, MEM: 14.75 MB peak
Yosys 0.9+3664 (git sha1 d9af3cadf, ccache clang 10.0.0-4ubuntu1 -fPIC -Os)
Time spent: 100% 2x ghdl (0 sec)

Spurious warnings after module rename

Since the imported module has been renamed, there should not be a name conflict when importing it again. (I suspect that a failure to track renames has wider repercussions in resolving file dependencies, but I have not had the chance to explore this further or generate a MCVE for a file dependency type situation.)

$ yosys -Q -m ghdl

yosys> ghdl addmodule_vhdl.vhd -e

1. Executing GHDL.
note: top entity is "addmodule"
Importing module addmodule.

yosys> rename addmodule addmodule_backup
Renaming module \addmodule to \addmodule_backup.

yosys> ghdl addmodule_vhdl.vhd -e

2. Executing GHDL.
addmodule_vhdl.vhd:1:1:warning: redefinition of a library unit in same design file: [-Wlibrary]
addmodule_vhdl.vhd:1:1:warning: entity "addmodule" defined at line 5:8 is now entity "addmodule" [-Wlibrary]
addmodule_vhdl.vhd:13:1:warning: redefinition of a library unit in same design file: [-Wlibrary]
addmodule_vhdl.vhd:13:1:warning: architecture "behavioral" of "addmodule" defined at line 13:14 is now architecture "behavioral" of "addmodule" [-Wlibrary]
note: top entity is "addmodule"
Importing module addmodule.

yosys> stat

3. Printing statistics.

=== addmodule ===

   Number of wires:                  4
   Number of wire bits:             32
   Number of public wires:           3
   Number of public wire bits:      24
   Number of memories:               0
   Number of memory bits:            0
   Number of processes:              0
   Number of cells:                  1
     $add                            1

=== addmodule_backup ===

   Number of wires:                  4
   Number of wire bits:             32
   Number of public wires:           3
   Number of public wire bits:      24
   Number of memories:               0
   Number of memory bits:            0
   Number of processes:              0
   Number of cells:                  1
     $add                            1

yosys> exit
End of script. Logfile hash: 2e9bf4b784, CPU: user 0.08s system 0.00s, MEM: 15.72 MB peak
Yosys 0.9+3664 (git sha1 d9af3cadf, ccache clang 10.0.0-4ubuntu1 -fPIC -Os)
Time spent: 98% 2x ghdl (0 sec), 0% 1x stat (0 sec), ...

Another example with a different file but same module:

$ yosys -Q -m ghdl

yosys> ghdl addmodule_vhdl_other.vhd -e

1. Executing GHDL.
note: top entity is "addmodule"
Importing module addmodule.

yosys> rename addmodule addmodule_signed
Renaming module \addmodule to \addmodule_signed.

yosys> ghdl addmodule_vhdl.vhd -e

2. Executing GHDL.
addmodule_vhdl.vhd:1:1:warning: entity "addmodule" was also defined in file "addmodule_vhdl_other.vhd" [-Wlibrary]
note: top entity is "addmodule"
Importing module addmodule.

yosys> stat

3. Printing statistics.

=== addmodule ===

   Number of wires:                  4
   Number of wire bits:             32
   Number of public wires:           3
   Number of public wire bits:      24
   Number of memories:               0
   Number of memory bits:            0
   Number of processes:              0
   Number of cells:                  1
     $add                            1

=== addmodule_signed ===

   Number of wires:                  4
   Number of wire bits:             32
   Number of public wires:           3
   Number of public wire bits:      24
   Number of memories:               0
   Number of memory bits:            0
   Number of processes:              0
   Number of cells:                  1
     $add                            1

yosys> exit
End of script. Logfile hash: de224ca81e, CPU: user 0.07s system 0.01s, MEM: 15.61 MB peak
Yosys 0.9+3664 (git sha1 d9af3cadf, ccache clang 10.0.0-4ubuntu1 -fPIC -Os)
Time spent: 98% 2x ghdl (0 sec), 1% 1x stat (0 sec), ...
tgingold commented 3 years ago

For the spurious warnings when the same file is re-analyzed, I am not sure what to do. Maybe I won't do anything, as this is not a common case.

Currently, ghdl keeps track of files that have been analyzed. Maybe the 'ghdl' command should start from scratch at each execution.

eine commented 3 years ago

Currently, ghdl keeps track of files that have been analyzed. Maybe the 'ghdl' command should start from scratch at each execution.

Would this impact the possibility for defining different libraries? Currently, I use several ghdl -a calls for adding different sources to multiple libs. Then, in yosys, I tell ghdl to elaborate a unit that depends on those libraries. If the ghdl command started from scratch that feature would be lost, isn't it?

tgingold commented 3 years ago

No, external libraries would be kept.

But files that have been read from a previous ghdl command wouldn't be kept for the next commands.

rlee287 commented 3 years ago

I added another example of the spurious warning, which also occurs when one imports two different files that state the same module name. This could occur in practice, for example, if one file is a rewrite of the other file and the author wishes to formally verify their equivalence using a miter circuit.

tgingold commented 3 years ago

Yes, but this is not spurious. This exactly what the warning is about.

rlee287 commented 3 years ago

I would think that renaming the first module to a new name after importing would make the old name freely available, and that the second module (sharing the old name) would no longer collide with the first module after the rename. (I realize now that I did not make it clear in my comment that there was a rename involved with the example I added afterwards). If having a warning in this situation (with a rename) is an intended feature, then the other header of "Broken state after trying to import nonexistent file" would seem to be fixed by your commit to GHDL, and this issue could be closed after I have time to test the committed change.