VUnit / vunit

VUnit is a unit testing framework for VHDL/SystemVerilog
http://vunit.github.io/
Other
703 stars 254 forks source link

vhdl_parser.py: components should not be converted to lower-case while instantiating Verilog modules #808

Open krishnan-gopal opened 2 years ago

krishnan-gopal commented 2 years ago

I have a case where a VHDL entity (say vhdl_top) is instantiating a Verilog module with upper-case letters (say VERILOG_MODULE):

VERILOG:

module VERILOG_MODULE (
.....
endmodule

The component declaration and instantiation of this would be done in the vhdl_top:

VHDL:

component VERILOG_MODULE
...
end component;

inst_module: VERILOG_MODULE
....

The vhdl_parser.py in VUnit is parsing the vhdl_top, and converts the component declaration of VERILOG_MODULE to lower-case assuming that it is a VHDL component. However, the Verilog module name remains unconverted in case. Therefore, we get warnings from the VUnit parser that look like this:

WARNING - vhdl_top.vhd: failed to find a primary design unit 'verilog_module' in library 'verilog_lib'

Since Verilog is case-sensitive, this creates a problem for VUnit to find the right dependencies and results in a wrong compilation order.

asicnet commented 2 years ago

Hello A suggested solution would be: When parsing the Verilog module, also convert the module name to lowercase and thus resolve the dependencies. Since Verilog is case sensitive, if the module exists in Verilog with different "same" names, an error will occur anyway when instantiating the module in VHDL. A count in a dict module[name]+=1 and an error message if module[name]>1 would be sufficient and in case of no error the dependencies would be correct

BR Helmut

LarsAsplund commented 2 years ago

As a workaround you can either change the Verilog name to lowercase or you can use the add_dependency_on method to enforce a correct compile_order.

krishnan-gopal commented 2 years ago

Changing the verilog name to lowercase:

add_dependency_on method:

We (@asicnet and me) can make an attempt with a pull-request.

LarsAsplund commented 2 years ago

The problem is that we have a potential dependency error today and a potential "same name" error with your suggested solution (I recall people actually using the case sensitivity in Verilog deliberately, it's part of their naming strategy). Either way we will have a situation where problems may occur and have to be managed manually in the run script. Is there a way to handle such special cases through FuseSOC or will it always produce a standard, non-configurable, run script?

krishnan-gopal commented 2 years ago

There is no way to specify in FuseSOC to make a single file dependent on another file(s). However, we can modify the VUnit backend in Edalize to 'enforce' the compile order to follow the order that is specified in the core-files. That way we would have each file in the generated run.py specified with an 'add_dependency_on' over the last file. Of course, this would deprive the user of VUnit's dependency evaluation

@asicnet : any comments on this ?

LarsAsplund commented 2 years ago

I cannot speak for FuseSOC but in general there will always be cases not supported by the standard flow of tools and there is a need for the frontend tools to provide a low-level interface to the backend tools. A similar example from VUnit is that the "standard flow" of doesn't cover all the capabilities of the backend simulators. That is why we have methods such as set_sim_option and set_compile_option that allow you to interface the simulator directly.

asicnet commented 2 years ago

We were able to further isolate the problem and have found a workaround.

  1. The lower case problem. Insert a dummy modul in the original file.

    module pf_dpsram_rccs_dummy(); endmodule

    // PF_DPSRAM_RCCS module PF_DPSRAM_RCCS( // Inputs A_ADDR, ........

And add the dummy in the vhdl file like

library polarfire; ..... dummy : entity polarfire.pf_dpsram_rccs_dummy;

now vunit can resolve the dependencies

  1. The component instantiation is not supported!

    use polarfire.PF_DPSRAM_RCCS; component PF_DPSRAM_RCCS port (....); end component; .... inRam_0 : PF_DPSRAM_RCCS port map( ...);

and also not !

use polarfire.pf_dpsram_rccs; component pf_dpsram_rccs port (....); end component; .... inram_0 : pf_dpsram_rccs port map( ...);

through many tests i have found out that the notation

inst_name : entity library_name.module_name

leads to success

Using a dummy is possible in some cases but cannot be the solution for delivered modules that have a larger size with multiple files

Possibly a comment pragma such as --VerilogSource '--\s[vV]erilog\s[sS]ource' after the instantiation could help to leave the lower/upper case (original name) of the module name and thus achieve the connection

inRam_0 : PF_DPSRAM_RCCS --verilog source port map( ...);

or an option to leave the original names. This forces a better coding style :)

VG Helmut

LarsAsplund commented 2 years ago

@asicnet Thanks for the workaround! Can you provide your test code? I'd like to do some experiments which are not workarounds but rather enhancements of VUnit. It would be nice if we use the same example code.

asicnet commented 2 years ago

Hi see the pull request. #839 only vhdl_parser.py is changed

asicnet commented 2 years ago

Hi I find a new effects

 ------------------------------------------------------------

library design_work; use design_work.PF_DPSRAM_SPECIAL;

component PF_DPSRAM_SPECIAL
port (...); end component;

inRam_0 : PF_DPSRAM_SPECIAL 
    port map( ....);
 ------------------------------------------------------------

Error: special_storage.vhd(50): (vcom-1195) Cannot find expanded name "design_work.PF_DPSRAM_SPECIAL".

Now if i change the use statement to

  use design_work.all;

The compile order is not correct, but it is running. Has this effects on the compile options?

i test some compinations of instantiation and use-clauses.

inRam_0 : PF_DPSRAM_SPECIAL -- verilog source OK with or without any use-clauses inRam_0 : PF_DPSRAM_SPECIAL OK with use design_work.all; FAIL with use design_work.PF_DPSRAM_SPECIAL;

i can not find a rule