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

GHDL synth internal state inconsistent with Yosys state #137

Open
rlee287 opened this issue Nov 2, 2020 · 6 comments
Open

GHDL synth internal state inconsistent with Yosys state #137

rlee287 opened this issue Nov 2, 2020 · 6 comments

Comments

@rlee287
Copy link
Contributor

rlee287 commented Nov 2, 2020

(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
Copy link
Member

tgingold commented Nov 4, 2020 via email

@eine
Copy link
Contributor

eine commented Nov 4, 2020

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
Copy link
Member

tgingold commented Nov 5, 2020 via email

@rlee287
Copy link
Contributor Author

rlee287 commented Nov 5, 2020

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
Copy link
Member

tgingold commented Nov 5, 2020 via email

@rlee287
Copy link
Contributor Author

rlee287 commented Nov 6, 2020

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.

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

No branches or pull requests

3 participants