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

source file ordering issues with python2.7 (VUnit is not properly finding compile order) #556

Closed
GlenNicholls opened this issue Oct 2, 2019 · 19 comments

Comments

@GlenNicholls
Copy link
Contributor

GlenNicholls commented Oct 2, 2019

I am transitioning a massive code-base to use VUnit for automated testing. For some reason, I am not able to get VUnit to properly discover the correct file ordering. Because of this, I used an OrderedDict() to prevent keys from getting shuffled, but when I set no_parse=True, then I don't know how to get VUnit to start compiling in activeHDL with the first library/file that I added since it also cannot find the testbench

When I keep no_parse=False, library.cfg gets the ordering correct. However, I start getting warnings about files that VUnit has parsed because it cannot find the library. Below, I am iterating over my dictionary of {'library' : ['<list of paths>']} to add all my source files to the proper libraries:

for srcLib, src in modules.items():
    print('Adding {} sources to {} lib'.format(len(src), srcLib))
    vu.add_library(srcLib, vhdl_standard='2008').add_source_files(src, file_type='vhdl')

below is library.cfg which is in the order that I would expect

$INCLUDE = "C:/Aldec/Active-HDL-10.4/bin/../vlib/library.cfg"
vunit_lib = "./libraries/vunit_lib/vunit_lib.lib" 1570052090479
osvvm = "./libraries/osvvm/osvvm.lib" 1570052090808
pmc_control_base = "./libraries/PMC_CONTROL_Base/PMC_CONTROL_Base.lib" 1570052092071
pmc_control = "./libraries/PMC_CONTROL/PMC_CONTROL.lib" 1570052092357

However, this is the first thing that VUnit prints. It is saying it cant find library PMC_CONTROL_Base, but it should have realized that dpCntrlPack.vhd needed to be parsed before dpCntrlBody.vhd:

WARNING - P:\libs\pmc_control_base\src\dpCntrlBody.vhd: failed to find library 'PMC_CONTROL_Base'

Then when it starts compiling, I see this problem. The first file that the VUnit parser thought was supposed to be compiled was incorrect as this file has a dependency of PMC_CONTROL_Base:

Compiling into PMC_CONTROL:      ..\..\libs\pmc_control\firmware\src\dpCntrlPack.vhd                                      failed
=== Command used: ===
C:\Aldec\Active-HDL-10.4\bin\vcom -quiet -j P:\modulator\test\vunit_out\activehdl -2008 -work PMC_CONTROL P:\libs\pmc_control\firmware\src\dpCntrlPack.vhd

=== Command output: ===
COMP96 ERROR COMP96_0078: "Unknown identifier "pmcCntrl"." "P:\libs\pmc_control\firmware\src\dpCntrlPack.vhd" 36 45

The names above might be confusing, but basically the VUnit parser incorrectly parsed dependencies in my code-base all over the place and I'm unsure of how to debug and fix it. When I add generate a .tcl script to compile and run all these files, everything works fine and I don't get any errors from aldec.

@kraigher
Copy link
Collaborator

kraigher commented Oct 3, 2019

I will have a look on this problem this evening. In the meantime would it be possible for you to create a minimal verifiable example (https://stackoverflow.com/help/minimal-reproducible-example)? Just take one small part of your code and try to reproduce it, maybe by focusing on just one test bench. When you can reproduce you can copy the code and strip everything except the shell of the entities/architectures/packages since it is not relevant for dependency scanning.

@GlenNicholls
Copy link
Contributor Author

Yeah, I will work on that. I'll have to go through a few people so this is probably not something I can share publically. Once I get the code stripped down, I will let you know, but it might be a few days.

From a use standpoint, is there a way to force VUnit to accept all incoming files as if they're in the correct compile order and just tell it which file the testbench is in, or only allow it to scan for testbenches?

@kraigher
Copy link
Collaborator

kraigher commented Oct 3, 2019

First of all no_parse=True is only intended for encrypted files which cannot be parsed. For those files you either need to pre-compile them outside of VUnit or add them with no_parse=True and add the dependencies yourself manually. To not have to keep track of dependencies manually I typically add these files as a dependency to all non-encrypted files. This is not a problem since they do not change often.

There is no way currently to disable parsing but enable test case scanning since test case scanning uses the parse results. Test case files should not be encrypted so this should not be a problem.

The order in library.cfg should not matter at all and is probably not relevant to your problem.

Regarding permission to release your code as a minimal verifiable example. You can remove everything in the code except the shell which removes. That is all processes, statements, signals, ports, generics etc. Thus only the name of entities remain and the instantiation statements without ports. If you still think the names give away to much information you can obfuscate them.

Maybe your bug is related to the upper/lower case of the library name. VHDL is case insensitive so the case can be inconsistently used. VUnit should handle this now but we used to be case sensitive until (#241). IMHO a good VHDL code should not rely on case insensitivity to work so this case might not be tested good since we do not write so much such code. Maybe there is still some bug related to inconsistent case of library name i.e mixing pmc_control_base with PMC_CONTROL_Base inconsistently in VUnit that your code base provokes. In VHDL you can get away with being inconsistent unfortunately. Just a theory... hopefully your minimal verifiable example can show the problem.

To get more debug information from VUnit you can use the --log-level=debug which will emit a lot of information regarding discovery of entities and dependencies. This might reveal some problem.

@joshrsmith
Copy link
Contributor

Another common thing that has caused this behavior for me is relying on default binding rule instead of explicitly indicating the library when instantiating a block.

YES: My_Block : entity the_library.the_block

@GlenNicholls
Copy link
Contributor Author

GlenNicholls commented Oct 4, 2019

Another common thing that has caused this behavior for me is relying on default binding rule instead of explicitly indicating the library when instantiating a block.

YES: My_Block : entity the_library.the_block

So if I understand you correctly, then you're saying you have issues when instantiating like this:

someComp_inst : entity work.compName

If this is the case, is there a way to still make this work? Unfortunately, we use this style all over the place. I like it because it simplifies either having to place the component inst in a package or noting the library name when instantiating.

@joshrsmith
Copy link
Contributor

Using work explicitly has always worked fine for me; your example should be fine (barring any capitalization issues that @kraigher noted.

@GlenNicholls
Copy link
Contributor Author

GlenNicholls commented Oct 7, 2019

@kraigher I found a couple problems that cause this bug in VUnit. The first is when using a type in one package and declaring a subtype in a seperate package with the path to the original type. I am adding those files now with the problem code uncommented in the second package and the fix uncommented. Unfortunately, this fix will not work for us, but this is what I had to do to get VUnit to stop compiling the two packages in the wrong order.

This package should be under PMC_CONTROL_Base:

library IEEE;
use IEEE.std_logic_1164.all;
use IEEE.std_logic_arith.all;

package pmcCntrl is

    type localBusInT is record
        LRST_N : std_logic; -- TMP for legacy support
        LReset : std_logic;
        LReadAddrReg : std_logic_vector(15 downto 2);
        LWriteAddrReg : std_logic_vector(15 downto 2);
    end record;

    type REGinT is record
        rdReqReady : std_logic;
        wrReqReady : std_logic;
        WriteData : std_logic_vector(31 downto 0);
    end record;

    type REGoutT is record
        ReadData : std_logic_vector(31 downto 0);
    end record;
end package;

The below package needs to be under PMC_CONTROL lib:

library IEEE;
use IEEE.std_logic_1164.all;
use IEEE.std_logic_arith.all;

library PMC_CONTROL_Base;

package pmcCntrl is

    type RegArray is array (natural range <>) of std_logic_vector(31 downto 0);

    subtype localBusInT is PMC_CONTROL_Base.pmcCntrl.localBusInT;
    subtype REGinT is PMC_CONTROL_Base.pmcCntrl.REGinT;
    subtype REGoutT is PMC_CONTROL_Base.pmcCntrl.REGoutT;
end package;

Basically our use case is that we cannot touch anything in the PMC_CONTROL_Base lib as that repository is locked by the creator, but we have other additional functionality that we're declaring in our new PMC_CONTROL lib. We need all port types and such to reference the original.

I have been running python run.py --clean --log-level=debug and I am getting this log:

Adding 2 sources to PMC_CONTROL_Base lib
  DEBUG - Adding library PMC_CONTROL_Base with path P:\mirkwood\modulator\test\vunit_out\activehdl\libraries\PMC_CONTROL_Base
  DEBUG - Adding source file P:\mirkwood\linux_driver-trunk\firmware\src\Altera\PMC_CONTROL\src\dpCntrlPack_test.vhd to library PMC_CONTROL_Base
  DEBUG - Adding primary design unit (package) pmccntrl
  DEBUG - The file 'P:\mirkwood\linux_driver-trunk\firmware\src\Altera\PMC_CONTROL\src\dpCntrlPack_test.vhd' has no components
  DEBUG - Adding source file P:\mirkwood\linux_driver-trunk\firmware\src\Altera\PMC_CONTROL\src\dpCntrlBody_test.vhd to library PMC_CONTROL_Base
  DEBUG - The file 'P:\mirkwood\linux_driver-trunk\firmware\src\Altera\PMC_CONTROL\src\dpCntrlBody_test.vhd' has no components
Adding 2 sources to PMC_CONTROL lib
  DEBUG - Adding library PMC_CONTROL with path P:\mirkwood\modulator\test\vunit_out\activehdl\libraries\PMC_CONTROL
  DEBUG - Adding source file P:\mirkwood\t600mmr-trunk\common\pmc_control\firmware\src\dpCntrlPack_test.vhd to library PMC_CONTROL
  DEBUG - Adding primary design unit (package) pmccntrl
  DEBUG - The file 'P:\mirkwood\t600mmr-trunk\common\pmc_control\firmware\src\dpCntrlPack_test.vhd' has no components
  DEBUG - Adding source file P:\mirkwood\t600mmr-trunk\common\pmc_control\firmware\src\dpCntrlBody_test.vhd to library PMC_CONTROL
  DEBUG - The file 'P:\mirkwood\t600mmr-trunk\common\pmc_control\firmware\src\dpCntrlBody_test.vhd' has no components
WARNING - Found no test benches using current filter rule:

I will start obfuscating the code for the next example for another peice of problem code I found.

@kraigher
Copy link
Collaborator

kraigher commented Oct 7, 2019

That type of dependency is not supported by VUnit as it requires full parsing and semantic analysis to perform reliably. The preferred work around is to add a use clause on top of the pmcCntl package in PMC_CONTROL library such as:

library PMC_CONTROL_Base;
use PMC_CONTROL_Base.pmcCntrl;

This will cause VUnit to pick up a dependency between the files. Alternatively you can use add a manual dependency via the Python API.

@GlenNicholls
Copy link
Contributor Author

@kraigher Okay awesome, that fixed both issues that I found. Is there a reason that using your method works, but the below doesn't?

library PMC_CONTROL_Base;
use PMC_CONTROL_Base.pmcCntrl.all;

@kraigher
Copy link
Collaborator

kraigher commented Oct 7, 2019

The method you list with .all should also work.

@GlenNicholls
Copy link
Contributor Author

Okay cool. As for dependencies, I am having the same problem in some other libraries with slightly different failure cases regarding the same parsing problem. One thing that we do all over the place is the following for instantiating dependencies in a library so we don't need to put the component instantiation in the package:

someComp_inst : entity work.someComp

In this case, we would like to keep only important components in our package for other libraries to reference while the building blocks that are not useful elsewhere are not accessible except locally. By this I mean that the component someComp is not declared in the library package(s) nor in the architecture of the entity using that. How do I get around this without having to re-factor my code base?

I guess this leads back to the problem that I don't need VUnit to re-shuffle all my files to scan for dependencies except the testbench. I feed in the files in the proper order, so it feels like I'm chasing my tail for a non-problem if there was support for that.

@kraigher
Copy link
Collaborator

kraigher commented Oct 7, 2019

Dependency scanning works very well for entity instantiation of the form someComp_inst : entity work.someComp.

@kraigher
Copy link
Collaborator

kraigher commented Oct 7, 2019

I guess this leads back to the problem that I don't need VUnit to re-shuffle all my files to scan for dependencies except the testbench. I feed in the files in the proper order, so it feels like I'm chasing my tail for a non-problem if there was support for that.

Sure maybe it could have some use to manually specify test benches without dependency scanning. There has been very little demand for it since dependency scanning works well except for a few cases which are easy to work around.

You claim to have a list of files in the proper order already. Howerver your dependencies are most likely not linear but a tree/graph. The distinction does not matter for a clean compile but for an incremental compile after changing one file it does. Not everything below that file in a typical file list needs to be compiled. This matters a lot in large projects.

@GlenNicholls
Copy link
Contributor Author

GlenNicholls commented Oct 7, 2019

Dependency scanning works very well for entity instantiation of the form someComp_inst : entity work.someComp.

So then what is the syntax for this? If I just use the code snippet that you referenced without importing the library work or having use work.somePack.all, I see the below warning:

WARNING - P:\libs\pmc_control\src\dpCntrlBody.vhd: failed to find library 'PMC_CONTROL'

Then I still see the same warning if I have the below:

library work
use work.somePack.all;

What is causing this error message?

Basically, the problem I am seeing is that VUnit is not recognizing the correct dependencies in certain libraries. In the PMC_CONTROL warning I posted above, having the below got rid of that warning.

library PMC_CONTROL;
use PMC_CONTROL.pmcCntrl.all;

However, in a different lib, I do not import the library at all, I just instantiate the component in the working directory using someComp_inst : entity work.someComp and I get the above warning about not being able to find the correct lib. Later, it ends up causing compilation in active HDL to fail because it got the source ordering wrong, so I'm trying to figure out how to fix this.

@kraigher
Copy link
Collaborator

kraigher commented Oct 8, 2019

I think you might have found a bug. VUnit will replace the work with the current library name but it seems it does not work when the library name is not all lower case. This is due to the complexity of supporting case sensitive library names in Verilog but not VHDL.

I will make a fix.

In the meantime as a work around you can use lower case names in the run.py file using string.lower(). Mixed case will still work from VHDL but the work reference will be correct.

@kraigher
Copy link
Collaborator

kraigher commented Oct 8, 2019

I have now pushed a fix to master. Could you try it?

@GlenNicholls
Copy link
Contributor Author

I have some bugs I'm working on for a different project, but I'll get to this as soon as I can!

@GlenNicholls
Copy link
Contributor Author

GlenNicholls commented Oct 11, 2019

@kraigher Okay last "bug" I believe that I found. Lets say I have a package with all my components defined and this compiles first:

pkg.vhd

package pkg is
    <ent1 component defined>
    <ent2 component defined>
end package;

Now, I have a file with multiple entities/architectures within it (horrible practice, but I didn't write this and have to live with it):

file.vhd

-- component 1
library ieee;
context ieee.ieee_std_context;

entity ent1
    <stuff here>
end entity;
architecture rtl of ent1 is
    <stuff>
begin
    <stuff>
end architecture;

-- component 2
library ieee;
context ieee.ieee_std_context;

library work;
use work.pkg;

entity ent2
    <stuff here>
end entity;
architecture rtl of ent1 is
    <stuff>
begin
    inst : entity work.pkg.ent1
    <stuff>
end architecture;

Why is it that the inst believes that it cannot find the context for ent1? By leaving everything the same, I can change inst : entity work.ent1 and this works. For reference, I am using active-HDL and this appears to be a problem with that, not with VUnit. This issue can be closed with what is on master, but I'd like to know why this above case fails.

@kraigher
Copy link
Collaborator

My understanding of your scenario is that work.pkg.ent1 refers to a component and work.ent1 references to an entity. Thus instantiating with inst : entity work.pkg.ent1 is illegal since it does not refer to an entity but a component. This method is called entity instantiation. However inst : component work.pkg.ent1 would work or just inst : work.pkg.ent1 since this is component instantiation.

In VHDL there is no equivalence between components and entities and it is a misunderstanding to say that a component ent1 is the same as entity ent1. To make a circuit board analogy: A component defines a socket and an entity defines a chip. A chip could fit into a socket if it has the correct pinning. But multiple chips can also fit into the same socket. The purpose of a component is to be able to switch which corresponding entity that is instantiated via for example a configuration block.

In most simple cases it would seem to a user that there is a direct relation between component and entity because of VHDL default binding rules where a entity in the same library as the component is automatically configured for the component instantiation without a configuration block explicitly selecting the entity to instantiate.

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