From 42b711b862571af996f7eadb0f0f756c98215bd0 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Tue, 5 Jul 2022 12:17:04 +0100 Subject: [PATCH 01/12] Don't use 'assert' in profiler initialization --- include/verilated_profiler.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/include/verilated_profiler.cpp b/include/verilated_profiler.cpp index 9f37addf9..21246827a 100644 --- a/include/verilated_profiler.cpp +++ b/include/verilated_profiler.cpp @@ -108,10 +108,13 @@ void VlExecutionProfiler::setupThread(uint32_t threadId) { // while profiling. t_trace.reserve(RESERVED_TRACE_CAPACITY); // Register thread-local buffer in list of all buffers + bool exists; { const VerilatedLockGuard lock{m_mutex}; - bool exists = !m_traceps.emplace(threadId, &t_trace).second; - assert(!exists); + exists = !m_traceps.emplace(threadId, &t_trace).second; + } + if (VL_UNLIKELY(exists)) { + VL_FATAL_MT(__FILE__, __LINE__, "", "multiple initialization of profiler on some thread"); } } From 3aa8624658613adddf1ad178f025ad0bd5816034 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Tue, 5 Jul 2022 10:57:16 +0100 Subject: [PATCH 02/12] Set 'threads' in tests via parameter to compile This is in preparation to #3454. --- test_regress/driver.pl | 8 ++++++-- test_regress/t/t_depth_flop.pl | 3 ++- test_regress/t/t_dotfiles.pl | 3 ++- test_regress/t/t_embed1.pl | 1 + test_regress/t/t_gantt.pl | 3 ++- test_regress/t/t_hier_block.pl | 3 ++- test_regress/t/t_hier_block_nohier.pl | 4 ++-- test_regress/t/t_hier_block_sc.pl | 2 +- test_regress/t/t_hier_block_sc_trace_fst.pl | 2 +- test_regress/t/t_hier_block_sc_trace_vcd.pl | 2 +- test_regress/t/t_hier_block_trace_fst.pl | 4 ++-- test_regress/t/t_hier_block_trace_vcd.pl | 4 ++-- test_regress/t/t_hier_block_vlt.pl | 4 ++-- test_regress/t/t_inst_tree_inl0_pub1.pl | 6 +++--- test_regress/t/t_lib_prot_shared.pl | 2 +- test_regress/t/t_pgo_profoutofdate_bad.pl | 2 +- test_regress/t/t_pgo_threads.pl | 7 ++++--- test_regress/t/t_split_var_0.pl | 4 ++-- test_regress/t/t_split_var_2_trace.pl | 4 ++-- test_regress/t/t_threads_counter_0.pl | 3 ++- test_regress/t/t_threads_counter_1.pl | 3 ++- test_regress/t/t_threads_counter_2.pl | 3 ++- test_regress/t/t_threads_counter_4.pl | 3 ++- test_regress/t/t_threads_crazy.pl | 3 ++- test_regress/t/t_threads_nondeterminism.pl | 3 ++- test_regress/t/t_trace_litendian.pl | 4 ++-- test_regress/t/t_trace_litendian_fst.pl | 4 ++-- test_regress/t/t_trace_litendian_fst_sc.pl | 4 ++-- test_regress/t/t_verilated_all.pl | 4 ++-- test_regress/t/t_verilated_threaded.pl | 3 ++- test_regress/t/t_wrapper_context.pl | 3 ++- test_regress/t/t_wrapper_context_fst.pl | 3 ++- test_regress/t/t_wrapper_context_seq.pl | 3 ++- 33 files changed, 67 insertions(+), 47 deletions(-) diff --git a/test_regress/driver.pl b/test_regress/driver.pl index 968b89f81..cbd9ba9ea 100755 --- a/test_regress/driver.pl +++ b/test_regress/driver.pl @@ -578,6 +578,7 @@ sub new { make_pli => 0, # need to compile pli sc_time_resolution => "SC_PS", # Keep - PS is SystemC default sim_time => 1100, + threads => -1, # --threads (negative means auto based on scenario) benchmark => $opt_benchmark, verbose => $opt_verbose, run_env => '', @@ -902,6 +903,7 @@ sub compile_vlt_flags { @{$param{verilator_flags}}, @{$param{verilator_flags2}}, @{$param{verilator_flags3}}); + die "%Error: specify threads via 'threads =>' argument, not as a command line option" unless ($checkflags !~ /(^|\s)-?-threads\s/ && $checkflags !~ /(^|\s)-?-no-threads($|\s)/); $self->{sc} = 1 if ($checkflags =~ /-sc\b/); $self->{trace} = ($opt_trace || $checkflags =~ /-trace\b/ || $checkflags =~ /-trace-fst\b/); @@ -920,8 +922,7 @@ sub compile_vlt_flags { unshift @verilator_flags, "--rr" if $opt_rr; unshift @verilator_flags, "--x-assign unique"; # More likely to be buggy unshift @verilator_flags, "--trace" if $opt_trace; - my $threads = ::calc_threads($Vltmt_threads); - unshift @verilator_flags, "--threads $threads" if $param{vltmt} && $checkflags !~ /-threads /; + unshift @verilator_flags, "--threads $param{threads}" if $param{threads} >= 0; unshift @verilator_flags, "--trace-threads 2" if $param{vltmt} && $checkflags =~ /-trace-fst /; unshift @verilator_flags, "--debug-partition" if $param{vltmt}; unshift @verilator_flags, "-CFLAGS -ggdb -LDFLAGS -ggdb" if $opt_gdbsim; @@ -972,6 +973,9 @@ sub compile { return 1 if $self->errors || $self->skips || $self->unsupporteds; $self->oprint("Compile\n") if $self->{verbose}; + die "%Error: 'threads =>' argument must be <= 1 for vlt scenario" if $param{vlt} && $param{threads} > 1; + $param{threads} = ::calc_threads($Vltmt_threads) if ($param{threads} < 0 && $param{vltmt}); + compile_vlt_cmd(%param); if (!$param{make_top_shell}) { diff --git a/test_regress/t/t_depth_flop.pl b/test_regress/t/t_depth_flop.pl index dabf3116e..8b4ad07ec 100755 --- a/test_regress/t/t_depth_flop.pl +++ b/test_regress/t/t_depth_flop.pl @@ -11,7 +11,8 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di scenarios(vltmt => 1); # Note issue shows up with --threads compile( - verilator_flags2 => ['--compiler clang --threads 2 -Wno-UNOPTTHREADS'], + verilator_flags2 => ['--compiler clang -Wno-UNOPTTHREADS'], + threads => 2 ); ok(1); diff --git a/test_regress/t/t_dotfiles.pl b/test_regress/t/t_dotfiles.pl index 31b8ac837..a9b52715e 100755 --- a/test_regress/t/t_dotfiles.pl +++ b/test_regress/t/t_dotfiles.pl @@ -14,7 +14,8 @@ scenarios(vltmt => 1); top_filename("t/t_gen_alw.v"); compile( - v_flags2 => ["--debug --debugi 5 --threads 2"] + v_flags2 => ["--debug --debugi 5"], + threads => 2 ); foreach my $dotname ("linkcells", "task_call", "gate_simp", "gate_opt", diff --git a/test_regress/t/t_embed1.pl b/test_regress/t/t_embed1.pl index a28adebe6..08e4c042c 100755 --- a/test_regress/t/t_embed1.pl +++ b/test_regress/t/t_embed1.pl @@ -22,6 +22,7 @@ mkdir $child_dir; (VM_PREFIX => "$Self->{VM_PREFIX}_child", top_filename => "$Self->{name}_child.v", verilator_flags => ["-cc", "-Mdir", "${child_dir}", "--debug-check"], + threads => $Self->{vltmt} ? $Self->get_default_vltmt_threads() : 0 ); run(logfile => "${child_dir}/vlt_compile.log", diff --git a/test_regress/t/t_gantt.pl b/test_regress/t/t_gantt.pl index c757d3fbe..1f92de577 100755 --- a/test_regress/t/t_gantt.pl +++ b/test_regress/t/t_gantt.pl @@ -18,8 +18,9 @@ scenarios(vlt_all => 1); top_filename("t/t_gen_alw.v"); compile( + v_flags2 => ["--prof-exec"], # Checks below care about thread count, so use 2 (minimum reasonable) - v_flags2 => ["--prof-exec", ($Self->{vltmt} ? "--threads 2" : "")] + threads => $Self->{vltmt} ? 2 : 0 ); execute( diff --git a/test_regress/t/t_hier_block.pl b/test_regress/t/t_hier_block.pl index cffc7ba04..e861c733e 100755 --- a/test_regress/t/t_hier_block.pl +++ b/test_regress/t/t_hier_block.pl @@ -18,11 +18,12 @@ scenarios(vlt_all => 1); compile( v_flags2 => ['t/t_hier_block.cpp'], - verilator_flags2 => ['--stats', ($Self->{vltmt} ? ' --threads 6' : ''), + verilator_flags2 => ['--stats', '--hierarchical', '--Wno-TIMESCALEMOD', '--CFLAGS', '"-pipe -DCPP_MACRO=cplusplus"' ], + threads => $Self->{vltmt} ? 6 : 0 ); execute( diff --git a/test_regress/t/t_hier_block_nohier.pl b/test_regress/t/t_hier_block_nohier.pl index 006c981b3..8da19e163 100755 --- a/test_regress/t/t_hier_block_nohier.pl +++ b/test_regress/t/t_hier_block_nohier.pl @@ -23,8 +23,8 @@ compile( v_flags2 => ['t/t_hier_block.cpp'], verilator_flags2 => ['--stats', '+define+USE_VLT', 't/t_hier_block_vlt.vlt', - '--CFLAGS', '"-pipe -DCPP_MACRO=cplusplus"', - ($Self->{vltmt} ? ' --threads 6' : '')], + '--CFLAGS', '"-pipe -DCPP_MACRO=cplusplus"'], + threads => $Self->{vltmt} ? 6 : 0 ); execute( diff --git a/test_regress/t/t_hier_block_sc.pl b/test_regress/t/t_hier_block_sc.pl index 139b280b9..8aff1358d 100755 --- a/test_regress/t/t_hier_block_sc.pl +++ b/test_regress/t/t_hier_block_sc.pl @@ -22,9 +22,9 @@ compile( verilator_flags2 => ['--sc', '--stats', '--hierarchical', - ($Self->{vltmt} ? ' --threads 6' : ''), '--CFLAGS', '"-pipe -DCPP_MACRO=cplusplus"' ], + threads => $Self->{vltmt} ? 6 : 0 ); execute( diff --git a/test_regress/t/t_hier_block_sc_trace_fst.pl b/test_regress/t/t_hier_block_sc_trace_fst.pl index 6c8dd260e..b2497ad14 100755 --- a/test_regress/t/t_hier_block_sc_trace_fst.pl +++ b/test_regress/t/t_hier_block_sc_trace_fst.pl @@ -22,11 +22,11 @@ compile( verilator_flags2 => ['--sc', '--stats', '--hierarchical', - ($Self->{vltmt} ? ' --threads 6' : ''), '--CFLAGS', '"-pipe -DCPP_MACRO=cplusplus"', "--CFLAGS", '"-O0 -ggdb"', "--trace-fst" ], + threads => $Self->{vltmt} ? 6 : 0 ); execute( diff --git a/test_regress/t/t_hier_block_sc_trace_vcd.pl b/test_regress/t/t_hier_block_sc_trace_vcd.pl index 3ce4b1da3..7239369c7 100755 --- a/test_regress/t/t_hier_block_sc_trace_vcd.pl +++ b/test_regress/t/t_hier_block_sc_trace_vcd.pl @@ -22,11 +22,11 @@ compile( verilator_flags2 => ['--sc', '--stats', '--hierarchical', - ($Self->{vltmt} ? ' --threads 6' : ''), '--CFLAGS', '"-pipe -DCPP_MACRO=cplusplus"', "--CFLAGS", '"-O0 -ggdb"', "--trace" ], + threads => $Self->{vltmt} ? 6 : 0 ); execute( diff --git a/test_regress/t/t_hier_block_trace_fst.pl b/test_regress/t/t_hier_block_trace_fst.pl index 29213f97b..753b9dda1 100755 --- a/test_regress/t/t_hier_block_trace_fst.pl +++ b/test_regress/t/t_hier_block_trace_fst.pl @@ -17,12 +17,12 @@ top_filename("t/t_hier_block.v"); compile( v_flags2 => ['t/t_hier_block.cpp'], - verilator_flags2 => [($Self->{vltmt} ? ' --threads 6' : ''), - '--hierarchical', + verilator_flags2 => ['--hierarchical', '--Wno-TIMESCALEMOD', '--trace-fst', '--no-trace-underscore', # To avoid handle mismatches ], + threads => $Self->{vltmt} ? 6 : 0 ); execute( diff --git a/test_regress/t/t_hier_block_trace_vcd.pl b/test_regress/t/t_hier_block_trace_vcd.pl index ed212b921..4f3b80f09 100755 --- a/test_regress/t/t_hier_block_trace_vcd.pl +++ b/test_regress/t/t_hier_block_trace_vcd.pl @@ -17,12 +17,12 @@ top_filename("t/t_hier_block.v"); compile( v_flags2 => ['t/t_hier_block.cpp'], - verilator_flags2 => [($Self->{vltmt} ? ' --threads 6' : ''), - '--hierarchical', + verilator_flags2 => ['--hierarchical', '--Wno-TIMESCALEMOD', '--trace', '--no-trace-underscore', # To avoid handle mismatches ], + threads => $Self->{vltmt} ? 6 : 0 ); execute( diff --git a/test_regress/t/t_hier_block_vlt.pl b/test_regress/t/t_hier_block_vlt.pl index 904182717..ef467ede9 100755 --- a/test_regress/t/t_hier_block_vlt.pl +++ b/test_regress/t/t_hier_block_vlt.pl @@ -22,8 +22,8 @@ compile( '--hierarchical', '+define+SHOW_TIMESCALE', '+define+USE_VLT', 't/t_hier_block_vlt.vlt', - '--CFLAGS', '"-pipe -DCPP_MACRO=cplusplus"', - ($Self->{vltmt} ? ' --threads 6' : '')], + '--CFLAGS', '"-pipe -DCPP_MACRO=cplusplus"'], + threads => $Self->{vltmt} ? 6 : 0 ); execute( diff --git a/test_regress/t/t_inst_tree_inl0_pub1.pl b/test_regress/t/t_inst_tree_inl0_pub1.pl index b06051080..bad6139b0 100755 --- a/test_regress/t/t_inst_tree_inl0_pub1.pl +++ b/test_regress/t/t_inst_tree_inl0_pub1.pl @@ -14,9 +14,9 @@ top_filename("t/t_inst_tree.v"); my $default_vltmt_threads = $Self->get_default_vltmt_threads(); compile( - verilator_flags2 => ['--stats', "$Self->{t_dir}/$Self->{name}.vlt", - # Force 3 threads even if we have fewer cores - $Self->{vltmt} ? "--threads $default_vltmt_threads" : ""] + verilator_flags2 => ['--stats', "$Self->{t_dir}/$Self->{name}.vlt"], + # Force 3 threads even if we have fewer cores + threads => $Self->{vltmt} ? $default_vltmt_threads : 0 ); sub checkRelativeRefs { diff --git a/test_regress/t/t_lib_prot_shared.pl b/test_regress/t/t_lib_prot_shared.pl index a770be51b..1a3f8af5f 100755 --- a/test_regress/t/t_lib_prot_shared.pl +++ b/test_regress/t/t_lib_prot_shared.pl @@ -56,10 +56,10 @@ while (1) { compile( verilator_flags2 => ["$secret_dir/secret.sv", - ($Self->{vltmt} ? ' --threads 1' : ''), "-LDFLAGS", "'-Wl,-rpath,$abs_secret_dir -L$abs_secret_dir -l$secret_prefix'"], xsim_flags2 => ["$secret_dir/secret.sv"], + threads => $Self->{vltmt} ? 1 : 0 ); execute( diff --git a/test_regress/t/t_pgo_profoutofdate_bad.pl b/test_regress/t/t_pgo_profoutofdate_bad.pl index e2cfc96a1..3e78104a6 100755 --- a/test_regress/t/t_pgo_profoutofdate_bad.pl +++ b/test_regress/t/t_pgo_profoutofdate_bad.pl @@ -11,7 +11,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di scenarios(vltmt => 1); compile( - v_flags2 => ["--threads 2"], + threads => 2, fails => 1, expect_filename => $Self->{golden_filename}, ); diff --git a/test_regress/t/t_pgo_threads.pl b/test_regress/t/t_pgo_threads.pl index 24e300882..974bbe3c4 100755 --- a/test_regress/t/t_pgo_threads.pl +++ b/test_regress/t/t_pgo_threads.pl @@ -14,7 +14,8 @@ scenarios(vltmt => 1); top_filename("t/t_gen_alw.v"); compile( - v_flags2 => ["--prof-pgo --threads 2"] + v_flags2 => ["--prof-pgo"], + threads => 2 ); execute( @@ -30,8 +31,8 @@ file_grep("$Self->{obj_dir}/profile.vlt", qr/profile_data/i); compile( # Intentinally no --prof-pgo here to make sure profile data can be read in # without it (that is: --prof-pgo has no effect on profile_data hash names) - v_flags2 => ["--threads 2", - " $Self->{obj_dir}/profile.vlt"], + v_flags2 => [" $Self->{obj_dir}/profile.vlt"], + threads => 2 ); execute( diff --git a/test_regress/t/t_split_var_0.pl b/test_regress/t/t_split_var_0.pl index 5f07afe64..0441871e4 100755 --- a/test_regress/t/t_split_var_0.pl +++ b/test_regress/t/t_split_var_0.pl @@ -14,8 +14,8 @@ scenarios(simulator => 1); # %Warning-UNOPTTHREADS: Thread scheduler is unable to provide requested parallelism; consider asking for fewer threads. # So use 6 threads here though it's not optimal in performace wise, but ok. compile( - verilator_flags2 => ['--stats' . ($Self->{vltmt} ? ' --threads 6' : ''), - "$Self->{t_dir}/t_split_var_0.vlt"], + verilator_flags2 => ['--stats', "$Self->{t_dir}/t_split_var_0.vlt"], + threads => $Self->{vltmt} ? 6 : 0 ); execute( diff --git a/test_regress/t/t_split_var_2_trace.pl b/test_regress/t/t_split_var_2_trace.pl index c79ed778a..35d20007a 100755 --- a/test_regress/t/t_split_var_2_trace.pl +++ b/test_regress/t/t_split_var_2_trace.pl @@ -15,8 +15,8 @@ top_filename("t/t_split_var_0.v"); # %Warning-UNOPTTHREADS: Thread scheduler is unable to provide requested parallelism; consider asking for fewer threads. # So use 6 threads here though it's not optimal in performace wise, but ok. compile( - verilator_flags2 => ['--cc --trace --stats' . ($Self->{vltmt} ? ' --threads 6' : ''), - '+define+TEST_ATTRIBUTES'], + verilator_flags2 => ['--cc --trace --stats +define+TEST_ATTRIBUTES'], + threads => $Self->{vltmt} ? 6 : 0 ); execute( diff --git a/test_regress/t/t_threads_counter_0.pl b/test_regress/t/t_threads_counter_0.pl index 8ba463b4e..b93572929 100755 --- a/test_regress/t/t_threads_counter_0.pl +++ b/test_regress/t/t_threads_counter_0.pl @@ -13,7 +13,8 @@ scenarios(simulator => 1); top_filename("t/t_threads_counter.v"); compile( - verilator_flags2 => ['--cc --no-threads'], + verilator_flags2 => ['--cc'], + threads => 0, ); execute( diff --git a/test_regress/t/t_threads_counter_1.pl b/test_regress/t/t_threads_counter_1.pl index f1b1cf7b0..95a607bfd 100755 --- a/test_regress/t/t_threads_counter_1.pl +++ b/test_regress/t/t_threads_counter_1.pl @@ -13,7 +13,8 @@ scenarios(vltmt => 1); top_filename("t/t_threads_counter.v"); compile( - verilator_flags2 => ['--cc --threads 1'], + verilator_flags2 => ['--cc'], + threads => 1 ); execute( diff --git a/test_regress/t/t_threads_counter_2.pl b/test_regress/t/t_threads_counter_2.pl index 460c79651..3625322fe 100755 --- a/test_regress/t/t_threads_counter_2.pl +++ b/test_regress/t/t_threads_counter_2.pl @@ -13,7 +13,8 @@ scenarios(vltmt => 1); top_filename("t/t_threads_counter.v"); compile( - verilator_flags2 => ['--cc --threads 2'], + verilator_flags2 => ['--cc'], + threads => 2 ); execute( diff --git a/test_regress/t/t_threads_counter_4.pl b/test_regress/t/t_threads_counter_4.pl index 97ac33b97..20ed3feb9 100755 --- a/test_regress/t/t_threads_counter_4.pl +++ b/test_regress/t/t_threads_counter_4.pl @@ -13,7 +13,8 @@ scenarios(vltmt => 1); top_filename("t/t_threads_counter.v"); compile( - verilator_flags2 => ['--cc --threads 4'], + verilator_flags2 => ['--cc'], + threads => 4 ); execute( diff --git a/test_regress/t/t_threads_crazy.pl b/test_regress/t/t_threads_crazy.pl index ee8031054..6bb21acb0 100755 --- a/test_regress/t/t_threads_crazy.pl +++ b/test_regress/t/t_threads_crazy.pl @@ -15,7 +15,8 @@ if ($Self->cfg_with_m32) { } compile( - verilator_flags2 => ['--cc --threads 1024'], + verilator_flags2 => ['--cc'], + threads => 1024 ); execute( diff --git a/test_regress/t/t_threads_nondeterminism.pl b/test_regress/t/t_threads_nondeterminism.pl index 418a03fc6..b584fa62d 100755 --- a/test_regress/t/t_threads_nondeterminism.pl +++ b/test_regress/t/t_threads_nondeterminism.pl @@ -13,7 +13,8 @@ scenarios(vltmt => 1); top_filename("t/t_threads_counter.v"); compile( - verilator_flags2 => ['--cc --threads 2 --debug-nondeterminism --no-skip-identical'], + verilator_flags2 => ['--cc --debug-nondeterminism --no-skip-identical'], + threads => 2 ); execute( diff --git a/test_regress/t/t_trace_litendian.pl b/test_regress/t/t_trace_litendian.pl index 91cc2e48f..5b7289295 100755 --- a/test_regress/t/t_trace_litendian.pl +++ b/test_regress/t/t_trace_litendian.pl @@ -14,8 +14,8 @@ scenarios(simulator => 1); # %Warning-UNOPTTHREADS: Thread scheduler is unable to provide requested parallelism; consider asking for fewer threads. # Strangely, asking for more threads makes it go away. compile( - verilator_flags2 => ['--cc --trace --trace-params -Wno-LITENDIAN', - ($Self->{vltmt} ? '--threads 6' : '')], + verilator_flags2 => ['--cc --trace --trace-params -Wno-LITENDIAN'], + threads => $Self->{vltmt} ? 6 : 0 ); execute( diff --git a/test_regress/t/t_trace_litendian_fst.pl b/test_regress/t/t_trace_litendian_fst.pl index 4918c10bf..3e24bf901 100755 --- a/test_regress/t/t_trace_litendian_fst.pl +++ b/test_regress/t/t_trace_litendian_fst.pl @@ -16,8 +16,8 @@ top_filename("t/t_trace_litendian.v"); # %Warning-UNOPTTHREADS: Thread scheduler is unable to provide requested parallelism; consider asking for fewer threads. # Strangely, asking for more threads makes it go away. compile( - verilator_flags2 => ['--cc --trace-fst --trace-params -Wno-LITENDIAN', - ($Self->{vltmt} ? '--threads 6' : '')], + verilator_flags2 => ['--cc --trace-fst --trace-params -Wno-LITENDIAN'], + threads => $Self->{vltmt} ? 6 : 0 ); execute( diff --git a/test_regress/t/t_trace_litendian_fst_sc.pl b/test_regress/t/t_trace_litendian_fst_sc.pl index 56754b9df..b79e1e78f 100755 --- a/test_regress/t/t_trace_litendian_fst_sc.pl +++ b/test_regress/t/t_trace_litendian_fst_sc.pl @@ -20,8 +20,8 @@ else { # %Warning-UNOPTTHREADS: Thread scheduler is unable to provide requested parallelism; consider asking for fewer threads. # Strangely, asking for more threads makes it go away. compile( - verilator_flags2 => ['--sc --trace-fst --trace-params -Wno-LITENDIAN', - ($Self->{vltmt} ? '--threads 6' : '')], + verilator_flags2 => ['--sc --trace-fst --trace-params -Wno-LITENDIAN'], + threads => $Self->{vltmt} ? 6 : 0 ); execute( diff --git a/test_regress/t/t_verilated_all.pl b/test_regress/t/t_verilated_all.pl index 450b5bd9c..402f834d4 100755 --- a/test_regress/t/t_verilated_all.pl +++ b/test_regress/t/t_verilated_all.pl @@ -8,7 +8,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di # Version 2.0. # SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 -scenarios(vlt => 1); +scenarios(vltmt => 1); my $root = ".."; @@ -17,10 +17,10 @@ compile( verilator_flags2 => ["--cc", "--coverage-toggle --coverage-line --coverage-user", "--trace --vpi ", - "--threads 2", "--trace-threads 1", "--prof-exec", "--prof-pgo", "$root/include/verilated_save.cpp"], + threads => 2 ); execute( diff --git a/test_regress/t/t_verilated_threaded.pl b/test_regress/t/t_verilated_threaded.pl index ec8edeb50..a11df1cfe 100755 --- a/test_regress/t/t_verilated_threaded.pl +++ b/test_regress/t/t_verilated_threaded.pl @@ -16,7 +16,8 @@ my $root = ".."; compile( # Can't use --coverage and --savable together, so cheat and compile inline - verilator_flags2 => ["--cc --coverage-toggle --coverage-line --coverage-user --trace --threads 1 --vpi $root/include/verilated_save.cpp"], + verilator_flags2 => ["--cc --coverage-toggle --coverage-line --coverage-user --trace --vpi $root/include/verilated_save.cpp"], + threads => 1 ); execute( diff --git a/test_regress/t/t_wrapper_context.pl b/test_regress/t/t_wrapper_context.pl index e95e77250..30c3a78a3 100755 --- a/test_regress/t/t_wrapper_context.pl +++ b/test_regress/t/t_wrapper_context.pl @@ -14,8 +14,9 @@ compile( make_top_shell => 0, make_main => 0, # link threads library, add custom .cpp code, add tracing & coverage support - verilator_flags2 => ["-threads 1 --exe $Self->{t_dir}/$Self->{name}.cpp", + verilator_flags2 => ["--exe $Self->{t_dir}/$Self->{name}.cpp", "--trace --coverage -cc"], + threads => 1, make_flags => 'CPPFLAGS_ADD=-DVL_NO_LEGACY', ); diff --git a/test_regress/t/t_wrapper_context_fst.pl b/test_regress/t/t_wrapper_context_fst.pl index 8b40a7b2d..98b3ceb5a 100755 --- a/test_regress/t/t_wrapper_context_fst.pl +++ b/test_regress/t/t_wrapper_context_fst.pl @@ -16,8 +16,9 @@ compile( make_top_shell => 0, make_main => 0, # link threads library, add custom .cpp code, add tracing & coverage support - verilator_flags2 => ["-threads 1 --exe $Self->{t_dir}/t_wrapper_context.cpp", + verilator_flags2 => ["--exe $Self->{t_dir}/t_wrapper_context.cpp", "--trace-fst --coverage -cc"], + threads => 1, make_flags => 'CPPFLAGS_ADD=-DVL_NO_LEGACY', ); diff --git a/test_regress/t/t_wrapper_context_seq.pl b/test_regress/t/t_wrapper_context_seq.pl index 8ddf958f8..162a0c596 100755 --- a/test_regress/t/t_wrapper_context_seq.pl +++ b/test_regress/t/t_wrapper_context_seq.pl @@ -16,8 +16,9 @@ compile( make_top_shell => 0, make_main => 0, # link threads library, add custom .cpp code, add tracing & coverage support - verilator_flags2 => ["-threads 1 --exe $Self->{t_dir}/t_wrapper_context.cpp", + verilator_flags2 => ["--exe $Self->{t_dir}/t_wrapper_context.cpp", "--trace --coverage -cc"], + threads => 1, make_flags => 'CPPFLAGS_ADD=-DVL_NO_LEGACY', ); From 0de1bbc85b281f811e3f539b2b297d39d2c31eac Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Tue, 5 Jul 2022 14:20:37 +0100 Subject: [PATCH 03/12] Add and use VL_CONSTEXPR_CXX17 --- include/verilatedos.h | 10 ++++++++++ src/V3Ast.h | 8 ++++---- src/V3AstUserAllocator.h | 27 ++++++++++++--------------- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/include/verilatedos.h b/include/verilatedos.h index 6bacfe27b..2b0cdd8ce 100644 --- a/include/verilatedos.h +++ b/include/verilatedos.h @@ -234,6 +234,16 @@ # error "Verilator requires a C++11 or newer compiler" #endif +//========================================================================= +// C++-2017 + +#if __cplusplus >= 201703L +# define VL_CONSTEXPR_CXX17 constexpr +#else +# define VL_CONSTEXPR_CXX17 +#endif + + //========================================================================= // Optimization diff --git a/src/V3Ast.h b/src/V3Ast.h index dc4cf6d8e..868fc73f8 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -1948,7 +1948,7 @@ private: ASTNODE_PREFETCH(nodep->op2p()); ASTNODE_PREFETCH(nodep->op3p()); ASTNODE_PREFETCH(nodep->op4p()); - if /* TODO: 'constexpr' in C++17 */ (VisitNext) ASTNODE_PREFETCH(nodep->nextp()); + if VL_CONSTEXPR_CXX17 (VisitNext) ASTNODE_PREFETCH(nodep->nextp()); // Apply function in pre-order if (privateTypeTest::type>(nodep)) { @@ -1964,7 +1964,7 @@ private: } // Traverse 'nextp()' chain if requested - if /* TODO: 'constexpr' in C++17 */ (VisitNext) { + if VL_CONSTEXPR_CXX17 (VisitNext) { nodep = nodep->nextp(); } else { break; @@ -1987,7 +1987,7 @@ private: ASTNODE_PREFETCH(nodep->op2p()); ASTNODE_PREFETCH(nodep->op3p()); ASTNODE_PREFETCH(nodep->op4p()); - if /* TODO: 'constexpr' in C++17 */ (VisitNext) ASTNODE_PREFETCH(nodep->nextp()); + if VL_CONSTEXPR_CXX17 (VisitNext) ASTNODE_PREFETCH(nodep->nextp()); // Apply function in pre-order if (privateTypeTest::type>(nodep)) { @@ -2011,7 +2011,7 @@ private: } // Traverse 'nextp()' chain if requested - if /* TODO: 'constexpr' in C++17 */ (VisitNext) { + if VL_CONSTEXPR_CXX17 (VisitNext) { nodep = nodep->nextp(); } else { break; diff --git a/src/V3AstUserAllocator.h b/src/V3AstUserAllocator.h index 8d63ad5a9..f8982bf16 100644 --- a/src/V3AstUserAllocator.h +++ b/src/V3AstUserAllocator.h @@ -35,17 +35,16 @@ private: std::vector m_allocated; inline T_Data* getUserp(const T_Node* nodep) const { - // This simplifies statically as T_UserN is constant. In C++17, use 'if constexpr'. - if (T_UserN == 1) { + if VL_CONSTEXPR_CXX17 (T_UserN == 1) { const VNUser user = nodep->user1u(); return user.to(); - } else if (T_UserN == 2) { + } else if VL_CONSTEXPR_CXX17 (T_UserN == 2) { const VNUser user = nodep->user2u(); return user.to(); - } else if (T_UserN == 3) { + } else if VL_CONSTEXPR_CXX17 (T_UserN == 3) { const VNUser user = nodep->user3u(); return user.to(); - } else if (T_UserN == 4) { + } else if VL_CONSTEXPR_CXX17 (T_UserN == 4) { const VNUser user = nodep->user4u(); return user.to(); } else { @@ -55,14 +54,13 @@ private: } inline void setUserp(T_Node* nodep, T_Data* userp) const { - // This simplifies statically as T_UserN is constant. In C++17, use 'if constexpr'. - if (T_UserN == 1) { + if VL_CONSTEXPR_CXX17 (T_UserN == 1) { nodep->user1u(VNUser(userp)); - } else if (T_UserN == 2) { + } else if VL_CONSTEXPR_CXX17 (T_UserN == 2) { nodep->user2u(VNUser(userp)); - } else if (T_UserN == 3) { + } else if VL_CONSTEXPR_CXX17 (T_UserN == 3) { nodep->user3u(VNUser(userp)); - } else if (T_UserN == 4) { + } else if VL_CONSTEXPR_CXX17 (T_UserN == 4) { nodep->user4u(VNUser(userp)); } else { nodep->user5u(VNUser(userp)); @@ -71,14 +69,13 @@ private: protected: AstUserAllocatorBase() { - // This simplifies statically as T_UserN is constant. In C++17, use 'if constexpr'. - if (T_UserN == 1) { + if VL_CONSTEXPR_CXX17 (T_UserN == 1) { VNUser1InUse::check(); - } else if (T_UserN == 2) { + } else if VL_CONSTEXPR_CXX17 (T_UserN == 2) { VNUser2InUse::check(); - } else if (T_UserN == 3) { + } else if VL_CONSTEXPR_CXX17 (T_UserN == 3) { VNUser3InUse::check(); - } else if (T_UserN == 4) { + } else if VL_CONSTEXPR_CXX17 (T_UserN == 4) { VNUser4InUse::check(); } else { VNUser5InUse::check(); From 9f37cef1bb5d3f87b8873abbfec69b3773b15f96 Mon Sep 17 00:00:00 2001 From: Yutetsu TAKATSUKASA Date: Wed, 6 Jul 2022 08:33:37 +0900 Subject: [PATCH 04/12] Fix #3470 of incorrect bit op tree optimization (#3476) * Tests: Add a test to reproduce #3470 * Update LSB during return path of traversal. No functional change is intended. * Introduce LeafInfo::m_msb * Update LeafInfo::m_msb when visitin AstCCast * Internals: Add comment, reorder. No functional change is intended. * Delete explicit from copy constructor to fix build error. * Update Changes * Internals: Remove unused parameter. No functional change is intended. * Tests: Add explanation to t_const_opt. --- Changes | 2 +- src/V3Const.cpp | 61 ++++++++++++++++++++++++----------- test_regress/t/t_const_opt.pl | 2 +- test_regress/t/t_const_opt.v | 36 +++++++++++++++++++-- 4 files changed, 79 insertions(+), 22 deletions(-) diff --git a/Changes b/Changes index b597d565f..2e1183c7b 100644 --- a/Changes +++ b/Changes @@ -13,7 +13,7 @@ Verilator 4.225 devel **Minor:** - +* Fix incorrect bit op tree optimization (#3470). [algrobman] Verilator 4.224 2022-06-19 ========================== diff --git a/src/V3Const.cpp b/src/V3Const.cpp index 7dae3f014..700a99e38 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -80,30 +80,48 @@ class ConstBitOpTreeVisitor final : public VNVisitor { using ResultTerm = std::tuple; class LeafInfo final { // Leaf node (either AstConst or AstVarRef) + // MEMBERS bool m_polarity = true; - int m_lsb = 0; + int m_lsb = 0; // LSB of actually used bit of m_refp->varp() + int m_msb = 0; // MSB of actually used bit of m_refp->varp() int m_wordIdx = -1; // -1 means AstWordSel is not used. AstVarRef* m_refp = nullptr; const AstConst* m_constp = nullptr; public: + // CONSTRUCTORS + LeafInfo() = default; + LeafInfo(const LeafInfo& other) = default; + explicit LeafInfo(int lsb) + : m_lsb{lsb} {} + + // METHODS void setLeaf(AstVarRef* refp) { UASSERT(!m_refp && !m_constp, "Must be called just once"); m_refp = refp; + m_msb = refp->varp()->widthMin() - 1; } void setLeaf(const AstConst* constp) { UASSERT(!m_refp && !m_constp, "Must be called just once"); m_constp = constp; + m_msb = constp->widthMin() - 1; } + void updateBitRange(const AstCCast* castp) { + m_msb = std::min(m_msb, m_lsb + castp->width() - 1); + } + void updateBitRange(const AstShiftR* shiftp) { + m_lsb += VN_AS(shiftp->rhsp(), Const)->toUInt(); + } + void wordIdx(int i) { m_wordIdx = i; } + void polarity(bool p) { m_polarity = p; } + AstVarRef* refp() const { return m_refp; } const AstConst* constp() const { return m_constp; } int wordIdx() const { return m_wordIdx; } bool polarity() const { return m_polarity; } int lsb() const { return m_lsb; } - void wordIdx(int i) { m_wordIdx = i; } - void lsb(int l) { m_lsb = l; } - void polarity(bool p) { m_polarity = p; } + int msb() const { return std::min(m_msb, varWidth() - 1); } int varWidth() const { UASSERT(m_refp, "m_refp should be set"); const int width = m_refp->varp()->widthMin(); @@ -382,7 +400,7 @@ class ConstBitOpTreeVisitor final : public VNVisitor { // Traverse down to see AstConst or AstVarRef LeafInfo findLeaf(AstNode* nodep, bool expectConst) { - LeafInfo info; + LeafInfo info{m_lsb}; { VL_RESTORER(m_leafp); m_leafp = &info; @@ -402,7 +420,10 @@ class ConstBitOpTreeVisitor final : public VNVisitor { virtual void visit(AstNode* nodep) override { CONST_BITOP_SET_FAILED("Hit unexpected op", nodep); } - virtual void visit(AstCCast* nodep) override { iterateChildren(nodep); } + virtual void visit(AstCCast* nodep) override { + iterateChildren(nodep); + if (m_leafp) m_leafp->updateBitRange(nodep); + } virtual void visit(AstShiftR* nodep) override { CONST_BITOP_RETURN_IF(!m_leafp, nodep); AstConst* const constp = VN_CAST(nodep->rhsp(), Const); @@ -410,12 +431,14 @@ class ConstBitOpTreeVisitor final : public VNVisitor { m_lsb += constp->toUInt(); incrOps(nodep, __LINE__); iterate(nodep->lhsp()); + m_leafp->updateBitRange(nodep); m_lsb -= constp->toUInt(); } virtual void visit(AstNot* nodep) override { CONST_BITOP_RETURN_IF(nodep->widthMin() != 1, nodep); AstNode* lhsp = nodep->lhsp(); - if (AstCCast* const castp = VN_CAST(lhsp, CCast)) lhsp = castp->lhsp(); + AstCCast* const castp = VN_CAST(lhsp, CCast); + if (castp) lhsp = castp->lhsp(); CONST_BITOP_RETURN_IF(!VN_IS(lhsp, VarRef) && !VN_IS(lhsp, Xor) && !VN_IS(lhsp, RedXor) && !VN_IS(lhsp, ShiftR), lhsp); @@ -424,6 +447,7 @@ class ConstBitOpTreeVisitor final : public VNVisitor { iterateChildren(nodep); // Don't restore m_polarity for Xor as it counts parity of the entire tree if (!isXorTree()) m_polarity = !m_polarity; + if (m_leafp && castp) m_leafp->updateBitRange(castp); } virtual void visit(AstWordSel* nodep) override { CONST_BITOP_RETURN_IF(!m_leafp, nodep); @@ -437,27 +461,27 @@ class ConstBitOpTreeVisitor final : public VNVisitor { CONST_BITOP_RETURN_IF(!m_leafp, nodep); m_leafp->setLeaf(nodep); m_leafp->polarity(m_polarity); - m_leafp->lsb(m_lsb); } virtual void visit(AstConst* nodep) override { CONST_BITOP_RETURN_IF(!m_leafp, nodep); m_leafp->setLeaf(nodep); - m_leafp->lsb(m_lsb); } virtual void visit(AstRedXor* nodep) override { Restorer restorer{*this}; CONST_BITOP_RETURN_IF(!VN_IS(m_rootp, Xor), nodep); AstNode* lhsp = nodep->lhsp(); - if (const AstCCast* const castp = VN_CAST(lhsp, CCast)) lhsp = castp->lhsp(); + const AstCCast* const castp = VN_CAST(lhsp, CCast); + if (castp) lhsp = castp->lhsp(); if (const AstAnd* const andp = VN_CAST(lhsp, And)) { // '^(mask & leaf)' CONST_BITOP_RETURN_IF(!andp, lhsp); const LeafInfo& mask = findLeaf(andp->lhsp(), true); CONST_BITOP_RETURN_IF(!mask.constp() || mask.lsb() != 0, andp->lhsp()); - const LeafInfo& ref = findLeaf(andp->rhsp(), false); + LeafInfo ref = findLeaf(andp->rhsp(), false); CONST_BITOP_RETURN_IF(!ref.refp(), andp->rhsp()); + if (castp) ref.updateBitRange(castp); restorer.disableRestore(); // Now all subtree succeeded @@ -467,7 +491,7 @@ class ConstBitOpTreeVisitor final : public VNVisitor { incrOps(andp, __LINE__); // Mark all bits checked in this reduction - const int maxBitIdx = std::min(ref.lsb() + maskNum.width(), ref.varWidth()); + const int maxBitIdx = std::min(ref.lsb() + maskNum.width(), ref.msb() + 1); for (int bitIdx = ref.lsb(); bitIdx < maxBitIdx; ++bitIdx) { const int maskIdx = bitIdx - ref.lsb(); if (maskNum.bitIs0(maskIdx)) continue; @@ -475,15 +499,16 @@ class ConstBitOpTreeVisitor final : public VNVisitor { m_bitPolarities.emplace_back(ref, true, bitIdx); } } else { // '^leaf' - const LeafInfo& ref = findLeaf(lhsp, false); + LeafInfo ref = findLeaf(lhsp, false); CONST_BITOP_RETURN_IF(!ref.refp(), lhsp); + if (castp) ref.updateBitRange(castp); restorer.disableRestore(); // Now all checks passed incrOps(nodep, __LINE__); // Mark all bits checked by this comparison - for (int bitIdx = ref.lsb(); bitIdx < ref.varWidth(); ++bitIdx) { + for (int bitIdx = ref.lsb(); bitIdx <= ref.msb(); ++bitIdx) { m_bitPolarities.emplace_back(ref, true, bitIdx); } } @@ -503,7 +528,7 @@ class ConstBitOpTreeVisitor final : public VNVisitor { for (const bool right : {false, true}) { Restorer restorer{*this}; - LeafInfo leafInfo; + LeafInfo leafInfo{m_lsb}; m_leafp = &leafInfo; AstNode* opp = right ? nodep->rhsp() : nodep->lhsp(); const bool origFailed = m_failed; @@ -522,7 +547,7 @@ class ConstBitOpTreeVisitor final : public VNVisitor { // The conditional on the lsb being in range is necessary for some degenerate // case, e.g.: (IData)((QData)wide[0] >> 32), or <1-bit-var> >> 1, which is // just zero - if (leafInfo.lsb() < leafInfo.varWidth()) { + if (leafInfo.lsb() <= leafInfo.msb()) { m_bitPolarities.emplace_back(leafInfo, isXorTree() || leafInfo.polarity(), leafInfo.lsb()); } else if (isAndTree() && leafInfo.polarity()) { @@ -559,7 +584,7 @@ class ConstBitOpTreeVisitor final : public VNVisitor { incrOps(andp, __LINE__); // Mark all bits checked by this comparison - const int maxBitIdx = std::min(ref.lsb() + maskNum.width(), ref.varWidth()); + const int maxBitIdx = std::min(ref.lsb() + maskNum.width(), ref.msb() + 1); for (int bitIdx = ref.lsb(); bitIdx < maxBitIdx; ++bitIdx) { const int maskIdx = bitIdx - ref.lsb(); if (maskNum.bitIs0(maskIdx)) continue; @@ -575,7 +600,7 @@ class ConstBitOpTreeVisitor final : public VNVisitor { incrOps(nodep, __LINE__); // Mark all bits checked by this comparison - const int maxBitIdx = std::min(ref.lsb() + compNum.width(), ref.varWidth()); + const int maxBitIdx = std::min(ref.lsb() + compNum.width(), ref.msb() + 1); for (int bitIdx = ref.lsb(); bitIdx < maxBitIdx; ++bitIdx) { const int maskIdx = bitIdx - ref.lsb(); const bool polarity = compNum.bitIs1(maskIdx) != maskFlip; diff --git a/test_regress/t/t_const_opt.pl b/test_regress/t/t_const_opt.pl index 83e301744..837b5f74f 100755 --- a/test_regress/t/t_const_opt.pl +++ b/test_regress/t/t_const_opt.pl @@ -19,7 +19,7 @@ execute( ); if ($Self->{vlt}) { - file_grep($Self->{stats}, qr/Optimizations, Const bit op reduction\s+(\d+)/i, 11); + file_grep($Self->{stats}, qr/Optimizations, Const bit op reduction\s+(\d+)/i, 14); } ok(1); 1; diff --git a/test_regress/t/t_const_opt.v b/test_regress/t/t_const_opt.v index 407fef13c..e24d28bf8 100644 --- a/test_regress/t/t_const_opt.v +++ b/test_regress/t/t_const_opt.v @@ -62,7 +62,7 @@ module t(/*AUTOARG*/ $write("[%0t] cyc==%0d crc=%x sum=%x\n", $time, cyc, crc, sum); if (crc !== 64'hc77bb9b3784ea091) $stop; // What checksum will we end up with (above print should match) -`define EXPECTED_SUM 64'hdccb9e7b8b638233 +`define EXPECTED_SUM 64'hde21e019a3e12039 if (sum !== `EXPECTED_SUM) $stop; $write("*-* All Finished *-*\n"); @@ -86,10 +86,11 @@ module Test(/*AUTOARG*/ logic bug3182_out; logic bug3197_out; logic bug3445_out; + logic bug3470_out; output logic o; - logic [7:0] tmp; + logic [8:0] tmp; assign o = ^tmp; always_ff @(posedge clk) begin @@ -113,11 +114,13 @@ module Test(/*AUTOARG*/ tmp[5] <= bug3182_out; tmp[6] <= bug3197_out; tmp[7] <= bug3445_out; + tmp[8] <= bug3470_out; end bug3182 i_bug3182(.in(d[4:0]), .out(bug3182_out)); bug3197 i_bug3197(.clk(clk), .in(d), .out(bug3197_out)); bug3445 i_bug3445(.clk(clk), .in(d), .out(bug3445_out)); + bug3470 i_bug3470(.clk(clk), .in(d), .out(bug3470_out)); endmodule @@ -203,3 +206,32 @@ module bug3445(input wire clk, input wire [31:0] in, output wire out); assign out = result0 ^ result1 ^ (result2 | result3); endmodule + +// Bug3470 +// CCast had been ignored in bit op tree optimization +// Assume the following HDL input: +// (^d[38:32]) ^ (^d[31:0]) +// where d is logic [38:0] +// ^d[31:0] becomes REDXOR(CCast(uint32_t, d)), +// but CCast was ignored and interpreted as ^d[38:0]. +// Finally (^d[38:32]) ^ (^d31:0]) was wrongly transformed to +// (^d[38:32]) ^ (^d[38:0]) +// -> (^d[38:32]) ^ ((^d[38:32]) ^ (^d[31:0])) +// -> ^d[31:0] +// Of course the correct result is ^d[38:0] = ^d +module bug3470(input wire clk, input wire [31:0] in, output wire out); + logic [38:0] d; + always_ff @(posedge clk) + d <= {d[6:0], in}; + + logic tmp, expected; + always_ff @(posedge clk) begin + tmp <= ^(d >> 32) ^ (^d[31:0]); + expected <= ^d; + end + + always @(posedge clk) + if (tmp != expected) $stop; + + assign out = tmp; +endmodule From 3d71716a8a6cc7a931abd78b9d7930a68e9f3b03 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 9 Jul 2022 07:28:28 -0400 Subject: [PATCH 05/12] Internals: Constructor style cleanup. No functional change. --- src/V3Simulate.h | 26 +++++++++++++------------- src/V3Table.cpp | 36 ++++++++++++++++++------------------ 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/V3Simulate.h b/src/V3Simulate.h index d5a7453ac..28691d478 100644 --- a/src/V3Simulate.h +++ b/src/V3Simulate.h @@ -15,7 +15,7 @@ //************************************************************************* // // void example_usage() { -// SimulateVisitor simvis (false, false); +// SimulateVisitor simvis{false, false}; // simvis.clear(); // // Set all inputs to the constant // for (deque::iterator it = m_inVarps.begin(); it!=m_inVarps.end(); ++it) { @@ -130,7 +130,7 @@ private: const int width = itemp->width(); const int lsb = itemp->lsb(); const int msb = lsb + width - 1; - V3Number fieldNum(nump, width); + V3Number fieldNum{nump, width}; fieldNum.opSel(*nump, msb, lsb); out << itemp->name() << ": "; if (AstNodeDType* const childTypep = itemp->subDTypep()) { @@ -152,7 +152,7 @@ private: const int width = childTypep->width(); const int lsb = width * element; const int msb = lsb + width - 1; - V3Number fieldNum(nump, width); + V3Number fieldNum{nump, width}; fieldNum.opSel(*nump, msb, lsb); const int arrayElem = arrayp->lo() + element; out << arrayElem << " = " << prettyNumber(&fieldNum, childTypep); @@ -236,7 +236,7 @@ private: } if (allocNewConst) { // Need to allocate new constant - constp = new AstConst(nodep->fileline(), AstConst::DtypedValue(), nodep->dtypep(), 0); + constp = new AstConst{nodep->fileline(), AstConst::DtypedValue{}, nodep->dtypep(), 0}; // Mark as in use, add to free list for later reuse constp->user2(1); freeList.push_back(constp); @@ -683,15 +683,15 @@ private: initp = vscpnump; } else { // Assignment to unassigned variable, all bits are X // TODO generic initialization which builds X/arrays by recursion - AstConst* const outconstp = new AstConst( - nodep->fileline(), AstConst::WidthedValue(), basicp->widthMin(), 0); + AstConst* const outconstp = new AstConst{ + nodep->fileline(), AstConst::WidthedValue{}, basicp->widthMin(), 0}; if (basicp->isZeroInit()) { outconstp->num().setAllBits0(); } else { outconstp->num().setAllBitsX(); } - initp = new AstInitArray(nodep->fileline(), arrayp, outconstp); + initp = new AstInitArray{nodep->fileline(), arrayp, outconstp}; m_reclaimValuesp.push_back(initp); } const uint32_t index = fetchConst(selp->bitp())->toUInt(); @@ -706,7 +706,7 @@ private: } void handleAssignSel(AstNodeAssign* nodep, AstSel* selp) { AstVarRef* varrefp = nullptr; - V3Number lsb(nodep); + V3Number lsb{nodep}; iterateAndNextNull(nodep->rhsp()); // Value to assign handleAssignSelRecurse(nodep, selp, varrefp /*ref*/, lsb /*ref*/, 0); if (!m_checkOnly && optimizable()) { @@ -719,8 +719,8 @@ private: } else if (AstConst* const vscpnump = fetchConstNull(vscp)) { outconstp = vscpnump; } else { // Assignment to unassigned variable, all bits are X or 0 - outconstp = new AstConst(nodep->fileline(), AstConst::WidthedValue(), - varrefp->varp()->widthMin(), 0); + outconstp = new AstConst{nodep->fileline(), AstConst::WidthedValue{}, + varrefp->varp()->widthMin(), 0}; if (varrefp->varp()->basicp() && varrefp->varp()->basicp()->isZeroInit()) { outconstp->num().setAllBits0(); } else { @@ -742,7 +742,7 @@ private: lsbRef = fetchConst(selp->lsbp())->num(); return; // And presumably still optimizable() } else if (AstSel* const subselp = VN_CAST(selp->lhsp(), Sel)) { - V3Number sublsb(nodep); + V3Number sublsb{nodep}; handleAssignSelRecurse(nodep, subselp, outVarrefpRef, sublsb /*ref*/, depth + 1); if (optimizable()) { lsbRef = sublsb; @@ -829,7 +829,7 @@ private: if (hit) break; iterateAndNextNull(ep); if (optimizable()) { - V3Number match(nodep, 1); + V3Number match{nodep, 1}; match.opEq(fetchConst(nodep->exprp())->num(), fetchConst(ep)->num()); if (match.isNeqZero()) { iterateAndNextNull(itemp->bodysp()); @@ -1097,7 +1097,7 @@ private: } AstConst* const resultConstp - = new AstConst(nodep->fileline(), AstConst::String(), result); + = new AstConst{nodep->fileline(), AstConst::String{}, result}; setValue(nodep, resultConstp); m_reclaimValuesp.push_back(resultConstp); } diff --git a/src/V3Table.cpp b/src/V3Table.cpp index 822407b4c..0944bb589 100644 --- a/src/V3Table.cpp +++ b/src/V3Table.cpp @@ -92,8 +92,8 @@ public: = elemDType->isString() ? elemDType : v3Global.rootp()->findBitDType(width, width, VSigning::UNSIGNED); - AstUnpackArrayDType* const tableDTypep - = new AstUnpackArrayDType(m_fl, subDTypep, new AstRange(m_fl, size, 0)); + AstUnpackArrayDType* const tableDTypep = new AstUnpackArrayDType{ + m_fl, subDTypep, new AstRange{m_fl, static_cast(size), 0}}; v3Global.rootp()->typeTablep()->addTypesp(tableDTypep); // Create table initializer (with default value 0) AstConst* const defaultp = elemDType->isString() @@ -106,7 +106,7 @@ public: UASSERT_OBJ(!m_varScopep, m_fl, "Table variable already created"); // Default value is zero/empty string so don't add it if (value.isString() ? value.toString().empty() : value.isEqZero()) return; - m_initp->addIndexValuep(index, new AstConst(m_fl, value)); + m_initp->addIndexValuep(index, new AstConst{m_fl, value}); } AstVarScope* varScopep() { @@ -247,14 +247,14 @@ private: // We will need a table index variable, create it here. AstVar* const indexVarp - = new AstVar(fl, VVarType::BLOCKTEMP, "__Vtableidx" + cvtToStr(m_modTables), - VFlagBitPacked(), m_inWidthBits); + = new AstVar{fl, VVarType::BLOCKTEMP, "__Vtableidx" + cvtToStr(m_modTables), + VFlagBitPacked{}, static_cast(m_inWidthBits)}; m_modp->addStmtp(indexVarp); - AstVarScope* const indexVscp = new AstVarScope(indexVarp->fileline(), m_scopep, indexVarp); + AstVarScope* const indexVscp = new AstVarScope{indexVarp->fileline(), m_scopep, indexVarp}; m_scopep->addVarp(indexVscp); // The 'output assigned' table builder - TableBuilder outputAssignedTableBuilder(fl); + TableBuilder outputAssignedTableBuilder{fl}; outputAssignedTableBuilder.setTableSize( nodep->findBitDType(m_outVarps.size(), m_outVarps.size(), VSigning::UNSIGNED), VL_MASK_I(m_inWidthBits)); @@ -311,7 +311,7 @@ private: << simvis.whyNotMessage()); // Build output value tables and the assigned flags table - V3Number outputAssignedMask(nodep, m_outVarps.size(), 0); + V3Number outputAssignedMask{nodep, static_cast(m_outVarps.size()), 0}; for (TableOutputVar& tov : m_outVarps) { if (V3Number* const outnump = simvis.fetchOutNumberNull(tov.varScopep())) { UINFO(8, " Output " << tov.name() << " = " << *outnump << endl); @@ -333,21 +333,21 @@ private: // First var in inVars becomes the LSB of the concat AstNode* concatp = nullptr; for (AstVarScope* invscp : m_inVarps) { - AstVarRef* const refp = new AstVarRef(fl, invscp, VAccess::READ); + AstVarRef* const refp = new AstVarRef{fl, invscp, VAccess::READ}; if (concatp) { - concatp = new AstConcat(fl, refp, concatp); + concatp = new AstConcat{fl, refp, concatp}; } else { concatp = refp; } } - return new AstAssign(fl, new AstVarRef(fl, indexVscp, VAccess::WRITE), concatp); + return new AstAssign{fl, new AstVarRef{fl, indexVscp, VAccess::WRITE}, concatp}; } AstArraySel* select(FileLine* fl, AstVarScope* fromp, AstVarScope* indexp) { - AstVarRef* const fromRefp = new AstVarRef(fl, fromp, VAccess::READ); - AstVarRef* const indexRefp = new AstVarRef(fl, indexp, VAccess::READ); - return new AstArraySel(fl, fromRefp, indexRefp); + AstVarRef* const fromRefp = new AstVarRef{fl, fromp, VAccess::READ}; + AstVarRef* const indexRefp = new AstVarRef{fl, indexp, VAccess::READ}; + return new AstArraySel{fl, fromRefp, indexRefp}; } void createOutputAssigns(AstNode* nodep, AstNode* stmtsp, AstVarScope* indexVscp, @@ -362,12 +362,12 @@ private: // If this output is unassigned on some code paths, wrap the assignment in an If if (tov.mayBeUnassigned()) { - V3Number outputChgMask(nodep, m_outVarps.size(), 0); + V3Number outputChgMask{nodep, static_cast(m_outVarps.size()), 0}; outputChgMask.setBit(tov.ord(), 1); AstNode* const condp - = new AstAnd(fl, select(fl, outputAssignedTableVscp, indexVscp), - new AstConst(fl, outputChgMask)); - outsetp = new AstIf(fl, condp, outsetp); + = new AstAnd{fl, select(fl, outputAssignedTableVscp, indexVscp), + new AstConst{fl, outputChgMask}}; + outsetp = new AstIf{fl, condp, outsetp}; } stmtsp->addNext(outsetp); From a4fddb3fbee12417257c29b95b2b82258b6c5c21 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 9 Jul 2022 07:55:46 -0400 Subject: [PATCH 06/12] Fix table misoptimizing away display (#3488). --- Changes | 2 ++ src/V3Simulate.h | 14 ++++++-- src/V3Table.cpp | 3 ++ test_regress/t/t_opt_table_display.out | 12 +++++++ test_regress/t/t_opt_table_display.pl | 23 ++++++++++++++ test_regress/t/t_opt_table_display.v | 44 ++++++++++++++++++++++++++ 6 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 test_regress/t/t_opt_table_display.out create mode 100755 test_regress/t/t_opt_table_display.pl create mode 100644 test_regress/t/t_opt_table_display.v diff --git a/Changes b/Changes index 2e1183c7b..5f68d4a40 100644 --- a/Changes +++ b/Changes @@ -14,6 +14,8 @@ Verilator 4.225 devel **Minor:** * Fix incorrect bit op tree optimization (#3470). [algrobman] +* Fix table misoptimizing away display (#3488). [Stefan Post] + Verilator 4.224 2022-06-19 ========================== diff --git a/src/V3Simulate.h b/src/V3Simulate.h index 28691d478..195139cd2 100644 --- a/src/V3Simulate.h +++ b/src/V3Simulate.h @@ -100,6 +100,7 @@ private: bool m_anyAssignDly; ///< True if found a delayed assignment bool m_anyAssignComb; ///< True if found a non-delayed assignment bool m_inDlyAssign; ///< Under delayed assignment + bool m_isOutputter; // Creates output int m_instrCount; ///< Number of nodes int m_dataCount; ///< Bytes of data AstJumpGo* m_jumpp; ///< Jump label we're branching from @@ -205,6 +206,7 @@ public: AstNode* whyNotNodep() const { return m_whyNotNodep; } bool isAssignDly() const { return m_anyAssignDly; } + bool isOutputter() const { return m_isOutputter; } int instrCount() const { return m_instrCount; } int dataCount() const { return m_dataCount; } @@ -342,15 +344,16 @@ private: nodep->user2p((void*)valuep); } - void checkNodeInfo(AstNode* nodep) { + void checkNodeInfo(AstNode* nodep, bool ignorePredict = false) { if (m_checkOnly) { m_instrCount += nodep->instrCount(); m_dataCount += nodep->width(); } - if (!nodep->isPredictOptimizable()) { + if (!ignorePredict && !nodep->isPredictOptimizable()) { // UINFO(9, " !predictopt " << nodep << endl); clearOptimizable(nodep, "Isn't predictable"); } + if (nodep->isOutputter()) m_isOutputter = true; } void badNodeType(AstNode* nodep) { @@ -756,6 +759,7 @@ private: virtual void visit(AstNodeAssign* nodep) override { if (jumpingOver(nodep)) return; if (!optimizable()) return; // Accelerate + checkNodeInfo(nodep); if (VN_IS(nodep, AssignForce)) { clearOptimizable(nodep, "Force"); } else if (VN_IS(nodep, AssignDly)) { @@ -970,6 +974,7 @@ private: if (jumpingOver(nodep)) return; if (!optimizable()) return; // Accelerate UINFO(5, " FUNCREF " << nodep << endl); + checkNodeInfo(nodep); if (!m_params) { badNodeType(nodep); return; @@ -1053,6 +1058,7 @@ private: virtual void visit(AstSFormatF* nodep) override { if (jumpingOver(nodep)) return; if (!optimizable()) return; // Accelerate + checkNodeInfo(nodep); iterateChildren(nodep); if (m_params) { AstNode* nextArgp = nodep->exprsp(); @@ -1106,6 +1112,9 @@ private: virtual void visit(AstDisplay* nodep) override { if (jumpingOver(nodep)) return; if (!optimizable()) return; // Accelerate + // We ignore isPredictOptimizable as $display is often in constant + // functions and we want them to work if used with parameters + checkNodeInfo(nodep, /*display:*/ true); iterateChildren(nodep); if (m_params) { AstConst* const textp = fetchConst(nodep->fmtp()); @@ -1155,6 +1164,7 @@ public: m_anyAssignComb = false; m_anyAssignDly = false; m_inDlyAssign = false; + m_isOutputter = false; m_instrCount = 0; m_dataCount = 0; m_jumpp = nullptr; diff --git a/src/V3Table.cpp b/src/V3Table.cpp index 0944bb589..30a9cb587 100644 --- a/src/V3Table.cpp +++ b/src/V3Table.cpp @@ -225,6 +225,9 @@ private: if (!m_outWidthBytes || !m_inWidthBits) { chkvis.clearOptimizable(nodep, "Table has no outputs"); } + if (chkvis.isOutputter()) { + chkvis.clearOptimizable(nodep, "Table creates display output"); + } UINFO(4, " Test: Opt=" << (chkvis.optimizable() ? "OK" : "NO") << ", Instrs=" << chkvis.instrCount() << " Data=" << chkvis.dataCount() << " in width (bits)=" << m_inWidthBits << " out width (bytes)=" diff --git a/test_regress/t/t_opt_table_display.out b/test_regress/t/t_opt_table_display.out new file mode 100644 index 000000000..3bbd99b6b --- /dev/null +++ b/test_regress/t/t_opt_table_display.out @@ -0,0 +1,12 @@ +Clocked +Clocked +Clocked +Clocked +Clocked +Clocked +Clocked +Clocked +Clocked +Clocked +Clocked +*-* All Finished *-* diff --git a/test_regress/t/t_opt_table_display.pl b/test_regress/t/t_opt_table_display.pl new file mode 100755 index 000000000..e08f4a744 --- /dev/null +++ b/test_regress/t/t_opt_table_display.pl @@ -0,0 +1,23 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 by Wilson Snyder. This program is free software; you +# can redistribute it and/or modify it under the terms of either the GNU +# Lesser General Public License Version 3 or the Perl Artistic License +# Version 2.0. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(simulator => 1); + +compile( + verilator_flags2 => ["--stats"], + ); + +execute( + check_finished => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_opt_table_display.v b/test_regress/t/t_opt_table_display.v new file mode 100644 index 000000000..19fbe03ad --- /dev/null +++ b/test_regress/t/t_opt_table_display.v @@ -0,0 +1,44 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2022 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +module t (/*AUTOARG*/ + // Outputs + test, + // Inputs + clk + ); + input clk; + + output reg [5:0] test; + parameter STATE1 = 6'b000001; + parameter STATE2 = 6'b000010; + parameter STATE3 = 6'b000100; + parameter STATE4 = 6'b001000; + parameter STATE5 = 6'b010000; + parameter STATE6 = 6'b100000; + + always @(posedge clk) begin + $display("Clocked"); + case (test) + STATE1: test <= STATE2; + STATE2: test <= STATE3; + STATE3: test <= STATE4; + STATE4: test <= STATE5; + STATE5: test <= STATE6; + default: test <= STATE1; + endcase + end + + int cyc; + always @(posedge clk) begin + cyc <= cyc + 1; + if (cyc == 10) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end + +endmodule From 5f3316d3dc68099adf1c512ae0658f99699cbab9 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 9 Jul 2022 08:30:57 -0400 Subject: [PATCH 07/12] * Fix empty string arguments to display (#3484). --- Changes | 1 + src/V3LinkResolve.cpp | 58 +++++++++++++++--------------- test_regress/t/t_display_merge.out | 2 ++ test_regress/t/t_display_merge.pl | 2 +- test_regress/t/t_display_merge.v | 4 +++ 5 files changed, 38 insertions(+), 29 deletions(-) diff --git a/Changes b/Changes index 5f68d4a40..910eceb2f 100644 --- a/Changes +++ b/Changes @@ -14,6 +14,7 @@ Verilator 4.225 devel **Minor:** * Fix incorrect bit op tree optimization (#3470). [algrobman] +* Fix empty string arguments to display (#3484). [Grulfen] * Fix table misoptimizing away display (#3488). [Stefan Post] diff --git a/src/V3LinkResolve.cpp b/src/V3LinkResolve.cpp index a0d198bc7..45ce55922 100644 --- a/src/V3LinkResolve.cpp +++ b/src/V3LinkResolve.cpp @@ -352,43 +352,45 @@ private: while (argp) { if (skipCount) { argp = argp->nextp(); - skipCount--; + --skipCount; continue; } const AstConst* const constp = VN_CAST(argp, Const); const bool isFromString = (constp) ? constp->num().isFromString() : false; if (isFromString) { const int numchars = argp->dtypep()->width() / 8; - string str(numchars, ' '); - // now scan for % operators - bool inpercent = false; - for (int i = 0; i < numchars; i++) { - const int ii = numchars - i - 1; - const char c = constp->num().dataByte(ii); - str[i] = c; - if (!inpercent && c == '%') { - inpercent = true; - } else if (inpercent) { - inpercent = false; - switch (c) { - case '0': // FALLTHRU - case '1': // FALLTHRU - case '2': // FALLTHRU - case '3': // FALLTHRU - case '4': // FALLTHRU - case '5': // FALLTHRU - case '6': // FALLTHRU - case '7': // FALLTHRU - case '8': // FALLTHRU - case '9': // FALLTHRU - case '.': inpercent = true; break; - case '%': break; - default: - if (V3Number::displayedFmtLegal(c, isScan)) ++skipCount; + if (!constp->num().toString().empty()) { + string str(numchars, ' '); + // now scan for % operators + bool inpercent = false; + for (int i = 0; i < numchars; i++) { + const int ii = numchars - i - 1; + const char c = constp->num().dataByte(ii); + str[i] = c; + if (!inpercent && c == '%') { + inpercent = true; + } else if (inpercent) { + inpercent = false; + switch (c) { + case '0': // FALLTHRU + case '1': // FALLTHRU + case '2': // FALLTHRU + case '3': // FALLTHRU + case '4': // FALLTHRU + case '5': // FALLTHRU + case '6': // FALLTHRU + case '7': // FALLTHRU + case '8': // FALLTHRU + case '9': // FALLTHRU + case '.': inpercent = true; break; + case '%': break; + default: + if (V3Number::displayedFmtLegal(c, isScan)) ++skipCount; + } } } + newFormat.append(str); } - newFormat.append(str); AstNode* const nextp = argp->nextp(); argp->unlinkFrBack(); VL_DO_DANGLING(pushDeletep(argp), argp); diff --git a/test_regress/t/t_display_merge.out b/test_regress/t/t_display_merge.out index 358455d76..0dcb8b10e 100644 --- a/test_regress/t/t_display_merge.out +++ b/test_regress/t/t_display_merge.out @@ -1,5 +1,7 @@ Merge: This should merge +Merge: +This should also merge f 1=1 a=top.t 1=1 1=1 b=top.t 1=1 pre diff --git a/test_regress/t/t_display_merge.pl b/test_regress/t/t_display_merge.pl index 75570aba2..27cc0ef25 100755 --- a/test_regress/t/t_display_merge.pl +++ b/test_regress/t/t_display_merge.pl @@ -20,7 +20,7 @@ execute( ); file_grep("$Self->{obj_dir}/$Self->{VM_PREFIX}__stats.txt", - qr/Node count, DISPLAY \s+ 41 \s+ 27 \s+ 27 \s+ 6/); + qr/Node count, DISPLAY \s+ 44 \s+ 27 \s+ 27 \s+ 6/); ok(1); 1; diff --git a/test_regress/t/t_display_merge.v b/test_regress/t/t_display_merge.v index 0704e7728..ede4c15eb 100644 --- a/test_regress/t/t_display_merge.v +++ b/test_regress/t/t_display_merge.v @@ -19,6 +19,10 @@ module t (/*AUTOARG*/); $write("should "); $display("merge"); + $display("Merge:"); + $write("This ", "", "should ", "", "also "); + $display("merge"); + $display("f"); $write(" 1=%0d a=%m 1=%0d", one, one); $display(" 1=%0d b=%m 1=%0d", one, one); From d8ea989edab83ba948db717b98c394c14c2d8deb Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 9 Jul 2022 09:50:50 -0400 Subject: [PATCH 08/12] Tests/examples: Remove some legacy Verilator:: calls. --- docs/guide/connecting.rst | 69 ++++--------------- docs/guide/exe_sim.rst | 4 +- docs/guide/languages.rst | 2 +- examples/make_hello_c/sim_main.cpp | 7 +- test_regress/t/t_dpi_accessors.cpp | 79 +++++++++++----------- test_regress/t/t_dpi_var.cpp | 26 +++---- test_regress/t/t_hier_block_cmake/main.cpp | 9 +-- test_regress/t/t_leak.cpp | 3 +- test_regress/t/t_trace_two_cc.cpp | 28 ++++---- test_regress/t/t_tri_gate.cpp | 2 - test_regress/t/t_var_pinsizes.cpp | 2 - test_regress/t/t_vpi_cb_iter.cpp | 17 +++-- test_regress/t/t_vpi_cbs_called.cpp | 35 +++++----- test_regress/t/t_vpi_get.cpp | 30 ++++---- test_regress/t/t_vpi_memory.cpp | 33 ++++----- test_regress/t/t_vpi_module.cpp | 44 ++++++------ test_regress/t/t_vpi_param.cpp | 34 +++++----- test_regress/t/t_vpi_time_cb.cpp | 35 +++++----- test_regress/t/t_vpi_unimpl.cpp | 33 ++++----- test_regress/t/t_vpi_var.cpp | 21 +++--- test_regress/t/t_vpi_zero_time_cb.cpp | 30 ++++---- 21 files changed, 258 insertions(+), 285 deletions(-) diff --git a/docs/guide/connecting.rst b/docs/guide/connecting.rst index edfc3d67b..443702c7d 100644 --- a/docs/guide/connecting.rst +++ b/docs/guide/connecting.rst @@ -86,61 +86,17 @@ Connecting to C++ In C++ output mode (:vlopt:`--cc`), the Verilator generated model class is a simple C++ class. The user must write a C++ wrapper and main loop for the simulation, which instantiates the model class, and link with the Verilated -model. Here is a simple example: +model. -.. code-block:: C++ +Refer to ``examples/make_tracing_c`` in the distribution for a detailed +commented example. - #include // Defines common routines - #include // Need std::cout - #include "Vtop.h" // From Verilating "top.v" +Top level IO signals are read and written as members of the model. You +call the model's :code:`eval()` method to evaluate the model. When the +simulation is complete call the model's :code:`final()` method to execute +any SystemVerilog final blocks, and complete any assertions. See +:ref:`Evaluation Loop`. - Vtop *top; // Instantiation of model - - uint64_t main_time = 0; // Current simulation time - // This is a 64-bit integer to reduce wrap over issues and - // allow modulus. This is in units of the timeprecision - // used in Verilog (or from --timescale-override) - - double sc_time_stamp() { // Called by $time in Verilog - return main_time; // converts to double, to match - // what SystemC does - } - - int main(int argc, char** argv) { - Verilated::commandArgs(argc, argv); // Remember args - - top = new Vtop; // Create model - // Do not instead make Vtop as a file-scope static - // variable, as the "C++ static initialization order fiasco" - // may cause a crash - - top->reset_l = 0; // Set some inputs - - while (!Verilated::gotFinish()) { - if (main_time > 10) { - top->reset_l = 1; // Deassert reset - } - if ((main_time % 10) == 1) { - top->clk = 1; // Toggle clock - } - if ((main_time % 10) == 6) { - top->clk = 0; - } - top->eval(); // Evaluate model - cout << top->out << endl; // Read a output - main_time++; // Time passes... - } - - top->final(); // Done simulating - // // (Though this example doesn't get here) - delete top; - } - - -Note top level IO signals are read and written as members of the model. You -call the :code:`eval()` method to evaluate the model. When the simulation is -complete call the :code:`final()` method to execute any SystemVerilog final -blocks, and complete any assertions. See :ref:`Evaluation Loop`. Connecting to SystemC @@ -449,14 +405,15 @@ accesses the above signal "readme" would be: int main(int argc, char** argv, char** env) { Verilated::commandArgs(argc, argv); - Vour* top = new Vour; - Verilated::internalsDump(); // See scopes to help debug - while (!Verilated::gotFinish()) { + const std::unique_ptr contextp{new VerilatedContext}; + const std::unique_ptr top{new Vour{contextp.get()}}; + + contextp->internalsDump(); // See scopes to help debug + while (!contextp->gotFinish()) { top->eval(); VerilatedVpi::callValueCbs(); // For signal callbacks read_and_check(); } - delete top; return 0; } EOF diff --git a/docs/guide/exe_sim.rst b/docs/guide/exe_sim.rst index 016340cc8..364ac5fba 100644 --- a/docs/guide/exe_sim.rst +++ b/docs/guide/exe_sim.rst @@ -8,7 +8,7 @@ Simulation Runtime Arguments The following are the arguments that may be passed to a Verilated executable, provided that executable calls -:code:`Verilated::commandArgs()`. +:code:`VerilatedContext*->commandArgs(argc, argv)`. All simulation runtime arguments begin with "+verilator", so that the user's executable may skip over all "+verilator" arguments when parsing its @@ -96,7 +96,7 @@ Summary: .. option:: +verilator+noassert Disable assert checking per runtime argument. This is the same as - calling :code:`Verilated::assertOn(false)` in the model. + calling :code:`VerilatedContext*->assertOn(false)` in the model. .. option:: +verilator+V diff --git a/docs/guide/languages.rst b/docs/guide/languages.rst index 074f91d31..9b8adc596 100644 --- a/docs/guide/languages.rst +++ b/docs/guide/languages.rst @@ -489,7 +489,7 @@ $test$plusargs, $value$plusargs .. code-block:: C++ - Verilated::commandArgs(argc, argv); + {VerilatedContext*} ->commandArgs(argc, argv); to register the command line before calling $test$plusargs or $value$plusargs. diff --git a/examples/make_hello_c/sim_main.cpp b/examples/make_hello_c/sim_main.cpp index b2c76c84a..2e605d512 100644 --- a/examples/make_hello_c/sim_main.cpp +++ b/examples/make_hello_c/sim_main.cpp @@ -21,11 +21,14 @@ int main(int argc, char** argv, char** env) { // Prevent unused variable warnings if (false && argc && argv && env) {} + // Construct a VerilatedContext to hold simulation time, etc. + VerilatedContext* contextp = new VerilatedContext; + // Construct the Verilated model, from Vtop.h generated from Verilating "top.v" - Vtop* top = new Vtop; + Vtop* top = new Vtop{contextp}; // Simulate until $finish - while (!Verilated::gotFinish()) { + while (!contextp->gotFinish()) { // Evaluate model top->eval(); diff --git a/test_regress/t/t_dpi_accessors.cpp b/test_regress/t/t_dpi_accessors.cpp index d0de0d9a2..f160a888a 100644 --- a/test_regress/t/t_dpi_accessors.cpp +++ b/test_regress/t/t_dpi_accessors.cpp @@ -26,11 +26,9 @@ using std::hex; using std::setfill; using std::setw; -double sc_time_stamp() { return 0; } - // Convenience function to check we didn't finish unexpectedly -static void checkFinish(const char* msg) { - if (Verilated::gotFinish()) { +static void checkFinish(VerilatedContext* contextp, const char* msg) { + if (contextp->gotFinish()) { vl_fatal(__FILE__, __LINE__, "dut", msg); exit(1); } @@ -61,7 +59,9 @@ static void checkResult(bool p, const char* msg_fail) { // Main function instantiates the model and steps through the test. int main() { - Vt_dpi_accessors* dut = new Vt_dpi_accessors("dut"); + const std::unique_ptr contextp{new VerilatedContext}; + const std::unique_ptr dut{new VM_PREFIX{contextp.get(), "dut"}}; + svScope scope = svGetScopeFromName("dut.t"); if (!scope) vl_fatal(__FILE__, __LINE__, "dut", "No svGetScopeFromName result"); svSetScope(scope); @@ -112,7 +112,7 @@ int main() { cout << "===============================\n"; #endif - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; a = (int)a_read(); logReg(dut->clk, "read a", a, " (before clk)"); @@ -130,7 +130,7 @@ int main() { "Test of scalar register reading failed."); } - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); // Check we can read a vector register. #ifdef TEST_VERBOSE @@ -138,7 +138,7 @@ int main() { cout << "===============================\n"; #endif - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; b = (int)b_read(); logRegHex(dut->clk, "read b", 8, b, " (before clk)"); @@ -153,7 +153,7 @@ int main() { "Test of vector register reading failed."); } - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); // Test we can read an array element #ifdef TEST_VERBOSE @@ -162,7 +162,7 @@ int main() { cout << "=============================\n"; #endif - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; mem32 = (int)mem32_read(); logRegHex(dut->clk, "read mem32", 8, mem32, " (before clk)"); @@ -177,7 +177,7 @@ int main() { checkResult(mem32 == 0x20, "Test of array element reading failed."); } - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); // Check we can read a scalar wire #ifdef TEST_VERBOSE @@ -186,7 +186,7 @@ int main() { cout << "===========================\n"; #endif - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; a = (int)a_read(); c = (int)c_read(); @@ -206,7 +206,7 @@ int main() { checkResult(c == (1 - a), "Test of scalar wire reading failed."); } - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); // Check we can read a vector wire #ifdef TEST_VERBOSE @@ -215,7 +215,7 @@ int main() { cout << "===========================\n"; #endif - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; b = (int)b_read(); d = (int)d_read(); @@ -236,7 +236,7 @@ int main() { checkResult(d == ((~b) & 0xff), "Test of vector wire reading failed."); } - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); // Check we can write a scalar register #ifdef TEST_VERBOSE @@ -245,7 +245,7 @@ int main() { cout << "===============================\n"; #endif - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; a = 1 - (int)a_read(); a_write(reinterpret_cast(&a)); @@ -265,7 +265,7 @@ int main() { "Test of scalar register writing failed."); } - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); // Check we can write a vector register #ifdef TEST_VERBOSE @@ -274,7 +274,7 @@ int main() { cout << "===============================\n"; #endif - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; b = (int)b_read() - 1; b_write(reinterpret_cast(&b)); @@ -294,7 +294,7 @@ int main() { "Test of vector register writing failed."); } - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); // Test we can write an array element #ifdef TEST_VERBOSE @@ -303,7 +303,7 @@ int main() { cout << "=============================\n"; #endif - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; mem32 = (int)mem32_read() - 1; mem32_write(reinterpret_cast(&mem32)); @@ -323,7 +323,7 @@ int main() { checkResult(mem32_after == mem32, "Test of array element writing failed."); } - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); // Check we can read a vector register slice #ifdef TEST_VERBOSE @@ -332,7 +332,7 @@ int main() { cout << "=====================================\n"; #endif - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; b = (int)b_read(); int b_slice = (int)b_slice_read(); @@ -350,7 +350,7 @@ int main() { checkResult(b_slice == (b & 0x0f), "Test of vector register slice reading failed."); } - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); // Test we can read an array element slice #ifdef TEST_VERBOSE @@ -359,7 +359,7 @@ int main() { cout << "===================================\n"; #endif - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; mem32 = (int)mem32_read(); int mem32_slice = (int)mem32_slice_read(); @@ -379,7 +379,7 @@ int main() { "Test of array element slice reading failed."); } - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); // Check we can read a vector wire slice #ifdef TEST_VERBOSE @@ -388,7 +388,7 @@ int main() { cout << "=================================\n"; #endif - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; b = (int)b_read(); d = (int)d_read(); @@ -410,7 +410,7 @@ int main() { checkResult(d_slice == ((d & 0x7e) >> 1), "Test of vector wire slice reading failed."); } - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); // Check we can write a vector register slice #ifdef TEST_VERBOSE @@ -419,7 +419,7 @@ int main() { cout << "=====================================\n"; #endif - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; b = (int)b_read(); @@ -449,7 +449,7 @@ int main() { logRegHex(dut->clk, "read b [3:0]", 4, b_slice, " (after clk)"); } - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); // Test we can write an array element slice #ifdef TEST_VERBOSE @@ -458,7 +458,7 @@ int main() { cout << "===================================\n"; #endif - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; mem32 = (int)mem32_read(); @@ -494,7 +494,7 @@ int main() { "Test of array element slice writing failed."); } - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); // Check we can read complex registers #ifdef TEST_VERBOSE @@ -503,7 +503,7 @@ int main() { cout << "================================\n"; #endif - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; b = (int)b_read(); @@ -540,9 +540,9 @@ int main() { cout << endl; #endif - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; e = 0x05 | (i << 4); @@ -574,7 +574,7 @@ int main() { "Test of complex register reading l2 failed."); } - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); // Test we can write a complex register #ifdef TEST_VERBOSE @@ -583,7 +583,7 @@ int main() { cout << "================================\n"; #endif - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; b = (int)b_read(); @@ -632,9 +632,9 @@ int main() { cout << endl; #endif - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); - for (int i = 0; !Verilated::gotFinish() && (i < 4); i++) { + for (int i = 0; !contextp->gotFinish() && (i < 4); i++) { dut->clk = 1 - dut->clk; e = (int)e_read(); @@ -671,11 +671,10 @@ int main() { logRegHex(dut->clk, "read l2", 8, l2, " (before clk)"); } - checkFinish("t_dpi_accessors unexpected finish"); + checkFinish(contextp.get(), "t_dpi_accessors unexpected finish"); // Tidy up dut->final(); - VL_DO_DANGLING(delete dut, dut); cout << "*-* All Finished *-*\n"; } diff --git a/test_regress/t/t_dpi_var.cpp b/test_regress/t/t_dpi_var.cpp index 0bd8aa1ac..190044f69 100644 --- a/test_regress/t/t_dpi_var.cpp +++ b/test_regress/t/t_dpi_var.cpp @@ -110,39 +110,39 @@ void mon_eval() { //====================================================================== -unsigned int main_time = 0; - -double sc_time_stamp() { return main_time; } int main(int argc, char** argv, char** env) { - uint64_t sim_time = 1100; - Verilated::commandArgs(argc, argv); - Verilated::debug(0); + const std::unique_ptr contextp{new VerilatedContext}; - VM_PREFIX* topp = new VM_PREFIX(""); // Note null name - we're flattening it out + uint64_t sim_time = 1100; + contextp->commandArgs(argc, argv); + contextp->debug(0); + + const std::unique_ptr topp{new VM_PREFIX{contextp.get(), + // Note null name - we're flattening it out + ""}}; // clang-format off #ifdef VERILATOR # ifdef TEST_VERBOSE - Verilated::scopesDump(); + contextp->scopesDump(); # endif #endif // clang-format on topp->eval(); topp->clk = 0; - main_time += 10; + contextp->timeInc(10); - while (vl_time_stamp64() < sim_time && !Verilated::gotFinish()) { - main_time += 1; + while (contextp->time() < sim_time && !contextp->gotFinish()) { + contextp->timeInc(1); topp->eval(); topp->clk = !topp->clk; // mon_do(); } - if (!Verilated::gotFinish()) { + if (!contextp->gotFinish()) { vl_fatal(__FILE__, __LINE__, "main", "%Error: Timeout; never got a $finish"); } topp->final(); - VL_DO_DANGLING(delete topp, topp); return 0; } diff --git a/test_regress/t/t_hier_block_cmake/main.cpp b/test_regress/t/t_hier_block_cmake/main.cpp index b26551f4d..e49101162 100644 --- a/test_regress/t/t_hier_block_cmake/main.cpp +++ b/test_regress/t/t_hier_block_cmake/main.cpp @@ -13,13 +13,14 @@ #include "Vt_hier_block.h" int main(int argc, char *argv[]) { - std::unique_ptr top{new Vt_hier_block("top")}; - Verilated::commandArgs(argc, argv); - for (int i = 0; i < 100 && !Verilated::gotFinish(); ++i) { + const std::unique_ptr contextp{new VerilatedContext}; + std::unique_ptr top{new Vt_hier_block{contextp.get(), "top"}}; + contextp->commandArgs(argc, argv); + for (int i = 0; i < 100 && !contextp->gotFinish(); ++i) { top->eval(); top->clk ^= 1; } - if (!Verilated::gotFinish()) { + if (!contextp->gotFinish()) { vl_fatal(__FILE__, __LINE__, "main", "%Error: Timeout; never got a $finish"); } top->final(); diff --git a/test_regress/t/t_leak.cpp b/test_regress/t/t_leak.cpp index a0ad9259b..247dce41a 100644 --- a/test_regress/t/t_leak.cpp +++ b/test_regress/t/t_leak.cpp @@ -48,11 +48,12 @@ void make_and_destroy() { #ifdef VL_NO_LEGACY VerilatedContext* contextp = new VerilatedContext; VM_PREFIX* topp = new VM_PREFIX{contextp}; + contextp->debug(0); #else VM_PREFIX* topp = new VM_PREFIX; + Verilated::debug(0); #endif - Verilated::debug(0); topp->eval(); topp->clk = true; while (! diff --git a/test_regress/t/t_trace_two_cc.cpp b/test_regress/t/t_trace_two_cc.cpp index 0074d4ecb..31c314f86 100644 --- a/test_regress/t/t_trace_two_cc.cpp +++ b/test_regress/t/t_trace_two_cc.cpp @@ -25,21 +25,21 @@ VM_PREFIX* ap; Vt_trace_two_b* bp; -uint64_t main_time = 0; -double sc_time_stamp() { return main_time; } int main(int argc, char** argv, char** env) { + const std::unique_ptr contextp{new VerilatedContext}; + uint64_t sim_time = 1100; - Verilated::commandArgs(argc, argv); - Verilated::debug(0); - Verilated::traceEverOn(true); + contextp->commandArgs(argc, argv); + contextp->debug(0); + contextp->traceEverOn(true); srand48(5); - ap = new VM_PREFIX("topa"); - bp = new Vt_trace_two_b("topb"); + ap = new VM_PREFIX{contextp.get(), "topa"}; + bp = new Vt_trace_two_b{contextp.get(), "topb"}; // clang-format off #ifdef TEST_HDR_TRACE - Verilated::traceEverOn(true); + contextp->traceEverOn(true); # ifdef TEST_FST VerilatedFstC* tfp = new VerilatedFstC; ap->trace(tfp, 99); @@ -59,14 +59,14 @@ int main(int argc, char** argv, char** env) { bp->eval_step(); ap->eval_end_step(); bp->eval_end_step(); - if (tfp) tfp->dump(main_time); + if (tfp) tfp->dump(contextp->time()); #endif { ap->clk = false; - main_time += 10; + contextp->timeInc(10); } - while (vl_time_stamp64() < sim_time && !Verilated::gotFinish()) { + while (contextp->time() < sim_time && !contextp->gotFinish()) { ap->clk = !ap->clk; bp->clk = ap->clk; ap->eval_step(); @@ -74,11 +74,11 @@ int main(int argc, char** argv, char** env) { ap->eval_end_step(); bp->eval_end_step(); #ifdef TEST_HDR_TRACE - if (tfp) tfp->dump(main_time); + if (tfp) tfp->dump(contextp->time()); #endif - main_time += 5; + contextp->timeInc(5); } - if (!Verilated::gotFinish()) { + if (!contextp->gotFinish()) { vl_fatal(__FILE__, __LINE__, "main", "%Error: Timeout; never got a $finish"); } ap->final(); diff --git a/test_regress/t/t_tri_gate.cpp b/test_regress/t/t_tri_gate.cpp index f16ae0a45..66159175b 100644 --- a/test_regress/t/t_tri_gate.cpp +++ b/test_regress/t/t_tri_gate.cpp @@ -8,8 +8,6 @@ VM_PREFIX* tb = nullptr; -double sc_time_stamp() { return 0; } - bool check() { bool pass; int c = (tb->A >> tb->SEL) & 0x1; diff --git a/test_regress/t/t_var_pinsizes.cpp b/test_regress/t/t_var_pinsizes.cpp index a5c75e363..5dc53e9e2 100644 --- a/test_regress/t/t_var_pinsizes.cpp +++ b/test_regress/t/t_var_pinsizes.cpp @@ -7,8 +7,6 @@ VM_PREFIX* tb = nullptr; -double sc_time_stamp() { return 0; } - int main() { Verilated::debug(0); tb = new VM_PREFIX("tb"); diff --git a/test_regress/t/t_vpi_cb_iter.cpp b/test_regress/t/t_vpi_cb_iter.cpp index 3afbe07e9..8258385e3 100644 --- a/test_regress/t/t_vpi_cb_iter.cpp +++ b/test_regress/t/t_vpi_cb_iter.cpp @@ -143,11 +143,15 @@ static void register_filler_cb() { double sc_time_stamp() { return main_time; } int main(int argc, char** argv, char** env) { - uint64_t sim_time = 100; - Verilated::commandArgs(argc, argv); - Verilated::debug(0); + const std::unique_ptr contextp{new VerilatedContext}; - VM_PREFIX* topp = new VM_PREFIX(""); // Note null name - we're flattening it out + uint64_t sim_time = 100; + contextp->commandArgs(argc, argv); + contextp->debug(0); + + const std::unique_ptr topp{new VM_PREFIX{contextp.get(), + // Note null name - we're flattening it out + ""}}; reregister_value_cb(); TEST_CHECK_NZ(vh_value_cb); @@ -158,7 +162,7 @@ int main(int argc, char** argv, char** env) { topp->eval(); topp->clk = 0; - while (vl_time_stamp64() < sim_time && !Verilated::gotFinish()) { + while (main_time < sim_time && !contextp->gotFinish()) { main_time += 1; if (verbose) VL_PRINTF("Sim Time %d got_error %d\n", main_time, errors); topp->clk = !topp->clk; @@ -168,11 +172,10 @@ int main(int argc, char** argv, char** env) { if (errors) vl_stop(__FILE__, __LINE__, "TOP-cpp"); } - if (!Verilated::gotFinish()) { + if (!contextp->gotFinish()) { vl_fatal(__FILE__, __LINE__, "main", "%Error: Timeout; never got a $finish"); } topp->final(); - VL_DO_DANGLING(delete topp, topp); return errors ? 10 : 0; } diff --git a/test_regress/t/t_vpi_cbs_called.cpp b/test_regress/t/t_vpi_cbs_called.cpp index c015b38af..c3e39db12 100644 --- a/test_regress/t/t_vpi_cbs_called.cpp +++ b/test_regress/t/t_vpi_cbs_called.cpp @@ -43,7 +43,6 @@ bool callbacks_expected_called[CB_COUNT] = {false}; std::vector::const_iterator cb_iter; std::vector::const_iterator state_iter; -unsigned int main_time = 0; bool got_error = false; #ifdef TEST_VERBOSE @@ -245,27 +244,29 @@ static int register_test_callback() { return 0; } -double sc_time_stamp() { return main_time; } - int main(int argc, char** argv, char** env) { + const std::unique_ptr contextp{new VerilatedContext}; + uint64_t sim_time = 100; bool cbs_called; - Verilated::commandArgs(argc, argv); + contextp->commandArgs(argc, argv); - VM_PREFIX* topp = new VM_PREFIX(""); // Note null name - we're flattening it out + const std::unique_ptr topp{new VM_PREFIX{contextp.get(), + // Note null name - we're flattening it out + ""}}; - if (verbose) VL_PRINTF("-- { Sim Time %d } --\n", main_time); + if (verbose) VL_PRINTF("-- { Sim Time %" PRId64 " } --\n", contextp->time()); register_test_callback(); topp->eval(); topp->clk = 0; - main_time += 1; + contextp->timeInc(1); - while (vl_time_stamp64() < sim_time && !Verilated::gotFinish()) { + while (contextp->time() < sim_time && !contextp->gotFinish()) { if (verbose) { - VL_PRINTF("-- { Sim Time %d , Callback %s (%d) , Testcase State %d } --\n", main_time, - cb_reason_to_string(*cb_iter), *cb_iter, *state_iter); + VL_PRINTF("-- { Sim Time %" PRId64 " , Callback %s (%d) , Testcase State %d } --\n", + contextp->time(), cb_reason_to_string(*cb_iter), *cb_iter, *state_iter); } topp->eval(); @@ -285,14 +286,17 @@ int main(int argc, char** argv, char** env) { VerilatedVpi::callTimedCbs(); - main_time = VerilatedVpi::cbNextDeadline(); - if (main_time == -1 && !Verilated::gotFinish()) { - if (verbose) VL_PRINTF("-- { Sim Time %d , No more testcases } --\n", main_time); + int64_t next_time = VerilatedVpi::cbNextDeadline(); + contextp->time(next_time); + if (next_time == -1 && !contextp->gotFinish()) { + if (verbose) + VL_PRINTF("-- { Sim Time %" PRId64 " , No more testcases } --\n", + contextp->time()); if (got_error) { vl_stop(__FILE__, __LINE__, "TOP-cpp"); } else { VL_PRINTF("*-* All Finished *-*\n"); - Verilated::gotFinish(true); + contextp->gotFinish(true); } } @@ -302,11 +306,10 @@ int main(int argc, char** argv, char** env) { topp->clk = !topp->clk; } - if (!Verilated::gotFinish()) { + if (!contextp->gotFinish()) { vl_fatal(__FILE__, __LINE__, "main", "%Error: Timeout; never got a $finish"); } topp->final(); - VL_DO_DANGLING(delete topp, topp); return 0; } diff --git a/test_regress/t/t_vpi_get.cpp b/test_regress/t/t_vpi_get.cpp index ab8a9bce5..20d5d0fd7 100644 --- a/test_regress/t/t_vpi_get.cpp +++ b/test_regress/t/t_vpi_get.cpp @@ -40,8 +40,6 @@ #define TEST_MSG \ if (0) printf -unsigned int main_time = 0; - //====================================================================== #define CHECK_RESULT_VH(got, exp) \ @@ -240,22 +238,25 @@ void vpi_compat_bootstrap(void) { void (*vlog_startup_routines[])() = {vpi_compat_bootstrap, 0}; #else -double sc_time_stamp() { return main_time; } int main(int argc, char** argv, char** env) { - uint64_t sim_time = 1100; - Verilated::commandArgs(argc, argv); - Verilated::debug(0); + const std::unique_ptr contextp{new VerilatedContext}; - Vt_vpi_get* topp = new Vt_vpi_get(""); // Note null name - we're flattening it out + uint64_t sim_time = 1100; + contextp->commandArgs(argc, argv); + contextp->debug(0); + + const std::unique_ptr topp{new VM_PREFIX{contextp.get(), + // Note null name - we're flattening it out + ""}}; #ifdef VERILATOR #ifdef TEST_VERBOSE - Verilated::scopesDump(); + contextp->scopesDump(); #endif #endif #if VM_TRACE - Verilated::traceEverOn(true); + contextp->traceEverOn(true); VL_PRINTF("Enabling waves...\n"); VerilatedVcdC* tfp = new VerilatedVcdC; topp->trace(tfp, 99); @@ -264,19 +265,19 @@ int main(int argc, char** argv, char** env) { topp->eval(); topp->clk = 0; - main_time += 10; + contextp->timeInc(10); - while (vl_time_stamp64() < sim_time && !Verilated::gotFinish()) { - main_time += 1; + while (contextp->time() < sim_time && !contextp->gotFinish()) { + contextp->timeInc(1); topp->eval(); VerilatedVpi::callValueCbs(); topp->clk = !topp->clk; // mon_do(); #if VM_TRACE - if (tfp) tfp->dump(main_time); + if (tfp) tfp->dump(contextp->time()); #endif } - if (!Verilated::gotFinish()) { + if (!contextp->gotFinish()) { vl_fatal(FILENM, __LINE__, "main", "%Error: Timeout; never got a $finish"); } topp->final(); @@ -285,7 +286,6 @@ int main(int argc, char** argv, char** env) { if (tfp) tfp->close(); #endif - VL_DO_DANGLING(delete topp, topp); return 0; } diff --git a/test_regress/t/t_vpi_memory.cpp b/test_regress/t/t_vpi_memory.cpp index 490d871e0..e7532b9ad 100644 --- a/test_regress/t/t_vpi_memory.cpp +++ b/test_regress/t/t_vpi_memory.cpp @@ -41,7 +41,6 @@ #define DEBUG \ if (0) printf -unsigned int main_time = 0; int errors = 0; //====================================================================== @@ -250,24 +249,27 @@ void (*vlog_startup_routines[])() = {vpi_compat_bootstrap, 0}; #else -double sc_time_stamp() { return main_time; } int main(int argc, char** argv, char** env) { - uint64_t sim_time = 1100; - Verilated::commandArgs(argc, argv); - Verilated::debug(0); - // we're going to be checking for these errors do don't crash out - Verilated::fatalOnVpiError(0); + const std::unique_ptr contextp{new VerilatedContext}; - VM_PREFIX* topp = new VM_PREFIX(""); // Note null name - we're flattening it out + uint64_t sim_time = 1100; + contextp->commandArgs(argc, argv); + contextp->debug(0); + // we're going to be checking for these errors do don't crash out + contextp->fatalOnVpiError(0); + + const std::unique_ptr topp{new VM_PREFIX{contextp.get(), + // Note null name - we're flattening it out + ""}}; #ifdef VERILATOR #ifdef TEST_VERBOSE - Verilated::scopesDump(); + contextp->scopesDump(); #endif #endif #if VM_TRACE - Verilated::traceEverOn(true); + contextp->traceEverOn(true); VL_PRINTF("Enabling waves...\n"); VerilatedVcdC* tfp = new VerilatedVcdC; topp->trace(tfp, 99); @@ -276,19 +278,19 @@ int main(int argc, char** argv, char** env) { topp->eval(); topp->clk = 0; - main_time += 10; + contextp->timeInc(10); - while (vl_time_stamp64() < sim_time && !Verilated::gotFinish()) { - main_time += 1; + while (contextp->time() < sim_time && !contextp->gotFinish()) { + contextp->timeInc(1); topp->eval(); VerilatedVpi::callValueCbs(); topp->clk = !topp->clk; // mon_do(); #if VM_TRACE - if (tfp) tfp->dump(main_time); + if (tfp) tfp->dump(contextp->time()); #endif } - if (!Verilated::gotFinish()) { + if (!contextp->gotFinish()) { vl_fatal(FILENM, __LINE__, "main", "%Error: Timeout; never got a $finish"); } topp->final(); @@ -297,7 +299,6 @@ int main(int argc, char** argv, char** env) { if (tfp) tfp->close(); #endif - VL_DO_DANGLING(delete topp, topp); return 0; } diff --git a/test_regress/t/t_vpi_module.cpp b/test_regress/t/t_vpi_module.cpp index cc3325eca..b09fe6ea7 100644 --- a/test_regress/t/t_vpi_module.cpp +++ b/test_regress/t/t_vpi_module.cpp @@ -40,8 +40,6 @@ #define DEBUG \ if (0) printf -unsigned int main_time = 0; - #define CHECK_RESULT_NZ(got) \ if (!(got)) { \ printf("%%Error: %s:%d: GOT = NULL EXP = !NULL\n", FILENM, __LINE__); \ @@ -172,27 +170,36 @@ void (*vlog_startup_routines[])() = {vpi_compat_bootstrap, 0}; #else -double sc_time_stamp() { return main_time; } int main(int argc, char** argv, char** env) { - uint64_t sim_time = 1100; - Verilated::commandArgs(argc, argv); - Verilated::debug(0); - // we're going to be checking for these errors do don't crash out - Verilated::fatalOnVpiError(0); + const std::unique_ptr contextp{new VerilatedContext}; + + uint64_t sim_time = 1100; + contextp->commandArgs(argc, argv); + contextp->debug(0); + // we're going to be checking for these errors do don't crash out + contextp->fatalOnVpiError(0); + + { + // Construct and destroy + const std::unique_ptr topp{ + new VM_PREFIX{contextp.get(), + // Note null name - we're flattening it out + ""}}; + } - VM_PREFIX* topp = new VM_PREFIX(""); // Note null name - we're flattening it out // Test second construction - delete topp; - topp = new VM_PREFIX(""); + const std::unique_ptr topp{new VM_PREFIX{contextp.get(), + // Note null name - we're flattening it out + ""}}; #ifdef VERILATOR #ifdef TEST_VERBOSE - Verilated::scopesDump(); + contextp->scopesDump(); #endif #endif #if VM_TRACE - Verilated::traceEverOn(true); + contextp->traceEverOn(true); VL_PRINTF("Enabling waves...\n"); VerilatedVcdC* tfp = new VerilatedVcdC; topp->trace(tfp, 99); @@ -201,19 +208,19 @@ int main(int argc, char** argv, char** env) { topp->eval(); topp->clk = 0; - main_time += 10; + contextp->timeInc(10); - while (vl_time_stamp64() < sim_time && !Verilated::gotFinish()) { - main_time += 1; + while (contextp->time() < sim_time && !contextp->gotFinish()) { + contextp->timeInc(1); topp->eval(); VerilatedVpi::callValueCbs(); topp->clk = !topp->clk; // mon_do(); #if VM_TRACE - if (tfp) tfp->dump(main_time); + if (tfp) tfp->dump(contextp->time()); #endif } - if (!Verilated::gotFinish()) { + if (!contextp->gotFinish()) { vl_fatal(FILENM, __LINE__, "main", "%Error: Timeout; never got a $finish"); } topp->final(); @@ -222,7 +229,6 @@ int main(int argc, char** argv, char** env) { if (tfp) tfp->close(); #endif - VL_DO_DANGLING(delete topp, topp); return 0; } diff --git a/test_regress/t/t_vpi_param.cpp b/test_regress/t/t_vpi_param.cpp index 639bb9196..8ac956531 100644 --- a/test_regress/t/t_vpi_param.cpp +++ b/test_regress/t/t_vpi_param.cpp @@ -40,8 +40,6 @@ #define DEBUG \ if (0) printf -unsigned int main_time = 0; - //====================================================================== #define CHECK_RESULT_VH(got, exp) \ @@ -240,24 +238,27 @@ void (*vlog_startup_routines[])() = {vpi_compat_bootstrap, 0}; #else -double sc_time_stamp() { return main_time; } int main(int argc, char** argv, char** env) { - uint64_t sim_time = 1100; - Verilated::commandArgs(argc, argv); - Verilated::debug(0); - // we're going to be checking for these errors do don't crash out - Verilated::fatalOnVpiError(0); + const std::unique_ptr contextp{new VerilatedContext}; - VM_PREFIX* topp = new VM_PREFIX(""); // Note null name - we're flattening it out + uint64_t sim_time = 1100; + contextp->commandArgs(argc, argv); + contextp->debug(0); + // we're going to be checking for these errors do don't crash out + contextp->fatalOnVpiError(0); + + const std::unique_ptr topp{new VM_PREFIX{contextp.get(), + // Note null name - we're flattening it out + ""}}; #ifdef VERILATOR #ifdef TEST_VERBOSE - Verilated::scopesDump(); + contextp->scopesDump(); #endif #endif #if VM_TRACE - Verilated::traceEverOn(true); + contextp->traceEverOn(true); VL_PRINTF("Enabling waves...\n"); VerilatedVcdC* tfp = new VerilatedVcdC; topp->trace(tfp, 99); @@ -266,19 +267,19 @@ int main(int argc, char** argv, char** env) { topp->eval(); topp->clk = 0; - main_time += 10; + contextp->timeInc(10); - while (vl_time_stamp64() < sim_time && !Verilated::gotFinish()) { - main_time += 1; + while (contextp->time() < sim_time && !contextp->gotFinish()) { + contextp->timeInc(1); topp->eval(); VerilatedVpi::callValueCbs(); topp->clk = !topp->clk; // mon_do(); #if VM_TRACE - if (tfp) tfp->dump(main_time); + if (tfp) tfp->dump(contextp->time()); #endif } - if (!Verilated::gotFinish()) { + if (!contextp->gotFinish()) { vl_fatal(FILENM, __LINE__, "main", "%Error: Timeout; never got a $finish"); } topp->final(); @@ -287,7 +288,6 @@ int main(int argc, char** argv, char** env) { if (tfp) tfp->close(); #endif - VL_DO_DANGLING(delete topp, topp); return 0; } diff --git a/test_regress/t/t_vpi_time_cb.cpp b/test_regress/t/t_vpi_time_cb.cpp index 70c65cdeb..1e59a384d 100644 --- a/test_regress/t/t_vpi_time_cb.cpp +++ b/test_regress/t/t_vpi_time_cb.cpp @@ -22,27 +22,27 @@ #include -unsigned int main_time = 0; - //====================================================================== -double sc_time_stamp() { return main_time; } - int main(int argc, char** argv, char** env) { - uint64_t sim_time = 1100; - Verilated::commandArgs(argc, argv); - Verilated::debug(0); + const std::unique_ptr contextp{new VerilatedContext}; - VM_PREFIX* topp = new VM_PREFIX(""); // Note null name - we're flattening it out + uint64_t sim_time = 1100; + contextp->commandArgs(argc, argv); + contextp->debug(0); + + const std::unique_ptr topp{new VM_PREFIX{contextp.get(), + // Note null name - we're flattening it out + ""}}; // clang-format off #ifdef TEST_VERBOSE - Verilated::scopesDump(); + contextp->scopesDump(); #endif // clang-format on #if VM_TRACE - Verilated::traceEverOn(true); + contextp->traceEverOn(true); VL_PRINTF("Enabling waves...\n"); VerilatedVcdC* tfp = new VerilatedVcdC; topp->trace(tfp, 99); @@ -54,24 +54,24 @@ int main(int argc, char** argv, char** env) { topp->eval(); topp->clk = 0; - while (vl_time_stamp64() < sim_time && !Verilated::gotFinish()) { - main_time += 1; + while (vl_time_stamp64() < sim_time && !contextp->gotFinish()) { + contextp->timeInc(1); topp->eval(); VerilatedVpi::callValueCbs(); VerilatedVpi::callTimedCbs(); - if (main_time > 20) { // Else haven't registered callbacks - TEST_CHECK_EQ(VerilatedVpi::cbNextDeadline(), main_time + 1); + if (contextp->time() > 20) { // Else haven't registered callbacks + TEST_CHECK_EQ(VerilatedVpi::cbNextDeadline(), contextp->time() + 1); } - if ((main_time % 5) == 0) topp->clk = !topp->clk; + if ((contextp->time() % 5) == 0) topp->clk = !topp->clk; // mon_do(); #if VM_TRACE - if (tfp) tfp->dump(main_time); + if (tfp) tfp->dump(contextp->time()); #endif } VerilatedVpi::callCbs(cbEndOfSimulation); - if (!Verilated::gotFinish()) { + if (!contextp->gotFinish()) { vl_fatal(__FILE__, __LINE__, "main", "%Error: Timeout; never got a $finish"); } topp->final(); @@ -80,6 +80,5 @@ int main(int argc, char** argv, char** env) { if (tfp) tfp->close(); #endif - VL_DO_DANGLING(delete topp, topp); return errors ? 10 : 0; } diff --git a/test_regress/t/t_vpi_unimpl.cpp b/test_regress/t/t_vpi_unimpl.cpp index e3763b504..e8359ba45 100644 --- a/test_regress/t/t_vpi_unimpl.cpp +++ b/test_regress/t/t_vpi_unimpl.cpp @@ -28,7 +28,6 @@ #define DEBUG \ if (0) printf -unsigned int main_time = 0; unsigned int callback_count = 0; //====================================================================== @@ -184,24 +183,27 @@ extern "C" int mon_check() { //====================================================================== -double sc_time_stamp() { return main_time; } int main(int argc, char** argv, char** env) { - uint64_t sim_time = 1100; - Verilated::commandArgs(argc, argv); - Verilated::debug(0); - // we're going to be checking for these errors do don't crash out - Verilated::fatalOnVpiError(0); + const std::unique_ptr contextp{new VerilatedContext}; - VM_PREFIX* topp = new VM_PREFIX(""); // Note null name - we're flattening it out + uint64_t sim_time = 1100; + contextp->commandArgs(argc, argv); + contextp->debug(0); + // we're going to be checking for these errors do don't crash out + contextp->fatalOnVpiError(0); + + const std::unique_ptr topp{new VM_PREFIX{contextp.get(), + // Note null name - we're flattening it out + ""}}; #ifdef VERILATOR #ifdef TEST_VERBOSE - Verilated::scopesDump(); + contextp->scopesDump(); #endif #endif #if VM_TRACE - Verilated::traceEverOn(true); + contextp->traceEverOn(true); VL_PRINTF("Enabling waves...\n"); VerilatedVcdC* tfp = new VerilatedVcdC; topp->trace(tfp, 99); @@ -210,20 +212,20 @@ int main(int argc, char** argv, char** env) { topp->eval(); topp->clk = 0; - main_time += 10; + contextp->timeInc(10); - while (vl_time_stamp64() < sim_time && !Verilated::gotFinish()) { - main_time += 1; + while (contextp->time() < sim_time && !contextp->gotFinish()) { + contextp->timeInc(1); topp->eval(); // VerilatedVpi::callValueCbs(); // Make sure can link without verilated_vpi.h included topp->clk = !topp->clk; // mon_do(); #if VM_TRACE - if (tfp) tfp->dump(main_time); + if (tfp) tfp->dump(contextp->time()); #endif } if (!callback_count) vl_fatal(FILENM, __LINE__, "main", "%Error: never got callbacks"); - if (!Verilated::gotFinish()) { + if (!contextp->gotFinish()) { vl_fatal(FILENM, __LINE__, "main", "%Error: Timeout; never got a $finish"); } topp->final(); @@ -232,6 +234,5 @@ int main(int argc, char** argv, char** env) { if (tfp) tfp->close(); #endif - VL_DO_DANGLING(delete topp, topp); return 0; } diff --git a/test_regress/t/t_vpi_var.cpp b/test_regress/t/t_vpi_var.cpp index 648557234..f2c4dfa8f 100644 --- a/test_regress/t/t_vpi_var.cpp +++ b/test_regress/t/t_vpi_var.cpp @@ -693,20 +693,24 @@ void (*vlog_startup_routines[])() = {vpi_compat_bootstrap, 0}; double sc_time_stamp() { return main_time; } int main(int argc, char** argv, char** env) { - uint64_t sim_time = 1100; - Verilated::commandArgs(argc, argv); - Verilated::debug(0); + const std::unique_ptr contextp{new VerilatedContext}; - VM_PREFIX* topp = new VM_PREFIX(""); // Note null name - we're flattening it out + uint64_t sim_time = 1100; + contextp->commandArgs(argc, argv); + contextp->debug(0); + + const std::unique_ptr topp{new VM_PREFIX{contextp.get(), + // Note null name - we're flattening it out + ""}}; #ifdef VERILATOR #ifdef TEST_VERBOSE - Verilated::scopesDump(); + contextp->scopesDump(); #endif #endif #if VM_TRACE - Verilated::traceEverOn(true); + contextp->traceEverOn(true); VL_PRINTF("Enabling waves...\n"); VerilatedVcdC* tfp = new VerilatedVcdC; topp->trace(tfp, 99); @@ -717,7 +721,7 @@ int main(int argc, char** argv, char** env) { topp->clk = 0; main_time += 10; - while (vl_time_stamp64() < sim_time && !Verilated::gotFinish()) { + while (vl_time_stamp64() < sim_time && !contextp->gotFinish()) { main_time += 1; topp->eval(); VerilatedVpi::callValueCbs(); @@ -731,7 +735,7 @@ int main(int argc, char** argv, char** env) { CHECK_RESULT(callback_count_half, 250); CHECK_RESULT(callback_count_quad, 2); CHECK_RESULT(callback_count_strs, callback_count_strs_max); - if (!Verilated::gotFinish()) { + if (!contextp->gotFinish()) { vl_fatal(FILENM, __LINE__, "main", "%Error: Timeout; never got a $finish"); } topp->final(); @@ -740,7 +744,6 @@ int main(int argc, char** argv, char** env) { if (tfp) tfp->close(); #endif - VL_DO_DANGLING(delete topp, topp); return 0; } diff --git a/test_regress/t/t_vpi_zero_time_cb.cpp b/test_regress/t/t_vpi_zero_time_cb.cpp index 243e97746..0427ac226 100644 --- a/test_regress/t/t_vpi_zero_time_cb.cpp +++ b/test_regress/t/t_vpi_zero_time_cb.cpp @@ -37,7 +37,6 @@ #include "TestVpi.h" int errors = 0; -unsigned int main_time = 0; unsigned int callback_count_zero_time = 0; unsigned int callback_count_start_of_sim = 0; @@ -105,25 +104,27 @@ void (*vlog_startup_routines[])() = {vpi_compat_bootstrap, 0}; #else -double sc_time_stamp() { return main_time; } - int main(int argc, char** argv, char** env) { - uint64_t sim_time = 1100; - Verilated::commandArgs(argc, argv); - Verilated::debug(0); + const std::unique_ptr contextp{new VerilatedContext}; - VM_PREFIX* topp = new VM_PREFIX(""); // Note null name - we're flattening it out + uint64_t sim_time = 1100; + contextp->commandArgs(argc, argv); + contextp->debug(0); + + const std::unique_ptr topp{new VM_PREFIX{contextp.get(), + // Note null name - we're flattening it out + ""}}; // clang-format off #ifdef VERILATOR # ifdef TEST_VERBOSE - Verilated::scopesDump(); + contextp->scopesDump(); # endif #endif // clang-format on #if VM_TRACE - Verilated::traceEverOn(true); + contextp->traceEverOn(true); VL_PRINTF("Enabling waves...\n"); VerilatedVcdC* tfp = new VerilatedVcdC; topp->trace(tfp, 99); @@ -146,23 +147,23 @@ int main(int argc, char** argv, char** env) { topp->eval(); topp->clk = 0; - main_time += 1; + contextp->timeInc(1); - while (vl_time_stamp64() < sim_time && !Verilated::gotFinish()) { - main_time += 1; + while (contextp->time() < sim_time && !contextp->gotFinish()) { + contextp->timeInc(1); topp->eval(); VerilatedVpi::callValueCbs(); VerilatedVpi::callTimedCbs(); topp->clk = !topp->clk; // mon_do(); #if VM_TRACE - if (tfp) tfp->dump(main_time); + if (tfp) tfp->dump(contextp->time()); #endif } VerilatedVpi::callCbs(cbEndOfSimulation); - if (!Verilated::gotFinish()) { + if (!contextp->gotFinish()) { vl_fatal(__FILE__, __LINE__, "main", "%Error: Timeout; never got a $finish"); } topp->final(); @@ -171,7 +172,6 @@ int main(int argc, char** argv, char** env) { if (tfp) tfp->close(); #endif - VL_DO_DANGLING(delete topp, topp); return 0; } From 8377514127d4e5b2885d3f32da4cf928a4185764 Mon Sep 17 00:00:00 2001 From: Arkadiusz Kozdra Date: Mon, 11 Jul 2022 12:21:35 +0200 Subject: [PATCH 09/12] Add support for $test$plusargs(expr) (#3489) --- docs/CONTRIBUTORS | 1 + include/verilated.cpp | 4 ++-- include/verilated_funcs.h | 2 +- src/V3AstNodes.h | 19 +++++++------------ src/V3EmitCFunc.h | 2 +- src/V3Hasher.cpp | 5 ----- src/V3LinkLValue.cpp | 7 +++++++ src/V3Width.cpp | 7 ++++++- src/verilog.y | 2 +- test_regress/t/t_sys_plusargs.v | 6 ++++++ 10 files changed, 32 insertions(+), 23 deletions(-) diff --git a/docs/CONTRIBUTORS b/docs/CONTRIBUTORS index efd6749cc..6089a5eaa 100644 --- a/docs/CONTRIBUTORS +++ b/docs/CONTRIBUTORS @@ -10,6 +10,7 @@ Alex Chadwick Aliaksei Chapyzhenka Ameya Vikram Singh Andreas Kuster +Arkadiusz Kozdra Chris Randall Chuxuan Wang Conor McCullough diff --git a/include/verilated.cpp b/include/verilated.cpp index 1f6fe3b4c..da8599a42 100644 --- a/include/verilated.cpp +++ b/include/verilated.cpp @@ -1606,8 +1606,8 @@ IData VL_SYSTEM_IW(int lhswords, const WDataInP lhsp) VL_MT_SAFE { return code >> 8; // Want exit status } -IData VL_TESTPLUSARGS_I(const char* formatp) VL_MT_SAFE { - const std::string& match = Verilated::threadContextp()->impp()->argPlusMatch(formatp); +IData VL_TESTPLUSARGS_I(const std::string& format) VL_MT_SAFE { + const std::string& match = Verilated::threadContextp()->impp()->argPlusMatch(format.c_str()); return match.empty() ? 0 : 1; } diff --git a/include/verilated_funcs.h b/include/verilated_funcs.h index ab71839b5..c8fb91d3e 100644 --- a/include/verilated_funcs.h +++ b/include/verilated_funcs.h @@ -146,7 +146,7 @@ extern IData VL_SYSTEM_IW(int lhswords, WDataInP const lhsp); extern IData VL_SYSTEM_IQ(QData lhs); inline IData VL_SYSTEM_II(IData lhs) VL_MT_SAFE { return VL_SYSTEM_IQ(lhs); } -extern IData VL_TESTPLUSARGS_I(const char* formatp); +extern IData VL_TESTPLUSARGS_I(const std::string& format); extern const char* vl_mc_scan_plusargs(const char* prefixp); // PLIish //========================================================================= diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 118666c0b..d9e4b81db 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -4551,26 +4551,21 @@ public: class AstTestPlusArgs final : public AstNodeMath { // Parents: expr // Child: variable to set. If nullptr then this is a $test$plusargs instead of $value$plusargs -private: - string m_text; - public: - AstTestPlusArgs(FileLine* fl, const string& text) - : ASTGEN_SUPER_TestPlusArgs(fl) - , m_text{text} {} + AstTestPlusArgs(FileLine* fl, AstNode* searchp) + : ASTGEN_SUPER_TestPlusArgs(fl) { + setOp1p(searchp); + } ASTNODE_NODE_FUNCS(TestPlusArgs) - virtual string name() const override { return m_text; } virtual string verilogKwd() const override { return "$test$plusargs"; } virtual string emitVerilog() override { return verilogKwd(); } virtual string emitC() override { return "VL_VALUEPLUSARGS_%nq(%lw, %P, nullptr)"; } virtual bool isGateOptimizable() const override { return false; } virtual bool isPredictOptimizable() const override { return false; } virtual bool cleanOut() const override { return true; } - virtual bool same(const AstNode* samep) const override { - return text() == static_cast(samep)->text(); - } - string text() const { return m_text; } // * = Text to display - void text(const string& text) { m_text = text; } + virtual bool same(const AstNode* samep) const override { return true; } + AstNode* searchp() const { return op1p(); } // op1 = Search expression + void searchp(AstNode* nodep) { setOp1p(nodep); } }; class AstGenFor final : public AstNodeFor { diff --git a/src/V3EmitCFunc.h b/src/V3EmitCFunc.h index 4bdfb1c57..305fa47a7 100644 --- a/src/V3EmitCFunc.h +++ b/src/V3EmitCFunc.h @@ -581,7 +581,7 @@ public: } virtual void visit(AstTestPlusArgs* nodep) override { puts("VL_TESTPLUSARGS_I("); - putsQuoted(nodep->text()); + emitCvtPackStr(nodep->searchp()); puts(")"); } virtual void visit(AstFError* nodep) override { diff --git a/src/V3Hasher.cpp b/src/V3Hasher.cpp index 7cf3672b6..4b0cbbbba 100644 --- a/src/V3Hasher.cpp +++ b/src/V3Hasher.cpp @@ -225,11 +225,6 @@ private: m_hash += nodep->text(); }); } - virtual void visit(AstTestPlusArgs* nodep) override { - m_hash += hashNodeAndIterate(nodep, HASH_DTYPE, HASH_CHILDREN, [=]() { // - m_hash += nodep->text(); - }); - } virtual void visit(AstAddrOfCFunc* nodep) override { m_hash += hashNodeAndIterate(nodep, HASH_DTYPE, HASH_CHILDREN, [=]() { // iterateNull(nodep->funcp()); diff --git a/src/V3LinkLValue.cpp b/src/V3LinkLValue.cpp index 6ab7eb8e7..1f955e361 100644 --- a/src/V3LinkLValue.cpp +++ b/src/V3LinkLValue.cpp @@ -210,6 +210,13 @@ private: iterateAndNextNull(nodep->msbp()); } } + virtual void visit(AstTestPlusArgs* nodep) override { + VL_RESTORER(m_setRefLvalue); + { + m_setRefLvalue = VAccess::NOCHANGE; + iterateAndNextNull(nodep->searchp()); + } + } virtual void visit(AstValuePlusArgs* nodep) override { VL_RESTORER(m_setRefLvalue); { diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 15bdf6f33..8a833d9f2 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -448,7 +448,6 @@ private: // Widths: Constant, terminal virtual void visit(AstTime* nodep) override { nodep->dtypeSetUInt64(); } virtual void visit(AstTimeD* nodep) override { nodep->dtypeSetDouble(); } - virtual void visit(AstTestPlusArgs* nodep) override { nodep->dtypeSetSigned32(); } virtual void visit(AstScopeName* nodep) override { nodep->dtypeSetUInt64(); // A pointer, but not that it matters } @@ -4350,6 +4349,12 @@ private: userIterateAndNext(nodep->lsbp(), WidthVP(SELF, BOTH).p()); userIterateAndNext(nodep->msbp(), WidthVP(SELF, BOTH).p()); } + virtual void visit(AstTestPlusArgs* nodep) override { + if (m_vup->prelim()) { + userIterateAndNext(nodep->searchp(), WidthVP{SELF, BOTH}.p()); + nodep->dtypeChgWidthSigned(32, 1, VSigning::SIGNED); // Spec says integer return + } + } virtual void visit(AstValuePlusArgs* nodep) override { if (m_vup->prelim()) { userIterateAndNext(nodep->searchp(), WidthVP(SELF, BOTH).p()); diff --git a/src/verilog.y b/src/verilog.y index ea4161500..34fbe86b0 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -3917,7 +3917,7 @@ system_f_call_or_t: // IEEE: part of system_tf_call (can be task or | yD_STABLE '(' expr ',' expr ')' { $$ = $3; BBUNSUP($1, "Unsupported: $stable and clock arguments"); } | yD_TAN '(' expr ')' { $$ = new AstTanD($1,$3); } | yD_TANH '(' expr ')' { $$ = new AstTanhD($1,$3); } - | yD_TESTPLUSARGS '(' str ')' { $$ = new AstTestPlusArgs($1,*$3); } + | yD_TESTPLUSARGS '(' expr ')' { $$ = new AstTestPlusArgs($1, $3); } | yD_TIME parenE { $$ = new AstTime($1, VTimescale(VTimescale::NONE)); } | yD_TYPENAME '(' exprOrDataType ')' { $$ = new AstAttrOf($1, VAttrType::TYPENAME, $3); } | yD_UNGETC '(' expr ',' expr ')' { $$ = new AstFUngetC($1, $5, $3); } // Arg swap to file first diff --git a/test_regress/t/t_sys_plusargs.v b/test_regress/t/t_sys_plusargs.v index 55daf5085..4aef6e33c 100644 --- a/test_regress/t/t_sys_plusargs.v +++ b/test_regress/t/t_sys_plusargs.v @@ -22,6 +22,12 @@ module t; //if ($test$plusargs("")!==1) $stop; // Simulators differ in this answer if ($test$plusargs("NOTTHERE")!==0) $stop; + sv_in = "PLUS"; +`ifdef VERILATOR + if ($c1(0)) sv_in = "NEVER"; // Prevent constant propagation +`endif + if ($test$plusargs(sv_in)!==1) $stop; + p_i = 10; if ($value$plusargs("NOTTHERE%d", p_i) !== 0) $stop; if ($value$plusargs("NOTTHERE%0d", p_i) !== 0) $stop; From f4038e36743bfb5d5860293520df781deedfe598 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Tue, 12 Jul 2022 11:41:15 +0100 Subject: [PATCH 10/12] Move thread pool and execution profiler into the context. (#3477) Fixes #3454 --- include/verilated.cpp | 58 +++++++++ include/verilated.h | 48 +++++++- include/verilated_profiler.cpp | 43 +++++-- include/verilated_profiler.h | 17 +-- include/verilated_threads.cpp | 54 ++++----- include/verilated_threads.h | 32 ++--- include/verilated_trace.h | 22 ++-- include/verilated_trace_imp.h | 133 ++++++++++----------- src/V3EmitCHeaders.cpp | 6 +- src/V3EmitCMake.cpp | 2 +- src/V3EmitCModel.cpp | 31 +++-- src/V3EmitCSyms.cpp | 24 ++-- src/V3EmitMk.cpp | 2 +- src/V3Trace.cpp | 9 +- test_regress/driver.pl | 8 +- test_regress/t/t_embed1.pl | 3 +- test_regress/t/t_gantt_two.cpp | 43 +++++++ test_regress/t/t_gantt_two.pl | 61 ++++++++++ test_regress/t/t_hier_block_cmake/main.cpp | 5 +- test_regress/t/t_lib_prot_shared.pl | 3 +- test_regress/t/t_threads_crazy.pl | 12 +- test_regress/t/t_threads_crazy_context.pl | 36 ++++++ test_regress/t/t_wrapper_context.cpp | 2 + 23 files changed, 470 insertions(+), 184 deletions(-) create mode 100644 test_regress/t/t_gantt_two.cpp create mode 100755 test_regress/t/t_gantt_two.pl create mode 100755 test_regress/t/t_threads_crazy_context.pl diff --git a/include/verilated.cpp b/include/verilated.cpp index da8599a42..cf1d76d8f 100644 --- a/include/verilated.cpp +++ b/include/verilated.cpp @@ -66,6 +66,10 @@ #if defined(_WIN32) || defined(__MINGW32__) # include // mkdir #endif + +#ifdef VL_THREADED +# include "verilated_threads.h" +#endif // clang-format on // Max characters in static char string for VL_VALUE_STRING @@ -2428,6 +2432,33 @@ const char* VerilatedContext::timeprecisionString() const VL_MT_SAFE { return vl_time_str(timeprecision()); } +void VerilatedContext::threads(unsigned n) { + if (n == 0) VL_FATAL_MT(__FILE__, __LINE__, "", "%Error: Simulation threads must be >= 1"); + + if (m_threadPool) { + VL_FATAL_MT( + __FILE__, __LINE__, "", + "%Error: Cannot set simulation threads after the thread pool has been created."); + } + +#if VL_THREADED + if (m_threads == n) return; // To avoid unnecessary warnings + m_threads = n; + const unsigned hardwareThreadsAvailable = std::thread::hardware_concurrency(); + if (m_threads > hardwareThreadsAvailable) { + VL_PRINTF_MT("%%Warning: System has %u hardware threads but simulation thread count set " + "to %u. This will likely cause significant slowdown.\n", + hardwareThreadsAvailable, m_threads); + } +#else + if (n > 1) { + VL_PRINTF_MT("%%Warning: Verilator run-time library built without VL_THREADS. Ignoring " + "call to 'VerilatedContext::threads' with argument %u.\n", + n); + } +#endif +} + void VerilatedContext::commandArgs(int argc, const char** argv) VL_MT_SAFE_EXCLUDES(m_argMutex) { const VerilatedLockGuard lock{m_argMutex}; m_args.m_argVec.clear(); // Empty first, then add @@ -2458,6 +2489,33 @@ void VerilatedContext::internalsDump() const VL_MT_SAFE { VerilatedImp::userDump(); } +void VerilatedContext::addModel(VerilatedModel* modelp) { + threadPoolp(); // Ensure thread pool is created, so m_threads cannot change any more + + if (modelp->threads() > m_threads) { + std::ostringstream msg; + msg << "VerilatedContext has " << m_threads << " threads but model '" + << modelp->modelName() << "' (instantiated as '" << modelp->hierName() + << "') was Verilated with --threads " << modelp->threads() << ".\n"; + const std::string str = msg.str(); + VL_FATAL_MT(__FILE__, __LINE__, modelp->hierName(), str.c_str()); + } +} + +VerilatedVirtualBase* VerilatedContext::threadPoolp() { + if (m_threads == 1) return nullptr; +#if VL_THREADED + if (!m_threadPool) m_threadPool.reset(new VlThreadPool{this, m_threads - 1}); +#endif + return m_threadPool.get(); +} + +VerilatedVirtualBase* +VerilatedContext::enableExecutionProfiler(VerilatedVirtualBase* (*construct)(VerilatedContext&)) { + if (!m_executionProfiler) m_executionProfiler.reset(construct(*this)); + return m_executionProfiler.get(); +} + //====================================================================== // VerilatedContextImp:: Methods - command line diff --git a/include/verilated.h b/include/verilated.h index f9cf79601..bc1d5a3f2 100644 --- a/include/verilated.h +++ b/include/verilated.h @@ -252,6 +252,28 @@ public: #endif }; +//========================================================================= +/// Base class of a Verilator generated (Verilated) model. +/// +/// VerilatedModel is a base class of the user facing primary class generated +/// by Verilator. + +class VerilatedModel VL_NOT_FINAL { + VL_UNCOPYABLE(VerilatedModel); + +protected: + explicit VerilatedModel() = default; + virtual ~VerilatedModel() = default; + +public: + /// Returns the hierarchical name of this module instance. + virtual const char* hierName() = 0; + /// Returns the name of this model (the name of the generated model class). + virtual const char* modelName() = 0; + /// Returns the thread level parallelism, this model was Verilated with. Always 1 or higher. + virtual unsigned threads() = 0; +}; + //========================================================================= /// Base class for all Verilated module classes. @@ -266,10 +288,6 @@ public: const char* name() const { return m_namep; } ///< Return name of module }; -/// Declare a module, ala SC_MODULE -#define VL_MODULE(modname) class modname VL_NOT_FINAL : public VerilatedModule -// Not class final in VL_MODULE, as users might be abstracting our models (--hierarchical) - //========================================================================= // Functions overridable by user defines // (Internals however must use VL_PRINTF_MT, which calls these.) @@ -362,6 +380,16 @@ protected: // Implementation details const std::unique_ptr m_impdatap; + // Number of threads to use for simulation (size of m_threadPool + 1 for main thread) +#ifdef VL_THREADED + unsigned m_threads = std::thread::hardware_concurrency(); +#else + const unsigned m_threads = 1; +#endif + // The thread pool shared by all models added to this context + std::unique_ptr m_threadPool; + // The execution profiler shared by all models added to this context + std::unique_ptr m_executionProfiler; // Coverage access std::unique_ptr m_coveragep; // Pointer for coveragep() @@ -495,6 +523,12 @@ public: /// Get time precision as IEEE-standard text const char* timeprecisionString() const VL_MT_SAFE; + /// Get number of threads used for simulation (including the main thread) + unsigned threads() const { return m_threads; } + /// Set number of threads used for simulation (including the main thread) + /// Can only be called before the thread pool is created (before first model is added). + void threads(unsigned n); + /// Allow traces to at some point be enabled (disables some optimizations) void traceEverOn(bool flag) VL_MT_SAFE { if (flag) calcUnusedSigs(true); @@ -517,6 +551,12 @@ public: // But for internal use only return reinterpret_cast(this); } + void addModel(VerilatedModel*); + + VerilatedVirtualBase* threadPoolp(); + VerilatedVirtualBase* + enableExecutionProfiler(VerilatedVirtualBase* (*construct)(VerilatedContext&)); + // Internal: $dumpfile void dumpfile(const std::string& flag) VL_MT_SAFE_EXCLUDES(m_timeDumpMutex); std::string dumpfile() const VL_MT_SAFE_EXCLUDES(m_timeDumpMutex); diff --git a/include/verilated_profiler.cpp b/include/verilated_profiler.cpp index 21246827a..d65442f44 100644 --- a/include/verilated_profiler.cpp +++ b/include/verilated_profiler.cpp @@ -66,41 +66,66 @@ template static size_t roundUptoMultipleOf(size_t value) { return (value + mask) & ~mask; } -VlExecutionProfiler::VlExecutionProfiler() { +VlExecutionProfiler::VlExecutionProfiler(VerilatedContext& context) + : m_context{context} { // Setup profiling on main thread setupThread(0); } -void VlExecutionProfiler::configure(const VerilatedContext& context) { +void VlExecutionProfiler::configure() { + if (VL_UNLIKELY(m_enabled)) { --m_windowCount; - if (VL_UNLIKELY(m_windowCount == context.profExecWindow())) { + if (VL_UNLIKELY(m_windowCount == m_context.profExecWindow())) { VL_DEBUG_IF(VL_DBG_MSGF("+ profile start collection\n");); clear(); // Clear the profile after the cache warm-up cycles. m_tickBegin = VL_CPU_TICK(); } else if (VL_UNLIKELY(m_windowCount == 0)) { const uint64_t tickEnd = VL_CPU_TICK(); VL_DEBUG_IF(VL_DBG_MSGF("+ profile end\n");); - const std::string& fileName = context.profExecFilename(); + const std::string& fileName = m_context.profExecFilename(); dump(fileName.c_str(), tickEnd); m_enabled = false; } return; } - const uint64_t startReq = context.profExecStart() + 1; // + 1, so we can start at time 0 + const uint64_t startReq = m_context.profExecStart() + 1; // + 1, so we can start at time 0 - if (VL_UNLIKELY(m_lastStartReq < startReq && VL_TIME_Q() >= context.profExecStart())) { + if (VL_UNLIKELY(m_lastStartReq < startReq && VL_TIME_Q() >= m_context.profExecStart())) { VL_DEBUG_IF(VL_DBG_MSGF("+ profile start warmup\n");); VL_DEBUG_IF(assert(m_windowCount == 0);); m_enabled = true; - m_windowCount = context.profExecWindow() * 2; + m_windowCount = m_context.profExecWindow() * 2; m_lastStartReq = startReq; } } -void VlExecutionProfiler::startWorkerSetup(VlExecutionProfiler* profilep, uint32_t threadId) { - profilep->setupThread(threadId); +VerilatedVirtualBase* VlExecutionProfiler::construct(VerilatedContext& context) { + VlExecutionProfiler* const selfp = new VlExecutionProfiler{context}; +#if VL_THREADED + if (VlThreadPool* const threadPoolp = static_cast(context.threadPoolp())) { + for (int i = 0; i < threadPoolp->numThreads(); ++i) { + // Data to pass to worker thread initialization + struct Data { + VlExecutionProfiler* const selfp; + const uint32_t threadId; + } data{selfp, static_cast(i + 1)}; + + // Initialize worker thread + threadPoolp->workerp(i)->addTask( + [](void* userp, bool) { + Data* const datap = static_cast(userp); + datap->selfp->setupThread(datap->threadId); + }, + &data); + + // Wait until initializationis complete + threadPoolp->workerp(i)->wait(); + } + } +#endif + return selfp; } void VlExecutionProfiler::setupThread(uint32_t threadId) { diff --git a/include/verilated_profiler.h b/include/verilated_profiler.h index f85c95528..61f2813d3 100644 --- a/include/verilated_profiler.h +++ b/include/verilated_profiler.h @@ -33,13 +33,14 @@ #include class VlExecutionProfiler; +class VlThreadPool; //============================================================================= // Macros to simplify generated code #define VL_EXEC_TRACE_ADD_RECORD(vlSymsp) \ - if (VL_UNLIKELY((vlSymsp)->__Vm_executionProfiler.enabled())) \ - (vlSymsp)->__Vm_executionProfiler.addRecord() + if (VL_UNLIKELY((vlSymsp)->__Vm_executionProfilerp->enabled())) \ + (vlSymsp)->__Vm_executionProfilerp->addRecord() //============================================================================= // Return high-precision counter for profiling, or 0x0 if not available @@ -131,7 +132,7 @@ static_assert(std::is_trivially_destructible::value, //============================================================================= // VlExecutionProfiler is for collecting profiling data about model execution -class VlExecutionProfiler final { +class VlExecutionProfiler final : public VerilatedVirtualBase { // CONSTANTS // In order to try to avoid dynamic memory allocations during the actual profiling phase, @@ -149,6 +150,7 @@ class VlExecutionProfiler final { using ExecutionTrace = std::vector; // STATE + VerilatedContext& m_context; // The context this profiler is under static VL_THREAD_LOCAL ExecutionTrace t_trace; // thread-local trace buffers mutable VerilatedMutex m_mutex; // Map from thread id to &t_trace of given thread @@ -162,7 +164,8 @@ class VlExecutionProfiler final { public: // CONSTRUCTOR - VlExecutionProfiler(); + explicit VlExecutionProfiler(VerilatedContext& context); + virtual ~VlExecutionProfiler() = default; // METHODS @@ -174,7 +177,7 @@ public: return t_trace.back(); } // Configure profiler (called in beginning of 'eval') - void configure(const VerilatedContext&); + void configure(); // Setup profiling on a particular thread; void setupThread(uint32_t threadId); // Clear all profiling data @@ -182,8 +185,8 @@ public: // Write profiling data into file void dump(const char* filenamep, uint64_t tickEnd) VL_MT_SAFE_EXCLUDES(m_mutex); - // Called via VlStartWorkerCb in VlWorkerThread::startWorker - static void startWorkerSetup(VlExecutionProfiler* profilep, uint32_t threadId); + // Passed to VerilatedContext to create the VlExecutionProfiler profiler instance + static VerilatedVirtualBase* construct(VerilatedContext& context); }; //============================================================================= diff --git a/include/verilated_threads.cpp b/include/verilated_threads.cpp index a78ea9ae6..6696d738d 100644 --- a/include/verilated_threads.cpp +++ b/include/verilated_threads.cpp @@ -47,11 +47,9 @@ VlMTaskVertex::VlMTaskVertex(uint32_t upstreamDepCount) //============================================================================= // VlWorkerThread -VlWorkerThread::VlWorkerThread(uint32_t threadId, VerilatedContext* contextp, - VlExecutionProfiler* profilerp, VlStartWorkerCb startCb) +VlWorkerThread::VlWorkerThread(VerilatedContext* contextp) : m_ready_size{0} - , m_cthread{startWorker, this, threadId, profilerp, startCb} - , m_contextp{contextp} {} + , m_cthread{startWorker, this, contextp} {} VlWorkerThread::~VlWorkerThread() { shutdown(); @@ -59,47 +57,49 @@ VlWorkerThread::~VlWorkerThread() { m_cthread.join(); } -void VlWorkerThread::shutdownTask(void*, bool) { +static void shutdownTask(void*, bool) { // Deliberately empty, we use the address of this function as a magic number } +void VlWorkerThread::shutdown() { addTask(shutdownTask, nullptr); } + +void VlWorkerThread::wait() { + // Enqueue a task that sets this flag. Execution is in-order so this ensures completion. + std::atomic flag{false}; + addTask([](void* flagp, bool) { static_cast*>(flagp)->store(true); }, &flag); + // Spin wait + for (unsigned i = 0; i < VL_LOCK_SPINS; ++i) { + if (flag.load()) return; + VL_CPU_RELAX(); + } + // Yield wait + while (!flag.load()) std::this_thread::yield(); +} + void VlWorkerThread::workerLoop() { ExecRec work; + // Wait for the first task without spinning, in case the thread is never actually used. + dequeWork(&work); + while (true) { - dequeWork(&work); if (VL_UNLIKELY(work.m_fnp == shutdownTask)) break; work.m_fnp(work.m_selfp, work.m_evenCycle); + // Wait for next task with spinning. + dequeWork(&work); } } -void VlWorkerThread::startWorker(VlWorkerThread* workerp, uint32_t threadId, - VlExecutionProfiler* profilerp, VlStartWorkerCb startCb) { - Verilated::threadContextp(workerp->m_contextp); - if (VL_UNLIKELY(startCb)) startCb(profilerp, threadId); +void VlWorkerThread::startWorker(VlWorkerThread* workerp, VerilatedContext* contextp) { + Verilated::threadContextp(contextp); workerp->workerLoop(); } //============================================================================= // VlThreadPool -VlThreadPool::VlThreadPool(VerilatedContext* contextp, int nThreads, - VlExecutionProfiler* profilerp, VlStartWorkerCb startCb) { - // --threads N passes nThreads=N-1, as the "main" threads counts as 1 - ++nThreads; - const unsigned cpus = std::thread::hardware_concurrency(); - if (cpus < nThreads) { - static int warnedOnce = 0; - if (!warnedOnce++) { - VL_PRINTF_MT("%%Warning: System has %u CPUs but model Verilated with" - " --threads %d; may run slow.\n", - cpus, nThreads); - } - } - // Create worker threads - for (uint32_t threadId = 1; threadId < nThreads; ++threadId) { - m_workers.push_back(new VlWorkerThread{threadId, contextp, profilerp, startCb}); - } +VlThreadPool::VlThreadPool(VerilatedContext* contextp, unsigned nThreads) { + for (unsigned i = 0; i < nThreads; ++i) m_workers.push_back(new VlWorkerThread{contextp}); } VlThreadPool::~VlThreadPool() { diff --git a/include/verilated_threads.h b/include/verilated_threads.h index eeb8f9342..fdb45580e 100644 --- a/include/verilated_threads.h +++ b/include/verilated_threads.h @@ -60,9 +60,6 @@ using VlSelfP = void*; using VlExecFnp = void (*)(VlSelfP, bool); -// VlWorkerThread::startWorker callback, used to hook in VlExecutionProfiler -using VlStartWorkerCb = void (*)(VlExecutionProfiler*, uint32_t threadId); - // Track dependencies for a single MTask. class VlMTaskVertex final { // MEMBERS @@ -166,24 +163,23 @@ private: std::atomic m_ready_size; std::thread m_cthread; // Underlying C++ thread record - VerilatedContext* const m_contextp; // Context for spawned thread VL_UNCOPYABLE(VlWorkerThread); public: // CONSTRUCTORS - explicit VlWorkerThread(uint32_t threadId, VerilatedContext* contextp, - VlExecutionProfiler* profilerp, VlStartWorkerCb startCb); + explicit VlWorkerThread(VerilatedContext* contextp); ~VlWorkerThread(); // METHODS + template // inline void dequeWork(ExecRec* workp) VL_MT_SAFE_EXCLUDES(m_mutex) { // Spin for a while, waiting for new data - for (int i = 0; i < VL_LOCK_SPINS; ++i) { - if (VL_LIKELY(m_ready_size.load(std::memory_order_relaxed))) { // - break; + if VL_CONSTEXPR_CXX17 (SpinWait) { + for (unsigned i = 0; i < VL_LOCK_SPINS; ++i) { + if (VL_LIKELY(m_ready_size.load(std::memory_order_relaxed))) break; + VL_CPU_RELAX(); } - VL_CPU_RELAX(); } VerilatedLockGuard lock{m_mutex}; while (m_ready.empty()) { @@ -197,7 +193,7 @@ public: m_ready.erase(m_ready.begin()); m_ready_size.fetch_sub(1, std::memory_order_relaxed); } - inline void addTask(VlExecFnp fnp, VlSelfP selfp, bool evenCycle) + inline void addTask(VlExecFnp fnp, VlSelfP selfp, bool evenCycle = false) VL_MT_SAFE_EXCLUDES(m_mutex) { bool notify; { @@ -209,15 +205,14 @@ public: if (notify) m_cv.notify_one(); } - inline void shutdown() { addTask(shutdownTask, nullptr, false); } - static void shutdownTask(void*, bool); + void shutdown(); // Finish current tasks, then terminate thread + void wait(); // Blocks calling thread until all tasks complete in this thread void workerLoop(); - static void startWorker(VlWorkerThread* workerp, uint32_t threadId, - VlExecutionProfiler* profilerp, VlStartWorkerCb startCb); + static void startWorker(VlWorkerThread* workerp, VerilatedContext* contextp); }; -class VlThreadPool final { +class VlThreadPool final : public VerilatedVirtualBase { // MEMBERS std::vector m_workers; // our workers @@ -226,9 +221,8 @@ public: // Construct a thread pool with 'nThreads' dedicated threads. The thread // pool will create these threads and make them available to execute tasks // via this->workerp(index)->addTask(...) - VlThreadPool(VerilatedContext* contextp, int nThreads, VlExecutionProfiler* profilerp, - VlStartWorkerCb startCb); - ~VlThreadPool(); + VlThreadPool(VerilatedContext* contextp, unsigned nThreads); + virtual ~VlThreadPool(); // METHODS inline int numThreads() const { return m_workers.size(); } diff --git a/include/verilated_trace.h b/include/verilated_trace.h index 7915c3645..0d0f7c0f6 100644 --- a/include/verilated_trace.h +++ b/include/verilated_trace.h @@ -194,8 +194,6 @@ private: static void parallelWorkerTask(void*, bool); #endif - using ParallelCallbackMap = std::unordered_map>; - protected: uint32_t* m_sigs_oldvalp = nullptr; // Previous value store EData* m_sigs_enabledp = nullptr; // Bit vector of enabled codes (nullptr = all on) @@ -203,10 +201,10 @@ private: uint64_t m_timeLastDump = 0; // Last time we did a dump std::vector m_sigs_enabledVec; // Staging for m_sigs_enabledp std::vector m_initCbs; // Routines to initialize tracing - ParallelCallbackMap m_fullCbs; // Routines to perform full dump - ParallelCallbackMap m_chgCbs; // Routines to perform incremental dump + std::vector m_fullCbs; // Routines to perform full dump + std::vector m_chgCbs; // Routines to perform incremental dump std::vector m_cleanupCbs; // Routines to call at the end of dump - std::vector m_threadPoolps; // All thread pools, in insertion order + VerilatedContext* m_contextp = nullptr; // The context used by the traced models bool m_fullDump = true; // Whether a full dump is required on the next call to 'dump' uint32_t m_nextCode = 0; // Next code number to assign uint32_t m_numSignals = 0; // Number of distinct signals @@ -217,16 +215,16 @@ private: double m_timeRes = 1e-9; // Time resolution (ns/ms etc) double m_timeUnit = 1e-0; // Time units (ns/ms etc) - void addThreadPool(VlThreadPool* threadPoolp) VL_MT_SAFE_EXCLUDES(m_mutex); + void addContext(VerilatedContext*) VL_MT_SAFE_EXCLUDES(m_mutex); - void addCallbackRecord(std::vector& cbVec, CallbackRecord& cbRec) + void addCallbackRecord(std::vector& cbVec, CallbackRecord&& cbRec) VL_MT_SAFE_EXCLUDES(m_mutex); // Equivalent to 'this' but is of the sub-type 'T_Trace*'. Use 'self()->' // to access duck-typed functions to avoid a virtual function call. T_Trace* self() { return static_cast(this); } - void runParallelCallbacks(const ParallelCallbackMap& cbMap); + void runCallbacks(const std::vector& cbVec); // Flush any remaining data for this file static void onFlush(void* selfp) VL_MT_UNSAFE_ONE; @@ -341,10 +339,10 @@ public: //========================================================================= // Non-hot path internal interface to Verilator generated code - void addInitCb(initCb_t cb, void* userp) VL_MT_SAFE; - void addFullCb(dumpCb_t cb, void* userp, VlThreadPool* = nullptr) VL_MT_SAFE; - void addChgCb(dumpCb_t cb, void* userp, VlThreadPool* = nullptr) VL_MT_SAFE; - void addCleanupCb(cleanupCb_t cb, void* userp) VL_MT_SAFE; + void addInitCb(initCb_t cb, void* userp, VerilatedContext*) VL_MT_SAFE; + void addFullCb(dumpCb_t cb, void* userp, VerilatedContext*) VL_MT_SAFE; + void addChgCb(dumpCb_t cb, void* userp, VerilatedContext*) VL_MT_SAFE; + void addCleanupCb(cleanupCb_t cb, void* userp, VerilatedContext*) VL_MT_SAFE; void scopeEscape(char flag) { m_scopeEscape = flag; } diff --git a/include/verilated_trace_imp.h b/include/verilated_trace_imp.h index d2ffa965c..a09ac0f43 100644 --- a/include/verilated_trace_imp.h +++ b/include/verilated_trace_imp.h @@ -478,55 +478,52 @@ template <> VL_ATTR_NOINLINE void VerilatedTrace::ParallelWo #endif template <> -void VerilatedTrace::runParallelCallbacks(const ParallelCallbackMap& cbMap) { - for (VlThreadPool* threadPoolp : m_threadPoolps) { +void VerilatedTrace::runCallbacks(const std::vector& cbVec) { #ifdef VL_TRACE_PARALLEL - // If tracing in parallel, dispatch to the thread pool (if exists) - if (threadPoolp && threadPoolp->numThreads()) { - // List of work items for thread (std::list, as ParallelWorkerData is not movable) - std::list workerData; - // We use the whole pool + the main thread - const unsigned threads = threadPoolp->numThreads() + 1; - // Main thread executes all jobs with index % threads == 0 - std::vector mainThreadWorkerData; - // The tracing callbacks to execute on this thread-pool - const auto& cbVec = cbMap.at(threadPoolp); - // Enuque all the jobs - for (unsigned i = 0; i < cbVec.size(); ++i) { - const CallbackRecord& cbr = cbVec[i]; - // Always get the trace buffer on the main thread - Buffer* const bufp = getTraceBuffer(); - // Create new work item - workerData.emplace_back(cbr.m_dumpCb, cbr.m_userp, bufp); - // Grab the new work item - ParallelWorkerData* const itemp = &workerData.back(); - // Enqueue task to thread pool, or main thread - if (unsigned rem = i % threads) { - threadPoolp->workerp(rem - 1)->addTask(parallelWorkerTask, itemp, false); - } else { - mainThreadWorkerData.push_back(itemp); - } - } - // Execute main thead jobs - for (ParallelWorkerData* const itemp : mainThreadWorkerData) { - parallelWorkerTask(itemp, false); - } - // Commit all trace buffers in order - for (ParallelWorkerData& item : workerData) { - // Wait until ready - item.wait(); - // Commit the buffer - commitTraceBuffer(item.m_bufp); - } - continue; + // If tracing in parallel, dispatch to the thread pool + VlThreadPool* threadPoolp = static_cast(m_contextp->threadPoolp()); + // List of work items for thread (std::list, as ParallelWorkerData is not movable) + std::list workerData; + // We use the whole pool + the main thread + const unsigned threads = threadPoolp->numThreads() + 1; + // Main thread executes all jobs with index % threads == 0 + std::vector mainThreadWorkerData; + // Enuque all the jobs + for (unsigned i = 0; i < cbVec.size(); ++i) { + const CallbackRecord& cbr = cbVec[i]; + // Always get the trace buffer on the main thread + Buffer* const bufp = getTraceBuffer(); + // Create new work item + workerData.emplace_back(cbr.m_dumpCb, cbr.m_userp, bufp); + // Grab the new work item + ParallelWorkerData* const itemp = &workerData.back(); + // Enqueue task to thread pool, or main thread + if (unsigned rem = i % threads) { + threadPoolp->workerp(rem - 1)->addTask(parallelWorkerTask, itemp); + } else { + mainThreadWorkerData.push_back(itemp); } + } + // Execute main thead jobs + for (ParallelWorkerData* const itemp : mainThreadWorkerData) { + parallelWorkerTask(itemp, false); + } + // Commit all trace buffers in order + for (ParallelWorkerData& item : workerData) { + // Wait until ready + item.wait(); + // Commit the buffer + commitTraceBuffer(item.m_bufp); + } + + // Done + return; #endif - // Fall back on sequential execution - for (const CallbackRecord& cbr : cbMap.at(threadPoolp)) { - Buffer* const traceBufferp = getTraceBuffer(); - cbr.m_dumpCb(cbr.m_userp, traceBufferp); - commitTraceBuffer(traceBufferp); - } + // Fall back on sequential execution + for (const CallbackRecord& cbr : cbVec) { + Buffer* const traceBufferp = getTraceBuffer(); + cbr.m_dumpCb(cbr.m_userp, traceBufferp); + commitTraceBuffer(traceBufferp); } } @@ -579,9 +576,9 @@ void VerilatedTrace::dump(uint64_t timeui) VL_MT_SAFE_EXCLUD // Run the callbacks if (VL_UNLIKELY(m_fullDump)) { m_fullDump = false; // No more need for next dump to be full - runParallelCallbacks(m_fullCbs); + runCallbacks(m_fullCbs); } else { - runParallelCallbacks(m_chgCbs); + runCallbacks(m_chgCbs); } for (uint32_t i = 0; i < m_cleanupCbs.size(); ++i) { @@ -607,18 +604,20 @@ void VerilatedTrace::dump(uint64_t timeui) VL_MT_SAFE_EXCLUD // Non-hot path internal interface to Verilator generated code template <> -void VerilatedTrace::addThreadPool(VlThreadPool* threadPoolp) +void VerilatedTrace::addContext(VerilatedContext* contextp) VL_MT_SAFE_EXCLUDES(m_mutex) { const VerilatedLockGuard lock{m_mutex}; - for (VlThreadPool* const poolp : m_threadPoolps) { - if (poolp == threadPoolp) return; + if (m_contextp && contextp != m_contextp) { + VL_FATAL_MT( + __FILE__, __LINE__, "", + "A trace file instance can only handle models from the same simulation context"); } - m_threadPoolps.push_back(threadPoolp); + m_contextp = contextp; } template <> void VerilatedTrace::addCallbackRecord(std::vector& cbVec, - CallbackRecord& cbRec) + CallbackRecord&& cbRec) VL_MT_SAFE_EXCLUDES(m_mutex) { const VerilatedLockGuard lock{m_mutex}; if (VL_UNCOVERABLE(timeLastDump() != 0)) { // LCOV_EXCL_START @@ -630,28 +629,28 @@ void VerilatedTrace::addCallbackRecord(std::vector -void VerilatedTrace::addInitCb(initCb_t cb, void* userp) VL_MT_SAFE { - CallbackRecord cbr{cb, userp}; - addCallbackRecord(m_initCbs, cbr); +void VerilatedTrace::addInitCb(initCb_t cb, void* userp, + VerilatedContext* contextp) VL_MT_SAFE { + addContext(contextp); + addCallbackRecord(m_initCbs, CallbackRecord{cb, userp}); } template <> void VerilatedTrace::addFullCb(dumpCb_t cb, void* userp, - VlThreadPool* threadPoolp) VL_MT_SAFE { - CallbackRecord cbr{cb, userp}; - addThreadPool(threadPoolp); - addCallbackRecord(m_fullCbs[threadPoolp], cbr); + VerilatedContext* contextp) VL_MT_SAFE { + addContext(contextp); + addCallbackRecord(m_fullCbs, CallbackRecord{cb, userp}); } template <> void VerilatedTrace::addChgCb(dumpCb_t cb, void* userp, - VlThreadPool* threadPoolp) VL_MT_SAFE { - CallbackRecord cbr{cb, userp}; - addThreadPool(threadPoolp); - addCallbackRecord(m_chgCbs[threadPoolp], cbr); + VerilatedContext* contextp) VL_MT_SAFE { + addContext(contextp); + addCallbackRecord(m_chgCbs, CallbackRecord{cb, userp}); } template <> -void VerilatedTrace::addCleanupCb(cleanupCb_t cb, void* userp) VL_MT_SAFE { - CallbackRecord cbr{cb, userp}; - addCallbackRecord(m_cleanupCbs, cbr); +void VerilatedTrace::addCleanupCb(cleanupCb_t cb, void* userp, + VerilatedContext* contextp) VL_MT_SAFE { + addContext(contextp); + addCallbackRecord(m_cleanupCbs, CallbackRecord{cb, userp}); } template <> void VerilatedTrace::pushNamePrefix(const std::string& prefix) { diff --git a/src/V3EmitCHeaders.cpp b/src/V3EmitCHeaders.cpp index ef53dfa16..77835212b 100644 --- a/src/V3EmitCHeaders.cpp +++ b/src/V3EmitCHeaders.cpp @@ -251,15 +251,15 @@ class EmitCHeader final : public EmitCConstInit { emitTextSection(modp, VNType::atScHdr); // Open class body {{{ + puts("\nclass "); + puts(prefixNameProtect(modp)); if (const AstClass* const classp = VN_CAST(modp, Class)) { - puts("class "); - puts(prefixNameProtect(modp)); if (classp->extendsp()) { puts(" : public "); puts(prefixNameProtect(classp->extendsp()->classp())); } } else { - puts("VL_MODULE(" + prefixNameProtect(modp) + ")"); + puts(" final : public VerilatedModule"); } puts(" {\n"); ofp()->resetPrivate(); diff --git a/src/V3EmitCMake.cpp b/src/V3EmitCMake.cpp index 7df71dfeb..710829eaf 100644 --- a/src/V3EmitCMake.cpp +++ b/src/V3EmitCMake.cpp @@ -173,7 +173,7 @@ class CMakeEmitter final { + ".cpp"); } } - if (v3Global.opt.mtasks()) { + if (v3Global.opt.threads()) { global.emplace_back("${VERILATOR_ROOT}/include/verilated_threads.cpp"); } if (v3Global.opt.usesProfiler()) { diff --git a/src/V3EmitCModel.cpp b/src/V3EmitCModel.cpp index e04c79f7e..203582609 100644 --- a/src/V3EmitCModel.cpp +++ b/src/V3EmitCModel.cpp @@ -89,11 +89,12 @@ class EmitCModel final : public EmitCFunc { puts("\n"); puts("// This class is the main interface to the Verilated model\n"); + puts("class " + topClassName() + " VL_NOT_FINAL : "); if (optSystemC()) { - puts("SC_MODULE(" + topClassName() + ") {\n"); - } else { - puts("class " + topClassName() + " VL_NOT_FINAL {\n"); + // SC_MODULE, but with multiple-inheritance of VerilatedModel + puts("public ::sc_core::sc_module, "); } + puts("public VerilatedModel {\n"); ofp()->resetPrivate(); ofp()->putsPrivate(true); // private: @@ -221,6 +222,11 @@ class EmitCModel final : public EmitCFunc { + topClassName() + "& rhs);\n"); } + puts("\n// Abstract methods from VerilatedModel\n"); + puts("const char* hierName() override;\n"); + puts("const char* modelName() override;\n"); + puts("unsigned threads() override;\n"); + puts("} VL_ATTR_ALIGNED(VL_CACHE_LINE_BYTES);\n"); ofp()->putsEndGuard(); @@ -235,7 +241,8 @@ class EmitCModel final : public EmitCFunc { puts(topClassName() + "::" + topClassName()); if (optSystemC()) { puts("(sc_module_name /* unused */)\n"); - puts(" : vlSymsp{new " + symClassName() + "(nullptr, name(), this)}\n"); + puts(" : vlSymsp{new " + symClassName() + + "(Verilated::threadContextp(), name(), this)}\n"); } else { puts(+"(VerilatedContext* _vcontextp__, const char* _vcname__)\n"); puts(" : vlSymsp{new " + symClassName() + "(_vcontextp__, _vcname__, this)}\n"); @@ -263,6 +270,8 @@ class EmitCModel final : public EmitCFunc { puts(" , rootp{&(vlSymsp->TOP)}\n"); puts("{\n"); + puts("// Register model with the context\n"); + puts("vlSymsp->_vm_contextp__->addModel(this);\n"); if (optSystemC()) { // Create sensitivity list for when to evaluate the model. @@ -301,7 +310,7 @@ class EmitCModel final : public EmitCFunc { if (!optSystemC()) { puts("\n"); puts(topClassName() + "::" + topClassName() + "(const char* _vcname__)\n"); - puts(" : " + topClassName() + "(nullptr, _vcname__)\n{\n}\n"); + puts(" : " + topClassName() + "(Verilated::threadContextp(), _vcname__)\n{\n}\n"); } } @@ -428,7 +437,7 @@ class EmitCModel final : public EmitCFunc { } if (v3Global.opt.profExec()) { - puts("vlSymsp->__Vm_executionProfiler.configure(*(vlSymsp->_vm_contextp__));\n"); + puts("vlSymsp->__Vm_executionProfilerp->configure();\n"); puts("VL_EXEC_TRACE_ADD_RECORD(vlSymsp).evalBegin();\n"); } @@ -477,6 +486,13 @@ class EmitCModel final : public EmitCFunc { puts("\nVL_ATTR_COLD void " + topClassName() + "::final() {\n"); puts(/**/ topModNameProtected + "__" + protect("_final") + "(&(vlSymsp->TOP));\n"); puts("}\n"); + + putSectionDelimiter("Implementations of abstract methods from VerilatedModel\n"); + puts("const char* " + topClassName() + "::hierName() { return vlSymsp->name(); }\n"); + puts("const char* " + topClassName() + "::modelName() { return \"" + topClassName() + + "\"; }\n"); + puts("unsigned " + topClassName() + "::threads() { return " + + cvtToStr(std::max(1, v3Global.opt.threads())) + "; }\n"); } void emitTraceMethods(AstNodeModule* modp) { @@ -529,7 +545,8 @@ class EmitCModel final : public EmitCFunc { puts(/**/ "}"); } puts(/**/ "if (false && levels && options) {} // Prevent unused\n"); - puts(/**/ "tfp->spTrace()->addInitCb(&" + protect("trace_init") + ", &(vlSymsp->TOP));\n"); + puts(/**/ "tfp->spTrace()->addInitCb(&" + protect("trace_init") + + ", &(vlSymsp->TOP), contextp());\n"); puts(/**/ topModNameProtected + "__" + protect("trace_register") + "(&(vlSymsp->TOP), tfp->spTrace());\n"); diff --git a/src/V3EmitCSyms.cpp b/src/V3EmitCSyms.cpp index c66f346b0..e325aa79e 100644 --- a/src/V3EmitCSyms.cpp +++ b/src/V3EmitCSyms.cpp @@ -445,17 +445,17 @@ void EmitCSyms::emitSymHdr() { } puts("bool __Vm_didInit = false;\n"); - if (v3Global.opt.profExec()) { - puts("\n// EXECUTION PROFILING\n"); - puts("VlExecutionProfiler __Vm_executionProfiler;\n"); - } - if (v3Global.opt.mtasks()) { puts("\n// MULTI-THREADING\n"); puts("VlThreadPool* const __Vm_threadPoolp;\n"); puts("bool __Vm_even_cycle = false;\n"); } + if (v3Global.opt.profExec()) { + puts("\n// EXECUTION PROFILING\n"); + puts("VlExecutionProfiler* const __Vm_executionProfilerp;\n"); + } + puts("\n// MODULE INSTANCE STATE\n"); for (const auto& i : m_scopes) { const AstScope* const scopep = i.first; @@ -673,7 +673,6 @@ void EmitCSyms::emitSymImp() { puts("_vm_pgoProfiler.write(\"" + topClassName() + "\", _vm_contextp__->profVltFilename());\n"); } - if (v3Global.opt.mtasks()) puts("delete __Vm_threadPoolp;\n"); puts("}\n\n"); // Constructor @@ -705,12 +704,13 @@ void EmitCSyms::emitSymImp() { // Note we create N-1 threads in the thread pool. The thread // that calls eval() becomes the final Nth thread for the // duration of the eval call. - puts(" , __Vm_threadPoolp{new VlThreadPool{_vm_contextp__, " - + cvtToStr(v3Global.opt.threads() - 1) + ", " - + (v3Global.opt.profExec() - ? "&__Vm_executionProfiler, &VlExecutionProfiler::startWorkerSetup" - : "nullptr, nullptr") - + "}}\n"); + puts(" , __Vm_threadPoolp{static_cast(contextp->threadPoolp())}\n"); + } + + if (v3Global.opt.profExec()) { + puts(" , " + "__Vm_executionProfilerp{static_cast(contextp->" + "enableExecutionProfiler(&VlExecutionProfiler::construct))}\n"); } puts(" // Setup module instances\n"); diff --git a/src/V3EmitMk.cpp b/src/V3EmitMk.cpp index 3fb3907be..800cf589c 100644 --- a/src/V3EmitMk.cpp +++ b/src/V3EmitMk.cpp @@ -116,7 +116,7 @@ public: putMakeClassEntry(of, v3Global.opt.traceSourceLang() + ".cpp"); } } - if (v3Global.opt.mtasks()) putMakeClassEntry(of, "verilated_threads.cpp"); + if (v3Global.opt.threads()) putMakeClassEntry(of, "verilated_threads.cpp"); if (v3Global.opt.usesProfiler()) { putMakeClassEntry(of, "verilated_profiler.cpp"); } diff --git a/src/V3Trace.cpp b/src/V3Trace.cpp index 9fa1b099a..7113c5e0e 100644 --- a/src/V3Trace.cpp +++ b/src/V3Trace.cpp @@ -512,8 +512,10 @@ private: m_regFuncp->addStmtsp(new AstText(flp, "tracep->addChgCb(", true)); } m_regFuncp->addStmtsp(new AstAddrOfCFunc(flp, funcp)); - const string threadPool{m_parallelism > 1 ? "vlSymsp->__Vm_threadPoolp" : "nullptr"}; - m_regFuncp->addStmtsp(new AstText(flp, ", vlSelf, " + threadPool + ");\n", true)); + m_regFuncp->addStmtsp(new AstText(flp, ", vlSelf", true)); + m_regFuncp->addStmtsp( + new AstText(flp, ", vlSelf->vlSymsp->__Vm_modelp->contextp()", true)); + m_regFuncp->addStmtsp(new AstText(flp, ");\n", true)); } else { // Sub functions funcp->argTypes(v3Global.opt.traceClassBase() + "::Buffer* bufp"); @@ -700,7 +702,8 @@ private: // Register it m_regFuncp->addStmtsp(new AstText(fl, "tracep->addCleanupCb(", true)); m_regFuncp->addStmtsp(new AstAddrOfCFunc(fl, cleanupFuncp)); - m_regFuncp->addStmtsp(new AstText(fl, ", vlSelf);\n", true)); + m_regFuncp->addStmtsp( + new AstText(fl, ", vlSelf, vlSelf->vlSymsp->__Vm_modelp->contextp());\n", true)); // Clear global activity flag cleanupFuncp->addStmtsp( diff --git a/test_regress/driver.pl b/test_regress/driver.pl index cbd9ba9ea..ae0ed4f36 100755 --- a/test_regress/driver.pl +++ b/test_regress/driver.pl @@ -579,6 +579,7 @@ sub new { sc_time_resolution => "SC_PS", # Keep - PS is SystemC default sim_time => 1100, threads => -1, # --threads (negative means auto based on scenario) + context_threads => 0, # Number of threads to allocate in the context benchmark => $opt_benchmark, verbose => $opt_verbose, run_env => '', @@ -974,7 +975,11 @@ sub compile { $self->oprint("Compile\n") if $self->{verbose}; die "%Error: 'threads =>' argument must be <= 1 for vlt scenario" if $param{vlt} && $param{threads} > 1; - $param{threads} = ::calc_threads($Vltmt_threads) if ($param{threads} < 0 && $param{vltmt}); + # Compute automatic parameter values + $param{threads} = ::calc_threads($Vltmt_threads) if $param{threads} < 0 && $param{vltmt}; + $param{context_threads} = $param{threads} >= 1 ? $param{threads} : 1 if !$param{context_threads}; + $self->{threads} = $param{threads}; + $self->{context_threads} = $param{context_threads}; compile_vlt_cmd(%param); @@ -1795,6 +1800,7 @@ sub _make_main { } print $fh " const std::unique_ptr contextp{new VerilatedContext};\n"; + print $fh " contextp->threads($self->{context_threads});\n"; print $fh " contextp->commandArgs(argc, argv);\n"; print $fh " contextp->debug(" . ($self->{verilated_debug} ? 1 : 0) . ");\n"; print $fh " srand48(5);\n"; # Ensure determinism diff --git a/test_regress/t/t_embed1.pl b/test_regress/t/t_embed1.pl index 08e4c042c..2c5b8a918 100755 --- a/test_regress/t/t_embed1.pl +++ b/test_regress/t/t_embed1.pl @@ -22,7 +22,8 @@ mkdir $child_dir; (VM_PREFIX => "$Self->{VM_PREFIX}_child", top_filename => "$Self->{name}_child.v", verilator_flags => ["-cc", "-Mdir", "${child_dir}", "--debug-check"], - threads => $Self->{vltmt} ? $Self->get_default_vltmt_threads() : 0 + # Can't use multi threading (like hier blocks), but needs to be thread safe + threads => $Self->{vltmt} ? 1 : 0, ); run(logfile => "${child_dir}/vlt_compile.log", diff --git a/test_regress/t/t_gantt_two.cpp b/test_regress/t/t_gantt_two.cpp new file mode 100644 index 000000000..da253fab7 --- /dev/null +++ b/test_regress/t/t_gantt_two.cpp @@ -0,0 +1,43 @@ +// +// DESCRIPTION: Verilator: Verilog Multiple Model Test Module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2022 by Geza Lore. +// SPDX-License-Identifier: CC0-1.0 +// + +#include +#include "verilated.h" +#include "Vt_gantt_two.h" + +int main(int argc, char** argv, char** env) { + srand48(5); + + const std::unique_ptr contextp{new VerilatedContext}; +#ifdef VL_THREADED + contextp->threads(2); +#endif + contextp->commandArgs(argc, argv); + contextp->debug(0); + + std::unique_ptr topap{new Vt_gantt_two{contextp.get(), "topa"}}; + std::unique_ptr topbp{new Vt_gantt_two{contextp.get(), "topb"}}; + + topap->clk = false; + topap->eval(); + topbp->clk = false; + topbp->eval(); + + contextp->timeInc(10); + while ((contextp->time() < 1100) && !contextp->gotFinish()) { + topap->clk = !topap->clk; + topap->eval(); + topbp->clk = !topbp->clk; + topbp->eval(); + contextp->timeInc(5); + } + if (!contextp->gotFinish()) { + vl_fatal(__FILE__, __LINE__, "main", "%Error: Timeout; never got a $finish"); + } + return 0; +} diff --git a/test_regress/t/t_gantt_two.pl b/test_regress/t/t_gantt_two.pl new file mode 100755 index 000000000..768f55440 --- /dev/null +++ b/test_regress/t/t_gantt_two.pl @@ -0,0 +1,61 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 by Wilson Snyder. This program is free software; you +# can redistribute it and/or modify it under the terms of either the GNU +# Lesser General Public License Version 3 or the Perl Artistic License +# Version 2.0. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +# Test for bin/verilator_gantt, + +scenarios(vlt_all => 1); + +# It doesn't really matter what test +# we use, so long as it runs several cycles, +# enough for the profiling to happen: +top_filename("t/t_gen_alw.v"); + +compile( + make_top_shell => 0, + make_main => 0, + v_flags2 => ["--prof-exec --exe $Self->{t_dir}/$Self->{name}.cpp"], + # Checks below care about thread count, so use 2 (minimum reasonable) + threads => $Self->{vltmt} ? 2 : 0, + make_flags => 'CPPFLAGS_ADD=-DVL_NO_LEGACY', + ); + +execute( + all_run_flags => ["+verilator+prof+exec+start+4", + " +verilator+prof+exec+window+4", + " +verilator+prof+exec+file+$Self->{obj_dir}/profile_exec.dat", + " +verilator+prof+vlt+file+$Self->{obj_dir}/profile.vlt", + ], + check_finished => 1, + ); + +# For now, verilator_gantt still reads from STDIN +# (probably it should take a file, gantt.dat like verilator_profcfunc) +# The profiling data still goes direct to the runtime's STDOUT +# (maybe that should go to a separate file - gantt.dat?) +run(cmd => ["$ENV{VERILATOR_ROOT}/bin/verilator_gantt", + "$Self->{obj_dir}/profile_exec.dat", + "--vcd $Self->{obj_dir}/profile_exec.vcd", + "| tee $Self->{obj_dir}/gantt.log"], + ); + +if ($Self->{vltmt}) { + file_grep("$Self->{obj_dir}/gantt.log", qr/Total threads += 2/i); + file_grep("$Self->{obj_dir}/gantt.log", qr/Total mtasks += 7/i); +} else { + file_grep("$Self->{obj_dir}/gantt.log", qr/Total threads += 1/i); + file_grep("$Self->{obj_dir}/gantt.log", qr/Total mtasks += 0/i); +} +file_grep("$Self->{obj_dir}/gantt.log", qr/Total evals += 4/i); + +# Diff to itself, just to check parsing +vcd_identical("$Self->{obj_dir}/profile_exec.vcd", "$Self->{obj_dir}/profile_exec.vcd"); + +ok(1); +1; diff --git a/test_regress/t/t_hier_block_cmake/main.cpp b/test_regress/t/t_hier_block_cmake/main.cpp index e49101162..58bc8d5ad 100644 --- a/test_regress/t/t_hier_block_cmake/main.cpp +++ b/test_regress/t/t_hier_block_cmake/main.cpp @@ -14,8 +14,11 @@ int main(int argc, char *argv[]) { const std::unique_ptr contextp{new VerilatedContext}; - std::unique_ptr top{new Vt_hier_block{contextp.get(), "top"}}; +#if VL_THREADED + contextp->threads(6); +#endif contextp->commandArgs(argc, argv); + std::unique_ptr top{new Vt_hier_block{contextp.get(), "top"}}; for (int i = 0; i < 100 && !contextp->gotFinish(); ++i) { top->eval(); top->clk ^= 1; diff --git a/test_regress/t/t_lib_prot_shared.pl b/test_regress/t/t_lib_prot_shared.pl index 1a3f8af5f..cc0c2f977 100755 --- a/test_regress/t/t_lib_prot_shared.pl +++ b/test_regress/t/t_lib_prot_shared.pl @@ -59,7 +59,8 @@ while (1) { "-LDFLAGS", "'-Wl,-rpath,$abs_secret_dir -L$abs_secret_dir -l$secret_prefix'"], xsim_flags2 => ["$secret_dir/secret.sv"], - threads => $Self->{vltmt} ? 1 : 0 + threads => $Self->{vltmt} ? 1 : 0, + context_threads => $Self->{vltmt} ? 6 : 1 ); execute( diff --git a/test_regress/t/t_threads_crazy.pl b/test_regress/t/t_threads_crazy.pl index 6bb21acb0..c72858f2c 100755 --- a/test_regress/t/t_threads_crazy.pl +++ b/test_regress/t/t_threads_crazy.pl @@ -10,20 +10,16 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di scenarios(vltmt => 1); -if ($Self->cfg_with_m32) { - skip("Does not work with -m32 (resource unavailable)"); -} - compile( verilator_flags2 => ['--cc'], - threads => 1024 + threads => 4, + context_threads => 2 ); execute( - check_finished => 1, + fails => 1 ); -file_grep($Self->{run_log_filename}, qr/System has .* CPUs but.*--threads 1024/); - +file_grep($Self->{run_log_filename}, qr/%Error: .*\/verilated\.cpp:\d+: VerilatedContext has 2 threads but model 'Vt_threads_crazy' \(instantiated as 'top'\) was Verilated with --threads 4\./); ok(1); 1; diff --git a/test_regress/t/t_threads_crazy_context.pl b/test_regress/t/t_threads_crazy_context.pl new file mode 100755 index 000000000..8e28bb87a --- /dev/null +++ b/test_regress/t/t_threads_crazy_context.pl @@ -0,0 +1,36 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003-2009 by Wilson Snyder. This program is free software; you +# can redistribute it and/or modify it under the terms of either the GNU +# Lesser General Public License Version 3 or the Perl Artistic License +# Version 2.0. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(vlt_all => 1); + +if ($Self->cfg_with_m32) { + skip("Does not work with -m32 (resource unavailable)"); +} + +top_filename("t/t_threads_crazy.v"); + +compile( + verilator_flags2 => ['--cc'], + threads => $Self->{vltmt} ? 2 : 0, + context_threads => 1024 + ); + +execute( + check_finished => 1, + ); + +if ($Self->{vltmt}) { + file_grep($Self->{run_log_filename}, qr/System has \d+ hardware threads but simulation thread count set to 1024\. This will likely cause significant slowdown\./); +} else { + file_grep($Self->{run_log_filename}, qr/Verilator run-time library built without VL_THREADS\. Ignoring call to 'VerilatedContext::threads' with argument 1024\./); +} + +ok(1); +1; diff --git a/test_regress/t/t_wrapper_context.cpp b/test_regress/t/t_wrapper_context.cpp index 87332eb8c..31a9334f0 100644 --- a/test_regress/t/t_wrapper_context.cpp +++ b/test_regress/t/t_wrapper_context.cpp @@ -92,6 +92,8 @@ int main(int argc, char** argv, char** env) { std::unique_ptr context1p{new VerilatedContext}; // configuration + context0p->threads(1); + context1p->threads(1); context0p->fatalOnError(false); context1p->fatalOnError(false); context0p->traceEverOn(true); From b61d819fcb03388a51ed589320e39adddc3b454d Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Tue, 12 Jul 2022 15:38:24 +0100 Subject: [PATCH 11/12] Move contextp() under VerilatedModel --- include/verilated.cpp | 6 ++++++ include/verilated.h | 8 +++++++- src/V3EmitCModel.cpp | 16 +++++----------- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/include/verilated.cpp b/include/verilated.cpp index cf1d76d8f..d5821b8f7 100644 --- a/include/verilated.cpp +++ b/include/verilated.cpp @@ -2908,6 +2908,12 @@ void VerilatedImp::versionDump() VL_MT_SAFE { VL_PRINTF_MT(" Version: %s %s\n", Verilated::productName(), Verilated::productVersion()); } +//=========================================================================== +// VerilatedModel:: Methods + +VerilatedModel::VerilatedModel(VerilatedContext& context) + : m_context{context} {} + //=========================================================================== // VerilatedModule:: Methods diff --git a/include/verilated.h b/include/verilated.h index bc1d5a3f2..3a4fab003 100644 --- a/include/verilated.h +++ b/include/verilated.h @@ -81,6 +81,7 @@ #endif // clang-format on +class VerilatedContext; class VerilatedContextImp; class VerilatedContextImpData; class VerilatedCovContext; @@ -261,11 +262,16 @@ public: class VerilatedModel VL_NOT_FINAL { VL_UNCOPYABLE(VerilatedModel); + VerilatedContext& m_context; // The VerilatedContext this model is instantiated under + protected: - explicit VerilatedModel() = default; + explicit VerilatedModel(VerilatedContext& context); virtual ~VerilatedModel() = default; public: + /// Returns the VerilatedContext this model is instantiated under + /// Used to get to e.g. simulation time via contextp()->time() + inline VerilatedContext* contextp() const { return &m_context; } /// Returns the hierarchical name of this module instance. virtual const char* hierName() = 0; /// Returns the name of this model (the name of the generated model class). diff --git a/src/V3EmitCModel.cpp b/src/V3EmitCModel.cpp index 203582609..9522b214f 100644 --- a/src/V3EmitCModel.cpp +++ b/src/V3EmitCModel.cpp @@ -195,9 +195,6 @@ class EmitCModel final : public EmitCFunc { } } - puts("/// Return current simulation context for this model.\n"); - puts("/// Used to get to e.g. simulation time via contextp()->time()\n"); - puts("VerilatedContext* contextp() const;\n"); if (!optSystemC()) { puts("/// Retrieve name of this model instance (as passed to constructor).\n"); puts("const char* name() const;\n"); @@ -241,11 +238,12 @@ class EmitCModel final : public EmitCFunc { puts(topClassName() + "::" + topClassName()); if (optSystemC()) { puts("(sc_module_name /* unused */)\n"); - puts(" : vlSymsp{new " + symClassName() - + "(Verilated::threadContextp(), name(), this)}\n"); + puts(" : VerilatedModel{*Verilated::threadContextp()}\n"); + puts(" , vlSymsp{new " + symClassName() + "(contextp(), name(), this)}\n"); } else { puts(+"(VerilatedContext* _vcontextp__, const char* _vcname__)\n"); - puts(" : vlSymsp{new " + symClassName() + "(_vcontextp__, _vcname__, this)}\n"); + puts(" : VerilatedModel{*_vcontextp__}\n"); + puts(" , vlSymsp{new " + symClassName() + "(contextp(), _vcname__, this)}\n"); } // Set up IO references @@ -271,7 +269,7 @@ class EmitCModel final : public EmitCFunc { puts("{\n"); puts("// Register model with the context\n"); - puts("vlSymsp->_vm_contextp__->addModel(this);\n"); + puts("contextp()->addModel(this);\n"); if (optSystemC()) { // Create sensitivity list for when to evaluate the model. @@ -469,10 +467,6 @@ class EmitCModel final : public EmitCFunc { } putSectionDelimiter("Utilities"); - // ::contextp - puts("\nVerilatedContext* " + topClassName() + "::contextp() const {\n"); - puts(/**/ "return vlSymsp->_vm_contextp__;\n"); - puts("}\n"); if (!optSystemC()) { // ::name From 79c901c220d931a27826b7a19382c0790d2241f5 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Tue, 12 Jul 2022 15:50:11 +0100 Subject: [PATCH 12/12] Tighten signatures/implementaion of VerilatedModel abstract methods. --- include/verilated.h | 6 +++--- src/V3EmitCModel.cpp | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/include/verilated.h b/include/verilated.h index 3a4fab003..ebb1990e8 100644 --- a/include/verilated.h +++ b/include/verilated.h @@ -273,11 +273,11 @@ public: /// Used to get to e.g. simulation time via contextp()->time() inline VerilatedContext* contextp() const { return &m_context; } /// Returns the hierarchical name of this module instance. - virtual const char* hierName() = 0; + virtual const char* hierName() const = 0; /// Returns the name of this model (the name of the generated model class). - virtual const char* modelName() = 0; + virtual const char* modelName() const = 0; /// Returns the thread level parallelism, this model was Verilated with. Always 1 or higher. - virtual unsigned threads() = 0; + virtual unsigned threads() const = 0; }; //========================================================================= diff --git a/src/V3EmitCModel.cpp b/src/V3EmitCModel.cpp index 9522b214f..04b422a97 100644 --- a/src/V3EmitCModel.cpp +++ b/src/V3EmitCModel.cpp @@ -220,9 +220,9 @@ class EmitCModel final : public EmitCFunc { } puts("\n// Abstract methods from VerilatedModel\n"); - puts("const char* hierName() override;\n"); - puts("const char* modelName() override;\n"); - puts("unsigned threads() override;\n"); + puts("const char* hierName() const override final;\n"); + puts("const char* modelName() const override final;\n"); + puts("unsigned threads() const override final;\n"); puts("} VL_ATTR_ALIGNED(VL_CACHE_LINE_BYTES);\n"); @@ -482,10 +482,10 @@ class EmitCModel final : public EmitCFunc { puts("}\n"); putSectionDelimiter("Implementations of abstract methods from VerilatedModel\n"); - puts("const char* " + topClassName() + "::hierName() { return vlSymsp->name(); }\n"); - puts("const char* " + topClassName() + "::modelName() { return \"" + topClassName() + puts("const char* " + topClassName() + "::hierName() const { return vlSymsp->name(); }\n"); + puts("const char* " + topClassName() + "::modelName() const { return \"" + topClassName() + "\"; }\n"); - puts("unsigned " + topClassName() + "::threads() { return " + puts("unsigned " + topClassName() + "::threads() const { return " + cvtToStr(std::max(1, v3Global.opt.threads())) + "; }\n"); }