search

verilator / verilator

Verilator open-source SystemVerilog simulator and lint system
https://verilator.org
GNU Lesser General Public License v3.0
2.24k stars 548 forks source link

nested -F file.F does't work with --hierarchical #5114

Open solomatnikov opened 1 month ago

solomatnikov commented 1 month ago 
./test_regress/t/t_hier_trace.pl 
==SUMMARY: Left 2  Passed 0  Failed 0  Time 0:00
======================================================================
dist/t_hier_trace: ==================================================
-Skip: dist/t_hier_trace: scenario 'dist' not enabled for test
dist/t_hier_trace: -Skip: Skip: scenario 'dist' not enabled for test
==SUMMARY: Left 1  Passed 0  Failed 0  Time 0:00
======================================================================
vlt/t_hier_trace: ==================================================
    perl .../verilator/test_regress/../bin/verilator --prefix Vt_hier_trace ../obj_vlt/t_hier_trace/Vt_hier_trace__main.cpp --exe --make gmake --x-assign unique -cc -Mdir obj_vlt/t_hier_trace --fdedup --debug-check --comp-limit-members 10 --trace -j 4 t/t_hier_trace_sub/t_hier_trace.vlt --top-module t --hierarchical -F t/t_hier_trace_sub/top.F --clk clk  -f input.vc +define+TEST_OBJ_DIR=obj_vlt/t_hier_trace +define+TEST_DUMPFILE=obj_vlt/t_hier_trace/simx.vcd t/t_hier_trace.v    > obj_vlt/t_hier_trace/vlt_compile.log
make: Entering directory '.../verilator/test_regress/obj_vlt/t_hier_trace'
make[1]: Entering directory '.../verilator/test_regress'
.../verilator/bin/verilator -f obj_vlt/t_hier_trace/Vdetail_code__hierMkArgs.f
%Error: Cannot open -f command file: sub.F
%Error: Exiting due to 1 error(s)
make[1]: *** [obj_vlt/t_hier_trace/Vt_hier_trace_hier.mk:32: hier_launch_verilator] Error 1
make[1]: Leaving directory '.../verilator/test_regress'
make: *** [Vt_hier_trace_hier.mk:41: Vdetail_code/detail_code.sv] Error 2
make: Leaving directory '.../verilator/test_regress/obj_vlt/t_hier_trace'
%Error: make -C obj_vlt/t_hier_trace -f Vt_hier_trace_hier.mk  -j 4  hier_verilation exited with 2
%Error: Command Failed ulimit -s unlimited 2>/dev/null; exec .../verilator/bin/verilator_bin --prefix Vt_hier_trace ../obj_vlt/t_hier_trace/Vt_hier_trace__main.cpp --exe --make gmake --x-assign unique -cc -Mdir obj_vlt/t_hier_trace --fdedup --debug-check --comp-limit-members 10 --trace -j 4 t/t_hier_trace_sub/t_hier_trace.vlt --top-module t --hierarchical -F t/t_hier_trace_sub/top.F --clk clk -f input.vc +define+TEST_OBJ_DIR=obj_vlt/t_hier_trace +define+TEST_DUMPFILE=obj_vlt/t_hier_trace/simx.vcd t/t_hier_trace.v
%Warning: vlt/t_hier_trace: Exec of perl failed: make: Entering directory '.../verilator/test_regress/obj_vlt/t_hier_trace'

vlt/t_hier_trace: %Error: Exec of perl failed: make: Entering directory '.../verilator/test_regress/obj_vlt/t_hier_trace'
vlt/t_hier_trace: FAILED: Exec of perl failed: make: Entering directory '.../verilator/test_regress/obj_vlt/t_hier_trace'
==SUMMARY: Passed 0  Failed 1  Time 0:00

======================================================================
    #vlt/t_hier_trace: %Error: Exec of perl failed: make: Entering directory '.../verilator/test_regress/obj_vlt/t_hier_trace'
        make && test_regress/t/t_hier_trace.pl  --vlt
==TESTS DONE, FAILED: Passed 0  Failed 1  Time 0:00

Can you attach an example that shows the issue? (Must be openly licensed, ideally in test_regress format.)

diff --git a/test_regress/t/t_hier_trace.pl b/test_regress/t/t_hier_trace.pl
index 931a95165..64fcc5912 100755
--- a/test_regress/t/t_hier_trace.pl
+++ b/test_regress/t/t_hier_trace.pl
@@ -11,7 +11,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di
 scenarios(simulator => 1);

 compile(
-    verilator_flags2 => ['--trace', '-j 4', 't/t_hier_trace.vlt', '--top-module t', '--hierarchical'],
+    verilator_flags2 => ['--trace', '-j 4', 't/t_hier_trace_sub/t_hier_trace.vlt', '--top-module t', '--hierarchical', '-F t/t_hier_trace_sub/top.F'],
     );

 execute(
diff --git a/test_regress/t/t_hier_trace.v b/test_regress/t/t_hier_trace.v
index eee3d5a1b..d8d492912 100644
--- a/test_regress/t/t_hier_trace.v
+++ b/test_regress/t/t_hier_trace.v
@@ -4,49 +4,6 @@
 // any use, without warranty, 2024 by Wilson Snyder.
 // SPDX-License-Identifier: CC0-1.0

-module detail_code(
-    input clk,
-    input reset_l);
-endmodule
-
-module sub_top(
-    input clk,
-    input reset_l);
-
-    detail_code u0(
-          .clk(clk),
-          .reset_l(reset_l)
-    );
-    detail_code u1(
-          .clk(clk),
-          .reset_l(reset_l)
-    );
-    detail_code u2(
-          .clk(clk),
-          .reset_l(reset_l)
-    );
-    detail_code u3(
-          .clk(clk),
-          .reset_l(reset_l)
-    );
-    detail_code u4(
-          .clk(clk),
-          .reset_l(reset_l)
-    );
-    detail_code u5(
-          .clk(clk),
-          .reset_l(reset_l)
-    );
-    detail_code u6(
-          .clk(clk),
-          .reset_l(reset_l)
-    );
-    detail_code u7(
-          .clk(clk),
-          .reset_l(reset_l)
-    );
-endmodule
-
 module t(
     input clk,
     input reset_l);
diff --git a/test_regress/t/t_hier_trace_sub/sub.F b/test_regress/t/t_hier_trace_sub/sub.F
new file mode 100644
index 000000000..f5fd69c1a
--- /dev/null
+++ b/test_regress/t/t_hier_trace_sub/sub.F
@@ -0,0 +1 @@
+t_hier_trace.v
diff --git a/test_regress/t/t_hier_trace_sub/t_hier_trace.v b/test_regress/t/t_hier_trace_sub/t_hier_trace.v
new file mode 100644
index 000000000..564e375d7
--- /dev/null
+++ b/test_regress/t/t_hier_trace_sub/t_hier_trace.v
@@ -0,0 +1,48 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2024 by Wilson Snyder.
+// SPDX-License-Identifier: CC0-1.0
+
+module detail_code(
+    input clk,
+    input reset_l);
+endmodule
+
+module sub_top(
+    input clk,
+    input reset_l);
+
+    detail_code u0(
+          .clk(clk),
+          .reset_l(reset_l)
+    );
+    detail_code u1(
+          .clk(clk),
+          .reset_l(reset_l)
+    );
+    detail_code u2(
+          .clk(clk),
+          .reset_l(reset_l)
+    );
+    detail_code u3(
+          .clk(clk),
+          .reset_l(reset_l)
+    );
+    detail_code u4(
+          .clk(clk),
+          .reset_l(reset_l)
+    );
+    detail_code u5(
+          .clk(clk),
+          .reset_l(reset_l)
+    );
+    detail_code u6(
+          .clk(clk),
+          .reset_l(reset_l)
+    );
+    detail_code u7(
+          .clk(clk),
+          .reset_l(reset_l)
+    );
+endmodule
diff --git a/test_regress/t/t_hier_trace.vlt b/test_regress/t/t_hier_trace_sub/t_hier_trace.vlt
similarity index 100%
rename from test_regress/t/t_hier_trace.vlt
rename to test_regress/t/t_hier_trace_sub/t_hier_trace.vlt
diff --git a/test_regress/t/t_hier_trace_sub/top.F b/test_regress/t/t_hier_trace_sub/top.F
new file mode 100644
index 000000000..de9c6655b
--- /dev/null
+++ b/test_regress/t/t_hier_trace_sub/top.F
@@ -0,0 +1 @@
+-F sub.F

What 'verilator' command line do we use to run your example?

See above

What 'verilator --version' are you using? Did you try it with the git master version?

v5.024-gce5cad17a

What OS and distribution are you using?

Linux 5.14.0-427.13.1.el9_4.x86_64

May we assist you in trying to fix this in Verilator yourself?

solomatnikov commented 1 month ago

Note: if --hierarchical is removed from test_regress/t/t_hier_trace.pl then it works:

./test_regress/t/t_hier_trace.pl 
==SUMMARY: Left 2  Passed 0  Failed 0  Time 0:00
======================================================================
dist/t_hier_trace: ==================================================
-Skip: dist/t_hier_trace: scenario 'dist' not enabled for test
dist/t_hier_trace: -Skip: Skip: scenario 'dist' not enabled for test
==SUMMARY: Left 1  Passed 0  Failed 0  Time 0:00
======================================================================
vlt/t_hier_trace: ==================================================
    perl .../verilator/test_regress/../bin/verilator --prefix Vt_hier_trace ../obj_vlt/t_hier_trace/Vt_hier_trace__main.cpp --exe --make gmake --x-assign unique -cc -Mdir obj_vlt/t_hier_trace --fdedup --debug-check --comp-limit-members 10 --trace -j 4 t/t_hier_trace_sub/t_hier_trace.vlt --top-module t -F t/t_hier_trace_sub/top.F --clk clk  -f input.vc +define+TEST_OBJ_DIR=obj_vlt/t_hier_trace +define+TEST_DUMPFILE=obj_vlt/t_hier_trace/simx.vcd t/t_hier_trace.v    > obj_vlt/t_hier_trace/vlt_compile.log
- V e r i l a t i o n   R e p o r t: Verilator 5.025 devel rev v5.024-47-gce5cad17a
- Verilator: Built from 0.028 MB sources in 4 modules, into 0.034 MB in 9 C++ files needing 0.000 MB
- Verilator: Walltime 0.023 s (elab=0.001, cvt=0.007, bld=0.000); cpu 0.000 s on 4 threads; alloced 14.734 MB
    make -C obj_vlt/t_hier_trace -f .../verilator/test_regress/Makefile_obj --no-print-directory VM_PREFIX=Vt_hier_trace TEST_OBJ_DIR=obj_vlt/t_hier_trace CPPFLAGS_DRIVER=-DT_HIER_TRACE  OPT_FAST=-O0 OPT_GLOBAL=-O0 Vt_hier_trace    > obj_vlt/t_hier_trace/vlt_gcc.log
driver: Entering directory '.../verilator/test_regress/obj_vlt/t_hier_trace'
g++  -I.  -MMD -I.../verilator/test_regress/../include -I.../verilator/test_regress/../include/vltstd -DVM_COVERAGE=0 -DVM_SC=0 -DVM_TRACE=1 -DVM_TRACE_FST=0 -DVM_TRACE_VCD=1 -faligned-new -fcf-protection=none -Wno-bool-operation -Wno-shadow -Wno-sign-compare -Wno-tautological-compare -Wno-uninitialized -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wno-unused-parameter -Wno-unused-variable      -DVERILATOR=1 -DVL_DEBUG=1 -DTEST_OBJ_DIR=obj_vlt/t_hier_trace -DVM_PREFIX=Vt_hier_trace -DVM_PREFIX_INCLUDE="<Vt_hier_trace.h>" -DVM_PREFIX_ROOT_INCLUDE="<Vt_hier_trace___024root.h>" -DT_HIER_TRACE   -DVL_LOCK_SPINS=10000 -O0 -c -o Vt_hier_trace__main.o ../../obj_vlt/t_hier_trace/Vt_hier_trace__main.cpp
g++ -O0  -I.  -MMD -I.../verilator/test_regress/../include -I.../verilator/test_regress/../include/vltstd -DVM_COVERAGE=0 -DVM_SC=0 -DVM_TRACE=1 -DVM_TRACE_FST=0 -DVM_TRACE_VCD=1 -faligned-new -fcf-protection=none -Wno-bool-operation -Wno-shadow -Wno-sign-compare -Wno-tautological-compare -Wno-uninitialized -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wno-unused-parameter -Wno-unused-variable      -DVERILATOR=1 -DVL_DEBUG=1 -DTEST_OBJ_DIR=obj_vlt/t_hier_trace -DVM_PREFIX=Vt_hier_trace -DVM_PREFIX_INCLUDE="<Vt_hier_trace.h>" -DVM_PREFIX_ROOT_INCLUDE="<Vt_hier_trace___024root.h>" -DT_HIER_TRACE   -DVL_LOCK_SPINS=10000 -c -o verilated.o .../verilator/test_regress/../include/verilated.cpp
g++ -O0  -I.  -MMD -I.../verilator/test_regress/../include -I.../verilator/test_regress/../include/vltstd -DVM_COVERAGE=0 -DVM_SC=0 -DVM_TRACE=1 -DVM_TRACE_FST=0 -DVM_TRACE_VCD=1 -faligned-new -fcf-protection=none -Wno-bool-operation -Wno-shadow -Wno-sign-compare -Wno-tautological-compare -Wno-uninitialized -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wno-unused-parameter -Wno-unused-variable      -DVERILATOR=1 -DVL_DEBUG=1 -DTEST_OBJ_DIR=obj_vlt/t_hier_trace -DVM_PREFIX=Vt_hier_trace -DVM_PREFIX_INCLUDE="<Vt_hier_trace.h>" -DVM_PREFIX_ROOT_INCLUDE="<Vt_hier_trace___024root.h>" -DT_HIER_TRACE   -DVL_LOCK_SPINS=10000 -c -o verilated_vcd_c.o .../verilator/test_regress/../include/verilated_vcd_c.cpp
g++ -O0  -I.  -MMD -I.../verilator/test_regress/../include -I.../verilator/test_regress/../include/vltstd -DVM_COVERAGE=0 -DVM_SC=0 -DVM_TRACE=1 -DVM_TRACE_FST=0 -DVM_TRACE_VCD=1 -faligned-new -fcf-protection=none -Wno-bool-operation -Wno-shadow -Wno-sign-compare -Wno-tautological-compare -Wno-uninitialized -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wno-unused-parameter -Wno-unused-variable      -DVERILATOR=1 -DVL_DEBUG=1 -DTEST_OBJ_DIR=obj_vlt/t_hier_trace -DVM_PREFIX=Vt_hier_trace -DVM_PREFIX_INCLUDE="<Vt_hier_trace.h>" -DVM_PREFIX_ROOT_INCLUDE="<Vt_hier_trace___024root.h>" -DT_HIER_TRACE   -DVL_LOCK_SPINS=10000 -c -o verilated_threads.o .../verilator/test_regress/../include/verilated_threads.cpp
/usr/bin/python3 .../verilator/test_regress/../bin/verilator_includer -DVL_INCLUDE_OPT=include Vt_hier_trace.cpp Vt_hier_trace___024root__DepSet_h8d33681b__0.cpp Vt_hier_trace___024root__DepSet_h58621984__0.cpp Vt_hier_trace__Trace__0.cpp Vt_hier_trace___024root__Slow.cpp Vt_hier_trace___024root__DepSet_h58621984__0__Slow.cpp Vt_hier_trace__Syms.cpp Vt_hier_trace__Trace__0__Slow.cpp Vt_hier_trace__TraceDecls__0__Slow.cpp > Vt_hier_trace__ALL.cpp
g++ -O0  -I.  -MMD -I.../verilator/test_regress/../include -I.../verilator/test_regress/../include/vltstd -DVM_COVERAGE=0 -DVM_SC=0 -DVM_TRACE=1 -DVM_TRACE_FST=0 -DVM_TRACE_VCD=1 -faligned-new -fcf-protection=none -Wno-bool-operation -Wno-shadow -Wno-sign-compare -Wno-tautological-compare -Wno-uninitialized -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wno-unused-parameter -Wno-unused-variable      -DVERILATOR=1 -DVL_DEBUG=1 -DTEST_OBJ_DIR=obj_vlt/t_hier_trace -DVM_PREFIX=Vt_hier_trace -DVM_PREFIX_INCLUDE="<Vt_hier_trace.h>" -DVM_PREFIX_ROOT_INCLUDE="<Vt_hier_trace___024root.h>" -DT_HIER_TRACE   -DVL_LOCK_SPINS=10000 -c -o Vt_hier_trace__ALL.o Vt_hier_trace__ALL.cpp
echo "" > Vt_hier_trace__ALL.verilator_deplist.tmp
g++    Vt_hier_trace__main.o verilated.o verilated_vcd_c.o verilated_threads.o Vt_hier_trace__ALL.a    -pthread -lpthread -latomic   -o Vt_hier_trace
rm Vt_hier_trace__ALL.verilator_deplist.tmp
driver: Leaving directory '.../verilator/test_regress/obj_vlt/t_hier_trace'
    obj_vlt/t_hier_trace/Vt_hier_trace -j 4    > obj_vlt/t_hier_trace/vlt_sim.log
*-* All Finished *-*
- t/t_hier_trace.v:22: Verilog $finish
Can't exec "vcddiff": No such file or directory at ./driver.pl line 2429.
-Skip: vlt/t_hier_trace: No vcddiff installed

vlt/t_hier_trace: -Skip: Skip: No vcddiff installed
==SUMMARY: Passed 0  Failed 0  Skipped 1  Time 0:12

======================================================================
    #vlt/t_hier_trace: -Skip:  Skip: No vcddiff installed
==TESTS DONE, PASSED w/SKIPS: Passed 0  Failed 0  Skipped 1  Time 0:12
wsnyder commented 1 month ago

Hierarchical changes the current directory (via make -C), so this is not surprising. It needs to either adjust all paths it might possibly reference, or probably better avoid changing directories when performing the hier make and instead reference the subdirectory as necessary via a new command line option.

Might you be willing to make a pull to do the latter?

solomatnikov commented 4 weeks ago

Which source files would you suggest to look at?

wsnyder commented 4 weeks ago

This is what puts the makefile line with -C for Verilator recursively: https://github.com/verilator/verilator/blob/19cccd170ec546ee4e136e20012e9023f47bc8f5/src/V3EmitMk.cpp#L320

solomatnikov commented 3 weeks ago

In file test_regress/obj_vlt/t_hier_trace/Vdetail_code__hierMkArgs.f there are:

"-F" "t/t_hier_trace_sub/top.F"
"-F" "sub.F"

the 2nd is incorrect.

solomatnikov commented 3 weeks ago

@wsnyder where is m_gparams in V3HierBlock set?

wsnyder commented 3 weeks ago

The constructor: https://github.com/search?q=repo%3Averilator%2Fverilator%20m_gparams&type=code

solomatnikov commented 3 weeks ago

The constructor: https://github.com/search?q=repo%3Averilator%2Fverilator%20m_gparams&type=code

Obviously, I knew where the constructor was.

The question was where/how m_gparams are set.

It looks like m_gparams is redundant and always empty:

- V3Ast.cpp:1324:     Dumping obj_vlt/t_hier_trace/Vt_hier_trace_015_linkdotparam.tree
- V3LinkLValue.cpp:328:linkLValue: 
- V3Dead.cpp:542:     deadifyModules: 
- V3HierBlock.cpp:264:Checking 't' from null
- V3HierBlock.cpp:264:Checking 'sub_top' from null
- V3HierBlock.cpp:264:Checking 'detail_code' from 'sub_top'
- V3HierBlock.cpp:117:gparams() 'detail_code' 
- V3HierBlock.cpp:325:Add 'detail_code' with 0 parameters
- V3HierBlock.cpp:117:gparams() 'sub_top' 
- V3HierBlock.cpp:325:Add 'sub_top' with 0 parameters
- V3HierBlock.cpp:337:Found usage relation 'sub_top' uses 'detail_code'
- V3HierBlock.cpp:264:Checking '@CONST-POOL@' from null
- V3StatsReport.cpp:231:statsReport: 
- V3Ast.cpp:1324:     Dumping obj_vlt/t_hier_trace/Vt_hier_trace_990_final.tree
- V3HierBlock.cpp:117:gparams() 'sub_top' 
- V3HierBlock.cpp:117:gparams() 'sub_top' 
- V3HierBlock.cpp:117:gparams() 'detail_code' 
- V3HierBlock.cpp:117:gparams() 'detail_code' 
- V3HierBlock.cpp:117:gparams() 'detail_code' 
- V3HierBlock.cpp:117:gparams() 'sub_top' 
- V3HierBlock.cpp:117:gparams() 'detail_code' 
- V3EmitMk.cpp:423:   emitHierVerilation: 
- V3Os.cpp:427:       Running system: make -C obj_vlt/t_hier_trace -f Vt_hier_trace_hier.mk  -j 4  hier_verilation
[Detaching after vfork from child process 399937]
make: Entering directory '/work/sols/work/verilator/test_regress/obj_vlt/t_hier_trace'
make[1]: Entering directory '/work/sols/work/verilator/test_regress'
/work/sols/work/verilator/bin/verilator -f obj_vlt/t_hier_trace/Vdetail_code__hierMkArgs.f
Starting Verilator 5.025 devel rev v5.024-52-gde950329b (mod)
Starting Verilator 5.025 devel rev v5.024-52-gde950329b (mod)
- V3Options.cpp:1752: Reading Options File t/t_hier_trace_sub/top.F
- V3Options.cpp:1752: Reading Options File t/t_hier_trace_sub/sub.F
- V3PreProc.cpp:342:  DEFINE 'TEST_OBJ_DIR' as 'obj_vlt/t_hier_trace' params ''
- V3PreProc.cpp:342:  DEFINE 'TEST_DUMPFILE' as 'obj_vlt/t_hier_trace/simx.vcd' params ''
- V3Options.cpp:1752: Reading Options File sub.F
- V3File.cpp:100:     -Info: File not statable: sub.F
%Error: Cannot open -f command file: sub.F

where gparams() is modified as:

const V3HierBlock::GParams& V3HierBlock::gparams() const {
    std::stringstream pss;
    for (const auto& p : stringifyParams(m_gparams, false)) {
    pss << p.first << " " << p.second << "\n";
    }
    UINFO(3, "gparams() " << modp()->prettyNameQ() << " " << pss.str() << std::endl);
    return m_gparams;
}
solomatnikov commented 3 weeks ago

The problem is that V3Options::allArgsStringForHierBlock returns 

"--make" "gmake" "--x-assign" "unique" "--fdedup" "--debug-check" "--comp-limit-members" "10" "--debug" "--debugi" "8" "--gdb" "--trace" "-F" "t/t_hier_trace_sub/top.F" "+define+TEST_OBJ_DIR=obj_vlt/t_hier_trace" "+define+TEST_DUMPFILE=obj_vlt/t_hier_trace/simx.vcd" "-F" "sub.F" "t_hier_trace.v" "+librescan" "+notimingchecks" "+libext+.v" "-y" "t" "+incdir+t"

with incorrect "-F" "sub.F"