Skip to content
This repository has been archived by the owner on Aug 20, 2024. It is now read-only.

stale black_box_verilog_files.f may cause test failures #504

Closed
ucbjrl opened this issue Mar 21, 2017 · 5 comments
Closed

stale black_box_verilog_files.f may cause test failures #504

ucbjrl opened this issue Mar 21, 2017 · 5 comments

Comments

@ucbjrl
Copy link
Contributor

ucbjrl commented Mar 21, 2017

BlackBoxSourceHelper.execute() creates a black_box_verilog_files.f in the target directory iff there are BlackBoxResource or BlackBoxInline annotations.
chisel3.iotesters.verilogToVCS() unconditionally adds a -f black_box_verilog_files.f to the vcsFlags if that file exists.
This can cause tests following a black box test to fail when the file(s) mentioned in black_box_verilog_files.f no longer exist.

[info] [0.000] Elaborating design...
[info] [0.158] Done elaborating.
verilator --cc doohickey.v -f .../test_run_dir/black_box_verilog_files.f --assert -Wno-fatal -Wno-WIDTH -Wno-STMTDLY --trace -O1 --top-module doohickey +define+TOP_TYPE=Vdoohickey +define+PRINTF_COND=!doohickey.reset +define+STOP_COND=!doohickey.reset -CFLAGS -Wno-undefined-bool-conversion -O1 -DTOP_TYPE=Vdoohickey -DVL_USER_FINISH -include Vdoohickey.h -Mdir .../test_run_dir --exe .../test_run_dir/doohickey-harness.cpp
%Error: Cannot find file containing library module: .../test_run_dir/BlackBoxTest.v
%Error: This may be because there's no search path specified with -I<dir>.
%Error: Looked in:
%Error:       .../test_run_dir/BlackBoxTest.v
%Error:       .../test_run_dir/BlackBoxTest.v.v
%Error:       .../test_run_dir/BlackBoxTest.v.sv
%Error:       .../test_run_dir/.../test_run_dir/BlackBoxTest.v
%Error:       .../test_run_dir/.../test_run_dir/BlackBoxTest.v.v
%Error:       .../test_run_dir/.../test_run_dir/BlackBoxTest.v.sv
%Error: Exiting due to 9 error(s)
%Error: Command Failed /usr/local/bin/verilator_bin --cc doohickey.v -f .../test_run_dir/black_box_verilog_files.f --assert -Wno-fatal -Wno-WIDTH -Wno-STMTDLY --trace -O1 --top-module doohickey '+define+TOP_TYPE=Vdoohickey' '+define+PRINTF_COND=!doohickey.reset' '+define+STOP_COND=!doohickey.reset' -CFLAGS '-Wno-undefined-bool-conversion -O1 -DTOP_TYPE=Vdoohickey -DVL_USER_FINISH -include Vdoohickey.h' -Mdir .../test_run_dir --exe .../test_run_dir/doohickey-harness.cpp
[info] VerilatorTest:
[info] The Verilator backend
[info] - should be able to compile the cpp code *** FAILED ***
[info]   java.lang.AssertionError: assertion failed:
@ucbjrl ucbjrl changed the title stale black_box_verilog_files.f may cause test failures stale black_box_verilog_files.f may cause test failures Mar 21, 2017
@shunshou
Copy link
Contributor

In general, there's a stale file problem. For Verilator, I'm having tests pass/fail when they should fail/pass b/c things didn't get updated for some reason. If you're going to be running tests, IMO, it might be good just to wipe out the directory the tests will be run in before running things. I'm doing that just so that the tests are consistent w/ my latest Verilog...

@ucbjrl
Copy link
Contributor Author

ucbjrl commented Mar 21, 2017

Agreed, but this doesn't play well with sbt test. We may want to revive @chick's idea of randomizing individual test directories (under test_run_dir).

@jackkoenig
Copy link
Contributor

Anything that uses createTestDirectory gets a fresh directory based on the current time. Any tests using files that are not using that probably should. All of the Execution and Compilation Tests already do.

@chick
Copy link
Contributor

chick commented Apr 25, 2017

The test directory naming conventions and practices assumed that there was some utility to keeping an individual test's work directory the same from run to run. For example, I keep an editor window open on the firrtl output and intellij refreshes it each time I run which helps with debugging sometimes. This is not a critical feature but it came from listening to complaints from people either not knowing or not have having a good way of seeing where things were built.

  • Reusing a build directory for repeated run of a specific test
    • it makes it easier to find intermediate files, which can be useful for debugging
    • minimizes the number of temp directories laying around.
    • can cause failures when directory name is based on DUT and multiple different tests use same DUT, particularly when using verilator backend
    • Seems to cause problems as per reason for this PT
  • Creating a new build directory based on random or similar
    • fixes problems with stale data, or shared DUTS
    • makes it hard to find intermediate files for inspection
    • leaves lots of old garbage directories around which can take up a fair amount of space
  • The solution here
    • MUST fix directory collision problems with verilator
    • MUST fix the black box helper problem described here-in
      • Should this be made to follow conventions of the new feature of splitting all verilog files or making them all in-line
    • Can above be done for developers use cases, or in a way that makes it easy to switch behavior
    • MUST include a wiki help page that describes the testing build directory infrastructure

Comments @shunshou @jackkoenig @ucbjrl @chick

@shunshou
Copy link
Contributor

I'm kind of confused how Verilator plays with Chisel. I keep getting burned when I forget to rm the test directory, b/c something (idk what) doesn't get updated and my tests fail/pass when they should pass/fail...

If you do reuse a directory (for a single simulation), I think it's important to delete the directory contents... or at least move the contents somewhere else if you really want to keep them...

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

No branches or pull requests

4 participants