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

Vivado test infrastructure fails on Parameters #2266

Closed
vmchale opened this issue Jul 3, 2022 · 8 comments
Closed

Vivado test infrastructure fails on Parameters #2266

vmchale opened this issue Jul 3, 2022 · 8 comments

Comments

@vmchale
Copy link
Contributor

vmchale commented Jul 3, 2022

ERROR: [XSIM 43-3225] Cannot find design unit topEntity.testBench in library work located at xsim.dir/work.

It seems to try to simulate topEntity.testBench instead of testBench.testBench, see #2257 (comment)

@vmchale
Copy link
Contributor Author

vmchale commented Jul 11, 2022

Same thing happening here? :( https://gitlab.com/clash-lang/clash-compiler/-/jobs/2704503129

Notably, only three out of the four tests trip this up.

@vmchale
Copy link
Contributor Author

vmchale commented Jul 11, 2022

Ok, I've gotten further figuring this out. I think that something trips up when the testBench bit is not monomorphic - mkTestBench x does not work, but testBench does.

My guess is that adding an INLINE pragma will fix this.

@vmchale
Copy link
Contributor Author

vmchale commented Jul 11, 2022

set_property top_lib {#{entity}} [current_fileset -sim]

fixes VHDL but breaks Verilog in the process.

@martijnbastiaan
Copy link
Member

Yeah that makes sense, Verilog doesn't have a library concept :).

@DigitalBrains1
Copy link
Member

  • The Parameters.hs test is a bit messy. Most importantly, it omits something you should always include : {-# NOINLINE topEntity #-} and {-# NOINLINE testBench #-}.
    It also imports a lot of stuff it doesn't use, whereas I'd prefer my -Wall to be silent. And it defines primitives for all three HDL's but the test is only run for VHDL?
  • Reordering the read files and removing update_compile_order fixes the test bench. So we're hitting some Vivado bug/shortcoming with update_compile_order here probably; no idea what's so different about it!
  • What do you mean by when the testBench bit is not monomorphic? A working testBench is always monomorphic, so I'm missing some context here. There is the issue of Test bench needs INLINE when passing down top entity #2196 relating to mandatory INLINE statements in test benches, but it doesn't apply to Parameters.hs.
  • Indeed, top_lib will do the wrong thing for Verilog. I haven't really figured out SystemVerilog libraries and interworking between VHDL and Verilog yet in Vivado.

@leonschoorl
Copy link
Member

leonschoorl commented Jul 12, 2022

it omits something you should always include : ... {-# NOINLINE testBench #-}

Since testBench generally isn't used anywhere, I don't think this is needed or does anything at all.

@DigitalBrains1
Copy link
Member

Leon and I discussed this in real life.

First off, the NOINLINEs are not in any way related to the failures here; we discovered what causes it already.

Secondly, Leon and I agree that topEntity should get a NOINLINE, but we have different opinions about testBench.

In the following, when I refer to the "top entity" I mean it in the sense of the HDL project, as you enter it into your synthesis or simulation tool. So I'm not referring to a function called topEntity.

  • My preference is for rigorously applying recommended practices, and annotating everything that has a Synthesize annotation and also topEntity and testBench with a NOINLINE. My reasoning is that when developing a project, today's top entity might not be tomorrow's top entity, and it's perfectly possible, if done right, to compose multiple functions called testBench into one combined test bench. So in the interest of the principle of least surprise, I put NOINLINE on all my Synthesize annotated functions in Clash. Note I'm not suggesting fixing this all throughout existing clash-testsuite test cases, but I will put NOINLINE on all my additions there.
  • Leons view is that we are specifically considering small tests in clash-testsuite, with little code churn and usually a simple topEntity/testBench structure. The NOINLINE on testBench doesn't actually add anything there, it can just as well be omitted when the author knows what they are doing.

@DigitalBrains1
Copy link
Member

DigitalBrains1 commented Oct 10, 2022

The problem with this test is update_compile_order. To use the official technical jargon, this function is what we call crappy.

The generated top entity has:

  my_add_block : block
    component my_add
      generic
      ( size : integer );
      port ( x      : in unsigned(31 downto 0)
           ; y      : in unsigned(31 downto 0)
           ; result : out unsigned(31 downto 0) );
    end component;
    begin
    my_add_inst : component my_add
      generic map
        (size => 32)
      port map
        (x      => a, y      => a, result => result);
  end block;

This entity requires the component my_add, but it does not depend on it because the component declaration is included. So this VHDL file can be compiled before the VHDL file containing the definition of my_add.

update_compile_order is a project-mode command which we are forced to use because of our own bugs (#2325 #2334). The implied current library is set to the library of the final file in the compile order , and we have no way to specify the library for the top entity. So, the final file in the compile order will need to be in the same library as the top entity for simulation. However, update_compile_order decides in this case to put the my_add...vhdl file last because it is not needed for compilation of any other file:

Vivado% report_compile_order
Source compile order for 'simulation' with fileset 'sim_1' in source management mode 'None' and simulation language 'Mixed':
Index  File Name                                    Used_In      File_Type  Library
-----  -------------------------------------------  -----------  ---------  ---------
1      Parameters_testBench_types.vhdl              Synth & Sim  VHDL       testBench
2      Parameters_topEntity_types.vhdl              Synth & Sim  VHDL       topEntity
3      testBench_slv2string_2F58399B7F4E729C.vhdl   Synth & Sim  VHDL       testBench
4      topEntity.vhdl                               Synth & Sim  VHDL       topEntity
5      testBench.vhdl                               Synth & Sim  VHDL       testBench
6      topEntity_my_vhdl_add_3027167B0BC5D786.vhdl  Synth & Sim  VHDL       topEntity

(edited for brevity)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants