From db6ecbd57e2b038d66644fc99479d4036a8872ad Mon Sep 17 00:00:00 2001 From: Todd Strader Date: Tue, 18 Feb 2020 17:52:28 -0500 Subject: [PATCH 01/16] Test for #2169 --- test_regress/t/t_prot_lib.v | 3 ++ test_regress/t/t_prot_lib_clk_gated.pl | 74 ++++++++++++++++++++++++++ test_regress/t/t_prot_lib_secret.v | 7 ++- 3 files changed, 82 insertions(+), 2 deletions(-) create mode 100755 test_regress/t/t_prot_lib_clk_gated.pl diff --git a/test_regress/t/t_prot_lib.v b/test_regress/t/t_prot_lib.v index d511fe16f..ce80add05 100644 --- a/test_regress/t/t_prot_lib.v +++ b/test_regress/t/t_prot_lib.v @@ -55,6 +55,8 @@ module t (/*AUTOARG*/ logic [3:0] [31:0] s4x32_in; logic [3:0] [31:0] s4x32_out; + wire clk_en = crc[0]; + secret secret ( .accum_in, @@ -77,6 +79,7 @@ module t (/*AUTOARG*/ .s129_out, .s4x32_in, .s4x32_out, + .clk_en, .clk); always @(posedge clk) begin diff --git a/test_regress/t/t_prot_lib_clk_gated.pl b/test_regress/t/t_prot_lib_clk_gated.pl new file mode 100755 index 000000000..9862323a7 --- /dev/null +++ b/test_regress/t/t_prot_lib_clk_gated.pl @@ -0,0 +1,74 @@ +#!/usr/bin/perl +# Makes the test run with tracing enabled by default, can be overridden +# with --notrace +unshift(@ARGV, "--trace"); +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2019 by Todd Strader. 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. + +scenarios( + vlt => 1, + xsim => 1, + ); + +$Self->{sim_time} = $Self->{benchmark} * 100 if $Self->{benchmark}; + +top_filename("t/t_prot_lib.v"); +my $secret_prefix = "secret"; +my $secret_dir = "$Self->{obj_dir}/$secret_prefix"; +mkdir $secret_dir; + +while (1) { + # Always compile the secret file with Verilator no matter what simulator + # we are testing with + run(logfile => "$secret_dir/vlt_compile.log", + cmd => ["perl", + "$ENV{VERILATOR_ROOT}/bin/verilator", + "--prefix", + "Vt_prot_lib_secret", + "-cc", + "-Mdir", + $secret_dir, + "-GGATED_CLK=1", + "--protect-lib", + $secret_prefix, + "t/t_prot_lib_secret.v"]); + last if $Self->{errors}; + + run(logfile => "$secret_dir/secret_gcc.log", + cmd=>["make", + "-C", + $secret_dir, + "-f", + "Vt_prot_lib_secret.mk"]); + last if $Self->{errors}; + + compile( + verilator_flags2 => ["$secret_dir/secret.sv", + "-LDFLAGS", + "'-L$secret_prefix -lsecret -static'"], + xsim_flags2 => ["$secret_dir/secret.sv"], + ); + + execute( + check_finished => 1, + xsim_run_flags2 => ["--sv_lib", + "$secret_dir/libsecret", + "--dpi_absolute"], + ); + + if ($Self->{vlt} && $Self->{trace}) { + # We can see the ports of the secret module + file_grep("$Self->{obj_dir}/simx.vcd", qr/accum_in/); + # but we can't see what's inside + file_grep_not("$Self->{obj_dir}/simx.vcd", qr/secret_/); + } + + ok(1); + last; +} +1; diff --git a/test_regress/t/t_prot_lib_secret.v b/test_regress/t/t_prot_lib_secret.v index b55164988..534aca6a1 100644 --- a/test_regress/t/t_prot_lib_secret.v +++ b/test_regress/t/t_prot_lib_secret.v @@ -2,7 +2,8 @@ // This file ONLY is placed into the Public Domain, for any use, // without warranty, 2019 by Todd Strader. -module secret ( +module secret #(parameter GATED_CLK = 0) + ( input [31:0] accum_in, output wire [31:0] accum_out, input accum_bypass, @@ -23,6 +24,7 @@ module secret ( output logic [128:0] s129_out, input [3:0] [31:0] s4x32_in, output logic [3:0] [31:0] s4x32_out, + input clk_en, input clk); logic [31:0] secret_accum_q = 0; @@ -30,7 +32,8 @@ module secret ( initial $display("created %m"); - always @(posedge clk) begin + wire the_clk = GATED_CLK != 0 ? clk & clk_en : clk; + always @(posedge the_clk) begin secret_accum_q <= secret_accum_q + accum_in + secret_value; end From 120f62fe854d3387e6bac702b8097e5c172c94a7 Mon Sep 17 00:00:00 2001 From: Todd Strader Date: Tue, 18 Feb 2020 18:19:55 -0500 Subject: [PATCH 02/16] Fix is probably to mark as a clock --- src/V3ProtectLib.cpp | 3 ++- test_regress/t/t_prot_lib_secret.v | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/V3ProtectLib.cpp b/src/V3ProtectLib.cpp index b44edf500..6e64d7563 100644 --- a/src/V3ProtectLib.cpp +++ b/src/V3ProtectLib.cpp @@ -353,7 +353,8 @@ class ProtectVisitor : public AstNVisitor { nodep->v3error("Unsupported: unpacked arrays with protect-lib on "<prettyNameQ()); } if (nodep->direction() == VDirection::INPUT) { - if (nodep->isUsedClock()) { + if (nodep->isUsedClock() + || nodep->attrClocker() == VVarAttrClocker::CLOCKER_YES) { handleClock(nodep); } else { handleDataInput(nodep); diff --git a/test_regress/t/t_prot_lib_secret.v b/test_regress/t/t_prot_lib_secret.v index 534aca6a1..05ca43e1b 100644 --- a/test_regress/t/t_prot_lib_secret.v +++ b/test_regress/t/t_prot_lib_secret.v @@ -25,7 +25,7 @@ module secret #(parameter GATED_CLK = 0) input [3:0] [31:0] s4x32_in, output logic [3:0] [31:0] s4x32_out, input clk_en, - input clk); + input clk /*verilator clocker*/); logic [31:0] secret_accum_q = 0; logic [31:0] secret_value = 7; From 4b4f10f5e67eda57de79b521fa8757cd750ee179 Mon Sep 17 00:00:00 2001 From: Todd Strader Date: Fri, 21 Feb 2020 05:47:00 -0500 Subject: [PATCH 03/16] Follow other clock gating examples --- test_regress/t/t_prot_lib.v | 20 +++++++++++++++++--- test_regress/t/t_prot_lib_clk_gated.pl | 1 + test_regress/t/t_prot_lib_secret.v | 14 +++++++++++++- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/test_regress/t/t_prot_lib.v b/test_regress/t/t_prot_lib.v index ce80add05..b8bb19ff2 100644 --- a/test_regress/t/t_prot_lib.v +++ b/test_regress/t/t_prot_lib.v @@ -14,7 +14,7 @@ if (cyc > 0 && sig``_in != sig``_out) begin \ $stop; \ end -module t (/*AUTOARG*/ +module t #(parameter GATED_CLK = 0) (/*AUTOARG*/ // Inputs clk ); @@ -90,8 +90,6 @@ module t (/*AUTOARG*/ cyc <= cyc + 1; crc <= {crc[62:0], crc[63]^crc[2]^crc[0]}; accum_in <= accum_in + 5; - // 7 is the secret_value inside the secret module - accum_out_expect <= accum_in + accum_out_expect + 7; `DRIVE(s1) `DRIVE(s2) `DRIVE(s8) @@ -125,6 +123,22 @@ module t (/*AUTOARG*/ end end + logic possibly_gated_clk; + if (GATED_CLK != 0) begin: yes_gated_clock + logic clk_en_latch /*verilator clock_enable*/; + /* verilator lint_off COMBDLY */ + always_comb if (clk == '0) clk_en_latch <= clk_en; + /* verilator lint_on COMBDLY */ + assign possibly_gated_clk = clk & clk_en_latch; + end else begin: no_gated_clock + assign possibly_gated_clk = clk; + end + + always @(posedge possibly_gated_clk) begin + // 7 is the secret_value inside the secret module + accum_out_expect <= accum_in + accum_out_expect + 7; + end + always @(*) begin // XSim (and maybe all event simulators?) sees the moment where // s1_in has not yet propagated to s1_out, however, they do always diff --git a/test_regress/t/t_prot_lib_clk_gated.pl b/test_regress/t/t_prot_lib_clk_gated.pl index 9862323a7..394bc08b9 100755 --- a/test_regress/t/t_prot_lib_clk_gated.pl +++ b/test_regress/t/t_prot_lib_clk_gated.pl @@ -49,6 +49,7 @@ while (1) { compile( verilator_flags2 => ["$secret_dir/secret.sv", + "-GGATED_CLK=1", "-LDFLAGS", "'-L$secret_prefix -lsecret -static'"], xsim_flags2 => ["$secret_dir/secret.sv"], diff --git a/test_regress/t/t_prot_lib_secret.v b/test_regress/t/t_prot_lib_secret.v index 05ca43e1b..85c3d5196 100644 --- a/test_regress/t/t_prot_lib_secret.v +++ b/test_regress/t/t_prot_lib_secret.v @@ -32,7 +32,19 @@ module secret #(parameter GATED_CLK = 0) initial $display("created %m"); - wire the_clk = GATED_CLK != 0 ? clk & clk_en : clk; + logic the_clk; + generate + if (GATED_CLK != 0) begin: yes_gated_clock + logic clk_en_latch /*verilator clock_enable*/; + /* verilator lint_off COMBDLY */ + always_comb if (clk == '0) clk_en_latch <= clk_en; + /* verilator lint_on COMBDLY */ + assign the_clk = clk & clk_en_latch; + end else begin: no_gated_clock + assign the_clk = clk; + end + endgenerate + always @(posedge the_clk) begin secret_accum_q <= secret_accum_q + accum_in + secret_value; end From f7d1c6ca72f0bc678e6f5590cfc31a07b6a0d733 Mon Sep 17 00:00:00 2001 From: Todd Strader Date: Fri, 21 Feb 2020 05:50:09 -0500 Subject: [PATCH 04/16] emacs verilog-batch-indent --- test_regress/t/t_prot_lib.v | 24 +++++------ test_regress/t/t_prot_lib_secret.v | 64 +++++++++++++++--------------- 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/test_regress/t/t_prot_lib.v b/test_regress/t/t_prot_lib.v index b8bb19ff2..6988bc5ba 100644 --- a/test_regress/t/t_prot_lib.v +++ b/test_regress/t/t_prot_lib.v @@ -15,16 +15,16 @@ if (cyc > 0 && sig``_in != sig``_out) begin \ end module t #(parameter GATED_CLK = 0) (/*AUTOARG*/ - // Inputs - clk - ); + // Inputs + clk + ); input clk; localparam last_cyc = `ifdef TEST_BENCHMARK - `TEST_BENCHMARK; + `TEST_BENCHMARK; `else - 10; + 10; `endif genvar x; @@ -55,7 +55,7 @@ module t #(parameter GATED_CLK = 0) (/*AUTOARG*/ logic [3:0] [31:0] s4x32_in; logic [3:0] [31:0] s4x32_out; - wire clk_en = crc[0]; + wire clk_en = crc[0]; secret secret ( @@ -125,13 +125,13 @@ module t #(parameter GATED_CLK = 0) (/*AUTOARG*/ logic possibly_gated_clk; if (GATED_CLK != 0) begin: yes_gated_clock - logic clk_en_latch /*verilator clock_enable*/; - /* verilator lint_off COMBDLY */ - always_comb if (clk == '0) clk_en_latch <= clk_en; - /* verilator lint_on COMBDLY */ - assign possibly_gated_clk = clk & clk_en_latch; + logic clk_en_latch /*verilator clock_enable*/; + /* verilator lint_off COMBDLY */ + always_comb if (clk == '0) clk_en_latch <= clk_en; + /* verilator lint_on COMBDLY */ + assign possibly_gated_clk = clk & clk_en_latch; end else begin: no_gated_clock - assign possibly_gated_clk = clk; + assign possibly_gated_clk = clk; end always @(posedge possibly_gated_clk) begin diff --git a/test_regress/t/t_prot_lib_secret.v b/test_regress/t/t_prot_lib_secret.v index 85c3d5196..6f0abc1b4 100644 --- a/test_regress/t/t_prot_lib_secret.v +++ b/test_regress/t/t_prot_lib_secret.v @@ -3,45 +3,45 @@ // without warranty, 2019 by Todd Strader. module secret #(parameter GATED_CLK = 0) - ( - input [31:0] accum_in, - output wire [31:0] accum_out, - input accum_bypass, - output [31:0] accum_bypass_out, - input s1_in, - output logic s1_out, - input [1:0] s2_in, - output logic [1:0] s2_out, - input [7:0] s8_in, - output logic [7:0] s8_out, - input [32:0] s33_in, - output logic [32:0] s33_out, - input [63:0] s64_in, - output logic [63:0] s64_out, - input [64:0] s65_in, - output logic [64:0] s65_out, - input [128:0] s129_in, - output logic [128:0] s129_out, - input [3:0] [31:0] s4x32_in, - output logic [3:0] [31:0] s4x32_out, - input clk_en, - input clk /*verilator clocker*/); + ( + input [31:0] accum_in, + output wire [31:0] accum_out, + input accum_bypass, + output [31:0] accum_bypass_out, + input s1_in, + output logic s1_out, + input [1:0] s2_in, + output logic [1:0] s2_out, + input [7:0] s8_in, + output logic [7:0] s8_out, + input [32:0] s33_in, + output logic [32:0] s33_out, + input [63:0] s64_in, + output logic [63:0] s64_out, + input [64:0] s65_in, + output logic [64:0] s65_out, + input [128:0] s129_in, + output logic [128:0] s129_out, + input [3:0] [31:0] s4x32_in, + output logic [3:0] [31:0] s4x32_out, + input clk_en, + input clk /*verilator clocker*/); - logic [31:0] secret_accum_q = 0; - logic [31:0] secret_value = 7; + logic [31:0] secret_accum_q = 0; + logic [31:0] secret_value = 7; initial $display("created %m"); - logic the_clk; + logic the_clk; generate if (GATED_CLK != 0) begin: yes_gated_clock - logic clk_en_latch /*verilator clock_enable*/; - /* verilator lint_off COMBDLY */ - always_comb if (clk == '0) clk_en_latch <= clk_en; - /* verilator lint_on COMBDLY */ - assign the_clk = clk & clk_en_latch; + logic clk_en_latch /*verilator clock_enable*/; + /* verilator lint_off COMBDLY */ + always_comb if (clk == '0) clk_en_latch <= clk_en; + /* verilator lint_on COMBDLY */ + assign the_clk = clk & clk_en_latch; end else begin: no_gated_clock - assign the_clk = clk; + assign the_clk = clk; end endgenerate From 60f82961b494fbde8c80be7e19ffa563db2de008 Mon Sep 17 00:00:00 2001 From: Todd Strader Date: Mon, 24 Feb 2020 05:34:10 -0500 Subject: [PATCH 05/16] De-tabify --- test_regress/t/t_prot_lib.v | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test_regress/t/t_prot_lib.v b/test_regress/t/t_prot_lib.v index 6988bc5ba..346c54e82 100644 --- a/test_regress/t/t_prot_lib.v +++ b/test_regress/t/t_prot_lib.v @@ -15,14 +15,14 @@ if (cyc > 0 && sig``_in != sig``_out) begin \ end module t #(parameter GATED_CLK = 0) (/*AUTOARG*/ - // Inputs - clk - ); + // Inputs + clk + ); input clk; localparam last_cyc = `ifdef TEST_BENCHMARK - `TEST_BENCHMARK; + `TEST_BENCHMARK; `else 10; `endif From 8319ea6c7327b1f3382ed31daa6b69635d1c37e7 Mon Sep 17 00:00:00 2001 From: Todd Strader Date: Mon, 24 Feb 2020 05:36:09 -0500 Subject: [PATCH 06/16] Changes --- Changes | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Changes b/Changes index b54ee2cb8..1b8aedeb5 100644 --- a/Changes +++ b/Changes @@ -54,6 +54,8 @@ The contributors that suggested a given feature are shown in []. Thanks! **** Fix OpenSolaris issues, #2154. [brancoliticus] +**** Fix gated clocks under --protect-lib, #2169. [Todd Strader] + * Verilator 4.026 2020-01-11 From 23eb96579cf0e6ba27bcbef91bcb738bd0da27ad Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Mon, 24 Feb 2020 18:11:41 -0500 Subject: [PATCH 07/16] Fix duplicate __STDC_FORMAT_MACROS definition. --- include/verilatedos.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/include/verilatedos.h b/include/verilatedos.h index 1db5ad3e1..cc3bdd3b3 100644 --- a/include/verilatedos.h +++ b/include/verilatedos.h @@ -223,9 +223,12 @@ #ifdef __MINGW32__ # define __USE_MINGW_ANSI_STDIO 1 // Force old MinGW (GCC 5 and older) to use C99 formats -# define __STDC_FORMAT_MACROS 1 // Otherwise MinGW doesn't get PRId64 for fstapi.c #endif +// The inttypes supplied with some GCC & MINGW32 versions requires STDC_FORMAT_MACROS +// to be declared in order to get the PRIxx macros used by fstapi.c +#define __STDC_FORMAT_MACROS + #if defined(__CYGWIN__) # include @@ -273,10 +276,6 @@ typedef signed __int32 ssize_t; ///< signed size_t; returned fro #else // Linux or compliant Unix flavors, -m64 -// The inttypes supplied with some GCC versions requires STDC_FORMAT_MACROS -// to be declared in order to get the PRIxx macros used by fstapi.c -#define __STDC_FORMAT_MACROS - # include // Solaris # include // Linux and most flavors # include // __WORDSIZE From c9b74847d10fae5753e4e70dc98c815571aed12d Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Mon, 24 Feb 2020 18:11:56 -0500 Subject: [PATCH 08/16] Docs: Tighter margins, save 10 pages. --- src/pod2latexfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pod2latexfix b/src/pod2latexfix index ce6ecaab5..149e311de 100755 --- a/src/pod2latexfix +++ b/src/pod2latexfix @@ -20,7 +20,7 @@ my $Opt_DistTitle = $ARGV[0] or die "%Error: No disttitle specified,"; my $Opt_DistDate = $ARGV[1] or die "%Error: No distdate specified,"; my $header = - ("\\usepackage[left=1.7in,right=1.7in,top=1.3in,bottom=1.3in]{geometry}\n" + ("\\usepackage[left=1.0in,right=1.0in,top=1.0in,bottom=1.0in]{geometry}\n" ."\\usepackage[pdftex,bookmarks=true,bookmarksnumbered=true,hypertexnames=false,breaklinks=true,colorlinks=true,linkcolor=blue]{hyperref}\n" ."\\usepackage{fancyhdr} \\pagestyle{fancy}\n" ."\\usepackage{graphicx}\n" From 93ac79981ba85994dd8d14b7e7ea10485bcbe49e Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Mon, 24 Feb 2020 18:51:44 -0500 Subject: [PATCH 09/16] Tests: Rename t_var_dotted. No functional change. --- test_regress/t/t_var_dotted.v | 173 ------------------ test_regress/t/t_var_dotted1.v | 173 ++++++++++++++++++ ...r_dotted_inl0.pl => t_var_dotted1_inl0.pl} | 2 +- ...r_dotted_inl1.pl => t_var_dotted1_inl1.pl} | 2 +- ...r_dotted_inl2.pl => t_var_dotted1_inl2.pl} | 2 +- 5 files changed, 176 insertions(+), 176 deletions(-) delete mode 100644 test_regress/t/t_var_dotted.v create mode 100644 test_regress/t/t_var_dotted1.v rename test_regress/t/{t_var_dotted_inl0.pl => t_var_dotted1_inl0.pl} (93%) rename test_regress/t/{t_var_dotted_inl1.pl => t_var_dotted1_inl1.pl} (93%) rename test_regress/t/{t_var_dotted_inl2.pl => t_var_dotted1_inl2.pl} (93%) diff --git a/test_regress/t/t_var_dotted.v b/test_regress/t/t_var_dotted.v deleted file mode 100644 index e693d26c8..000000000 --- a/test_regress/t/t_var_dotted.v +++ /dev/null @@ -1,173 +0,0 @@ -// DESCRIPTION: Verilator: Verilog Test module -// -// This file ONLY is placed into the Public Domain, for any use, -// without warranty, 2006 by Wilson Snyder. - -module t (/*AUTOARG*/ - // Inputs - clk - ); - - // verilator lint_off MULTIDRIVEN - - wire [31:0] outb0c0; - wire [31:0] outb0c1; - wire [31:0] outb1c0; - wire [31:0] outb1c1; - - reg [7:0] lclmem [7:0]; - - ma ma0 (.outb0c0(outb0c0), .outb0c1(outb0c1), - .outb1c0(outb1c0), .outb1c1(outb1c1) - ); - - global_mod #(32'hf00d) global_cell (); - global_mod #(32'hf22d) global_cell2 (); - - input clk; - integer cyc=1; - always @ (posedge clk) begin - cyc <= cyc + 1; -`ifdef TEST_VERBOSE - $write("[%0t] cyc%0d: %0x %0x %0x %0x\n", $time, cyc, outb0c0, outb0c1, outb1c0, outb1c1); -`endif - if (cyc==2) begin - if (global_cell.globali != 32'hf00d) $stop; - if (global_cell2.globali != 32'hf22d) $stop; - if (outb0c0 != 32'h00) $stop; - if (outb0c1 != 32'h01) $stop; - if (outb1c0 != 32'h10) $stop; - if (outb1c1 != 32'h11) $stop; - end - if (cyc==3) begin - // Can we scope down and read and write vars? - ma0.mb0.mc0.out <= ma0.mb0.mc0.out + 32'h100; - ma0.mb0.mc1.out <= ma0.mb0.mc1.out + 32'h100; - ma0.mb1.mc0.out <= ma0.mb1.mc0.out + 32'h100; - ma0.mb1.mc1.out <= ma0.mb1.mc1.out + 32'h100; - end - if (cyc==4) begin - // Can we do dotted's inside array sels? - ma0.rmtmem[ma0.mb0.mc0.out[2:0]] = 8'h12; - lclmem[ma0.mb0.mc0.out[2:0]] = 8'h24; - if (outb0c0 != 32'h100) $stop; - if (outb0c1 != 32'h101) $stop; - if (outb1c0 != 32'h110) $stop; - if (outb1c1 != 32'h111) $stop; - end - if (cyc==5) begin - if (ma0.rmtmem[ma0.mb0.mc0.out[2:0]] != 8'h12) $stop; - if (lclmem[ma0.mb0.mc0.out[2:0]] != 8'h24) $stop; - if (outb0c0 != 32'h1100) $stop; - if (outb0c1 != 32'h2101) $stop; - if (outb1c0 != 32'h2110) $stop; - if (outb1c1 != 32'h3111) $stop; - end - if (cyc==6) begin - if (outb0c0 != 32'h31100) $stop; - if (outb0c1 != 32'h02101) $stop; - if (outb1c0 != 32'h42110) $stop; - if (outb1c1 != 32'h03111) $stop; - end - if (cyc==9) begin - $write("*-* All Finished *-*\n"); - $finish; - end - end - -endmodule - -`ifdef USE_INLINE_MID - `define INLINE_MODULE /*verilator inline_module*/ - `define INLINE_MID_MODULE /*verilator no_inline_module*/ -`else - `ifdef USE_INLINE - `define INLINE_MODULE /*verilator inline_module*/ - `define INLINE_MID_MODULE /*verilator inline_module*/ - `else - `define INLINE_MODULE /*verilator public_module*/ - `define INLINE_MID_MODULE /*verilator public_module*/ - `endif -`endif - -module global_mod; - `INLINE_MODULE - parameter INITVAL = 0; - integer globali; - initial globali = INITVAL; -endmodule - -module ma ( - output wire [31:0] outb0c0, - output wire [31:0] outb0c1, - output wire [31:0] outb1c0, - output wire [31:0] outb1c1 - ); - `INLINE_MODULE - - reg [7:0] rmtmem [7:0]; - - mb #(0) mb0 (.outc0(outb0c0), .outc1(outb0c1)); - mb #(1) mb1 (.outc0(outb1c0), .outc1(outb1c1)); -endmodule - -module mb ( - output wire [31:0] outc0, - output wire [31:0] outc1 - ); - `INLINE_MID_MODULE - parameter P2 = 0; - mc #(P2,0) mc0 (.out(outc0)); - mc #(P2,1) mc1 (.out(outc1)); - global_mod #(32'hf33d) global_cell2 (); - - wire reach_up_clk = t.clk; - always @(reach_up_clk) begin - if (P2==0) begin // Only for mb0 - if (outc0 !== t.ma0.mb0.mc0.out) $stop; // Top module name and lower instances - if (outc0 !== ma0.mb0.mc0.out) $stop; // Upper module name and lower instances - if (outc0 !== ma .mb0.mc0.out) $stop; // Upper module name and lower instances - if (outc0 !== mb.mc0.out) $stop; // This module name and lower instances - if (outc0 !== mb0.mc0.out) $stop; // Upper instance name and lower instances - if (outc0 !== mc0.out) $stop; // Lower instances - - if (outc1 !== t.ma0.mb0.mc1.out) $stop; // Top module name and lower instances - if (outc1 !== ma0.mb0.mc1.out) $stop; // Upper module name and lower instances - if (outc1 !== ma .mb0.mc1.out) $stop; // Upper module name and lower instances - if (outc1 !== mb.mc1.out) $stop; // This module name and lower instances - if (outc1 !== mb0.mc1.out) $stop; // Upper instance name and lower instances - if (outc1 !== mc1.out) $stop; // Lower instances - end - end -endmodule - -module mc (output reg [31:0] out); - `INLINE_MODULE - parameter P2 = 0; - parameter P3 = 0; - initial begin - out = {24'h0,P2[3:0],P3[3:0]}; - //$write("%m P2=%0x p3=%0x out=%x\n",P2, P3, out); - end - - // Can we look from the top module name down? - wire [31:0] reach_up_cyc = t.cyc; - - always @ (posedge t.clk) begin - //$write("[%0t] %m: Got reachup, cyc=%0d\n", $time, reach_up_cyc); - if (reach_up_cyc==2) begin - if (global_cell.globali != 32'hf00d) $stop; - if (global_cell2.globali != 32'hf33d) $stop; - end - if (reach_up_cyc==4) begin - out[15:12] <= {P2[3:0]+P3[3:0]+4'd1}; - end - if (reach_up_cyc==5) begin - // Can we set another instance? - if (P3==1) begin // Without this, there are two possible correct answers... - mc0.out[19:16] <= {mc0.out[19:16]+P2[3:0]+P3[3:0]+4'd2}; - $display("%m Set %x->%x %x %x %x %x",mc0.out, {mc0.out[19:16]+P2[3:0]+P3[3:0]+4'd2}, mc0.out[19:16],P2[3:0],P3[3:0],4'd2); - end - end - end -endmodule diff --git a/test_regress/t/t_var_dotted1.v b/test_regress/t/t_var_dotted1.v new file mode 100644 index 000000000..da1fb4c7b --- /dev/null +++ b/test_regress/t/t_var_dotted1.v @@ -0,0 +1,173 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2006 by Wilson Snyder. + +module t (/*AUTOARG*/ + // Inputs + clk + ); + + // verilator lint_off MULTIDRIVEN + + wire [31:0] outb0c0; + wire [31:0] outb0c1; + wire [31:0] outb1c0; + wire [31:0] outb1c1; + + reg [7:0] lclmem [7:0]; + + ma ma0 (.outb0c0(outb0c0), .outb0c1(outb0c1), + .outb1c0(outb1c0), .outb1c1(outb1c1) + ); + + global_mod #(32'hf00d) global_cell (); + global_mod #(32'hf22d) global_cell2 (); + + input clk; + integer cyc=1; + always @ (posedge clk) begin + cyc <= cyc + 1; +`ifdef TEST_VERBOSE + $write("[%0t] cyc%0d: %0x %0x %0x %0x\n", $time, cyc, outb0c0, outb0c1, outb1c0, outb1c1); +`endif + if (cyc==2) begin + if (global_cell.globali != 32'hf00d) $stop; + if (global_cell2.globali != 32'hf22d) $stop; + if (outb0c0 != 32'h00) $stop; + if (outb0c1 != 32'h01) $stop; + if (outb1c0 != 32'h10) $stop; + if (outb1c1 != 32'h11) $stop; + end + if (cyc==3) begin + // Can we scope down and read and write vars? + ma0.mb0.mc0.out <= ma0.mb0.mc0.out + 32'h100; + ma0.mb0.mc1.out <= ma0.mb0.mc1.out + 32'h100; + ma0.mb1.mc0.out <= ma0.mb1.mc0.out + 32'h100; + ma0.mb1.mc1.out <= ma0.mb1.mc1.out + 32'h100; + end + if (cyc==4) begin + // Can we do dotted's inside array sels? + ma0.rmtmem[ma0.mb0.mc0.out[2:0]] = 8'h12; + lclmem[ma0.mb0.mc0.out[2:0]] = 8'h24; + if (outb0c0 != 32'h100) $stop; + if (outb0c1 != 32'h101) $stop; + if (outb1c0 != 32'h110) $stop; + if (outb1c1 != 32'h111) $stop; + end + if (cyc==5) begin + if (ma0.rmtmem[ma0.mb0.mc0.out[2:0]] != 8'h12) $stop; + if (lclmem[ma0.mb0.mc0.out[2:0]] != 8'h24) $stop; + if (outb0c0 != 32'h1100) $stop; + if (outb0c1 != 32'h2101) $stop; + if (outb1c0 != 32'h2110) $stop; + if (outb1c1 != 32'h3111) $stop; + end + if (cyc==6) begin + if (outb0c0 != 32'h31100) $stop; + if (outb0c1 != 32'h02101) $stop; + if (outb1c0 != 32'h42110) $stop; + if (outb1c1 != 32'h03111) $stop; + end + if (cyc==9) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end + +endmodule + +`ifdef USE_INLINE_MID + `define INLINE_MODULE /*verilator inline_module*/ + `define INLINE_MID_MODULE /*verilator no_inline_module*/ +`else + `ifdef USE_INLINE + `define INLINE_MODULE /*verilator inline_module*/ + `define INLINE_MID_MODULE /*verilator inline_module*/ + `else + `define INLINE_MODULE /*verilator public_module*/ + `define INLINE_MID_MODULE /*verilator public_module*/ + `endif +`endif + +module global_mod; + `INLINE_MODULE + parameter INITVAL = 0; + integer globali; + initial globali = INITVAL; +endmodule + +module ma ( + output wire [31:0] outb0c0, + output wire [31:0] outb0c1, + output wire [31:0] outb1c0, + output wire [31:0] outb1c1 + ); + `INLINE_MODULE + + reg [7:0] rmtmem [7:0]; + + mb #(0) mb0 (.outc0(outb0c0), .outc1(outb0c1)); + mb #(1) mb1 (.outc0(outb1c0), .outc1(outb1c1)); +endmodule + +module mb ( + output wire [31:0] outc0, + output wire [31:0] outc1 + ); + `INLINE_MID_MODULE + parameter P2 = 0; + mc #(P2,0) mc0 (.out(outc0)); + mc #(P2,1) mc1 (.out(outc1)); + global_mod #(32'hf33d) global_cell2 (); + + wire reach_up_clk = t.clk; + always @(reach_up_clk) begin + if (P2==0) begin // Only for mb0 + if (outc0 !== t.ma0.mb0.mc0.out) $stop; // Top module name and lower instances + if (outc0 !== ma0.mb0.mc0.out) $stop; // Upper module name and lower instances + if (outc0 !== ma .mb0.mc0.out) $stop; // Upper module name and lower instances + if (outc0 !== mb.mc0.out) $stop; // This module name and lower instances + if (outc0 !== mb0.mc0.out) $stop; // Upper instance name and lower instances + if (outc0 !== mc0.out) $stop; // Lower instances + + if (outc1 !== t.ma0.mb0.mc1.out) $stop; // Top module name and lower instances + if (outc1 !== ma0.mb0.mc1.out) $stop; // Upper module name and lower instances + if (outc1 !== ma .mb0.mc1.out) $stop; // Upper module name and lower instances + if (outc1 !== mb.mc1.out) $stop; // This module name and lower instances + if (outc1 !== mb0.mc1.out) $stop; // Upper instance name and lower instances + if (outc1 !== mc1.out) $stop; // Lower instances + end + end +endmodule + +module mc (output reg [31:0] out); + `INLINE_MODULE + parameter P2 = 0; + parameter P3 = 0; + initial begin + out = {24'h0,P2[3:0],P3[3:0]}; + //$write("%m P2=%0x p3=%0x out=%x\n",P2, P3, out); + end + + // Can we look from the top module name down? + wire [31:0] reach_up_cyc = t.cyc; + + always @ (posedge t.clk) begin + //$write("[%0t] %m: Got reachup, cyc=%0d\n", $time, reach_up_cyc); + if (reach_up_cyc==2) begin + if (global_cell.globali != 32'hf00d) $stop; + if (global_cell2.globali != 32'hf33d) $stop; + end + if (reach_up_cyc==4) begin + out[15:12] <= {P2[3:0]+P3[3:0]+4'd1}; + end + if (reach_up_cyc==5) begin + // Can we set another instance? + if (P3==1) begin // Without this, there are two possible correct answers... + mc0.out[19:16] <= {mc0.out[19:16]+P2[3:0]+P3[3:0]+4'd2}; + $display("%m Set %x->%x %x %x %x %x",mc0.out, {mc0.out[19:16]+P2[3:0]+P3[3:0]+4'd2}, mc0.out[19:16],P2[3:0],P3[3:0],4'd2); + end + end + end +endmodule diff --git a/test_regress/t/t_var_dotted_inl0.pl b/test_regress/t/t_var_dotted1_inl0.pl similarity index 93% rename from test_regress/t/t_var_dotted_inl0.pl rename to test_regress/t/t_var_dotted1_inl0.pl index 73474fc47..3dde25892 100755 --- a/test_regress/t/t_var_dotted_inl0.pl +++ b/test_regress/t/t_var_dotted1_inl0.pl @@ -9,7 +9,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di scenarios(simulator => 1); -top_filename("t/t_var_dotted.v"); +top_filename("t/t_var_dotted1.v"); compile( v_flags2 => ['+define+NOUSE_INLINE',], diff --git a/test_regress/t/t_var_dotted_inl1.pl b/test_regress/t/t_var_dotted1_inl1.pl similarity index 93% rename from test_regress/t/t_var_dotted_inl1.pl rename to test_regress/t/t_var_dotted1_inl1.pl index 2dca3e9a2..a1676170d 100755 --- a/test_regress/t/t_var_dotted_inl1.pl +++ b/test_regress/t/t_var_dotted1_inl1.pl @@ -9,7 +9,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di scenarios(simulator => 1); -top_filename("t/t_var_dotted.v"); +top_filename("t/t_var_dotted1.v"); compile( v_flags2 => ['+define+USE_INLINE',], diff --git a/test_regress/t/t_var_dotted_inl2.pl b/test_regress/t/t_var_dotted1_inl2.pl similarity index 93% rename from test_regress/t/t_var_dotted_inl2.pl rename to test_regress/t/t_var_dotted1_inl2.pl index 9d713d5ee..39dbfed77 100755 --- a/test_regress/t/t_var_dotted_inl2.pl +++ b/test_regress/t/t_var_dotted1_inl2.pl @@ -9,7 +9,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di scenarios(simulator => 1); -top_filename("t/t_var_dotted.v"); +top_filename("t/t_var_dotted1.v"); compile( v_flags2 => ['+define+USE_INLINE_MID',], From 5b83484f20482cce9e95b538d6b1b386cc12e353 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 25 Feb 2020 18:57:51 -0500 Subject: [PATCH 10/16] Remove dead genblk code & some cleanups. --- src/V3AstNodes.h | 15 --------------- src/V3EmitV.cpp | 7 +------ src/V3LinkDot.cpp | 15 --------------- src/V3Param.cpp | 14 -------------- src/verilog.y | 12 +++++++----- 5 files changed, 8 insertions(+), 55 deletions(-) diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index dcba2822f..b7b5fe89e 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -2332,21 +2332,6 @@ public: //###################################################################### -class AstGenerate : public AstNode { - // A Generate/end block - // Parents: MODULE - // Children: modItems -public: - AstGenerate(FileLine* fl, AstNode* stmtsp) - : ASTGEN_SUPER(fl) { - addNOp1p(stmtsp); - } - ASTNODE_NODE_FUNCS(Generate) - // op1 = Statements - AstNode* stmtsp() const { return op1p(); } // op1 = List of statements - void addStmtp(AstNode* nodep) { addOp1p(nodep); } -}; - class AstParseRef : public AstNode { // A reference to a variable, function or task // We don't know which at parse time due to bison constraints diff --git a/src/V3EmitV.cpp b/src/V3EmitV.cpp index a07f6d477..afa57b8cf 100644 --- a/src/V3EmitV.cpp +++ b/src/V3EmitV.cpp @@ -76,7 +76,7 @@ class EmitVBaseVisitor : public EmitCBaseVisitor { } virtual void visit(AstBegin* nodep) VL_OVERRIDE { - if (nodep->unnamed()) { + if (nodep->name() == "") { putbs("begin\n"); } else { putbs("begin : "+nodep->name()+"\n"); @@ -84,11 +84,6 @@ class EmitVBaseVisitor : public EmitCBaseVisitor { iterateChildren(nodep); puts("end\n"); } - virtual void visit(AstGenerate* nodep) VL_OVERRIDE { - putfs(nodep, "generate\n"); - iterateChildren(nodep); - putqs(nodep, "end\n"); - } virtual void visit(AstFinal* nodep) VL_OVERRIDE { putfs(nodep, "final begin\n"); iterateChildren(nodep); diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index c7a045d1d..17db13e72 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -683,7 +683,6 @@ class LinkDotFindVisitor : public AstNVisitor { string m_scope; // Scope text AstBegin* m_beginp; // Current Begin/end block AstNodeFTask* m_ftaskp; // Current function/task - bool m_inGenerate; // Inside a generate bool m_inRecursion; // Inside a recursive module int m_paramNum; // Parameter number, for position based connection int m_beginNum; // Begin block number, 0=none seen @@ -891,16 +890,6 @@ class LinkDotFindVisitor : public AstNVisitor { nodep->user1p(m_curSymp); iterateChildren(nodep); } - virtual void visit(AstGenerate* nodep) VL_OVERRIDE { - // Begin: ... blocks often replicate under genif/genfor, so simply - // suppress duplicate checks. See t_gen_forif.v for an example. - bool lastInGen = m_inGenerate; - { - m_inGenerate = true; - iterateChildren(nodep); - } - m_inGenerate = lastInGen; - } virtual void visit(AstBegin* nodep) VL_OVERRIDE { UINFO(5," "<name(nodep->name()+cvtToStr(m_beginNum)); } - // Just for loop index, make special name. The [00] is so it will "dearray" to same - // name as after we expand the GENFOR - if (nodep->genforp()) nodep->name(nodep->name()); } // All blocks are numbered in the standard, IE we start with "genblk1" even if only one. if (nodep->name()=="" && nodep->unnamed()) { @@ -1202,7 +1188,6 @@ public: m_statep = statep; m_beginp = NULL; m_ftaskp = NULL; - m_inGenerate = false; m_inRecursion = false; m_paramNum = 0; m_beginNum = 0; diff --git a/src/V3Param.cpp b/src/V3Param.cpp index 1d758de07..9b551167c 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -431,20 +431,6 @@ private: } // Generate Statements - virtual void visit(AstGenerate* nodep) VL_OVERRIDE { - if (debug()>=9) nodep->dumpTree(cout, "-genin: "); - iterateChildren(nodep); - // After expanding the generate, all statements under it can be moved - // up, and the generate block deleted as it's not relevant - if (AstNode* stmtsp = nodep->stmtsp()) { - stmtsp->unlinkFrBackWithNext(); - nodep->replaceWith(stmtsp); - if (debug()>=9) stmtsp->dumpTree(cout, "-genout: "); - } else { - nodep->unlinkFrBack(); - } - VL_DO_DANGLING(nodep->deleteTree(), nodep); - } virtual void visit(AstGenIf* nodep) VL_OVERRIDE { UINFO(9," GENIF "<condp()); diff --git a/src/verilog.y b/src/verilog.y index 9427d1b8d..211e60a0b 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -1155,7 +1155,7 @@ interface_item: // IEEE: interface_item + non_port_interface_item ; interface_generate_region: // ==IEEE: generate_region - yGENERATE interface_itemList yENDGENERATE { $$ = new AstGenerate($1, $2); } + yGENERATE interface_itemList yENDGENERATE { $$ = $2; } | yGENERATE yENDGENERATE { $$ = NULL; } ; @@ -2035,7 +2035,7 @@ bind_instantiation: // ==IEEE: bind_instantiation // different, so we copy all rules for checkers. generate_region: // ==IEEE: generate_region - yGENERATE ~c~genItemList yENDGENERATE { $$ = new AstGenerate($1, $2); } + yGENERATE ~c~genItemList yENDGENERATE { $$ = $2; } | yGENERATE yENDGENERATE { $$ = NULL; } ; @@ -2043,7 +2043,7 @@ generate_region: // ==IEEE: generate_region //UNSUP BISONPRE_COPY(generate_region,{s/~c~/c_/g}) // {copied} //UNSUP ; -generate_block_or_null: // IEEE: generate_block_or_null +generate_block_or_null: // IEEE: generate_block_or_null (called from gencase/genif/genfor) // ';' // is included in // // IEEE: generate_block // // Must always return a BEGIN node, or NULL - see GenFor construction @@ -2054,9 +2054,11 @@ generate_block_or_null: // IEEE: generate_block_or_null genItemBegin: // IEEE: part of generate_block yBEGIN ~c~genItemList yEND { $$ = new AstBegin($1,"genblk",$2,true); } | yBEGIN yEND { $$ = NULL; } - | id ':' yBEGIN ~c~genItemList yEND endLabelE { $$ = new AstBegin($1,*$1,$4,true); GRAMMARP->endLabel($6,*$1,$6); } + | id ':' yBEGIN ~c~genItemList yEND endLabelE + { $$ = new AstBegin($1,*$1,$4,true); GRAMMARP->endLabel($6,*$1,$6); } | id ':' yBEGIN yEND endLabelE { $$ = NULL; GRAMMARP->endLabel($5,*$1,$5); } - | yBEGIN ':' idAny ~c~ genItemList yEND endLabelE { $$ = new AstBegin($3,*$3,$4,true); GRAMMARP->endLabel($6,*$3,$6); } + | yBEGIN ':' idAny ~c~genItemList yEND endLabelE + { $$ = new AstBegin($3,*$3,$4,true); GRAMMARP->endLabel($6,*$3,$6); } | yBEGIN ':' idAny yEND endLabelE { $$ = NULL; GRAMMARP->endLabel($5,*$3,$5); } ; From 4c438bbc672b225404ce2d447c0f494810785bad Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 25 Feb 2020 19:01:14 -0500 Subject: [PATCH 11/16] Commentary --- bin/verilator | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/verilator b/bin/verilator index 41be3b13d..0a3beebf0 100755 --- a/bin/verilator +++ b/bin/verilator @@ -1906,13 +1906,13 @@ Now we run Verilator on our little example. We then can compile it - cd obj_dir - make -j -f Vour.mk Vour__ALL.a - make -j -f Vour.mk ../sc_main.o verilated.o + make -j -C obj_dir -f Vour.mk Vour__ALL.a + make -j -C obj_dir -f Vour.mk ../sc_main.o verilated.o And link with SystemC. Note your path to the libraries may vary, depending on the operating system. + cd obj_dir export SYSTEMC_LIBDIR=/path/to/where/libsystemc.a/exists export LD_LIBRARY_PATH=$SYSTEMC_LIBDIR:$LD_LIBRARY_PATH # Might be needed if SystemC 2.3.0 From 68b6a0b66743cc660f25cc3b660e2a5ee3189692 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 25 Feb 2020 22:21:16 -0500 Subject: [PATCH 12/16] Fix genblk naming with directly nested generate blocks, #2176. --- Changes | 2 + src/V3AstNodes.cpp | 1 + src/V3AstNodes.h | 10 +- src/V3Begin.cpp | 2 +- src/V3LinkDot.cpp | 34 +++--- src/V3LinkParse.cpp | 17 ++- src/verilog.y | 21 ++-- test_regress/t/t_gen_genblk.out | 6 + test_regress/t/t_gen_genblk.pl | 20 ++++ test_regress/t/t_gen_genblk.v | 51 +++++++++ test_regress/t/t_gen_genblk_noinl.pl | 23 ++++ test_regress/t/t_interface_gen7.v | 6 +- test_regress/t/t_var_dotted2.v | 131 ++++++++++++++++++++++ test_regress/t/t_var_dotted2_inl0.pl | 23 ++++ test_regress/t/t_var_dotted2_inl1.pl | 23 ++++ test_regress/t/t_var_dotted_dup_bad.out | 7 ++ test_regress/t/t_var_dotted_dup_bad.pl | 18 +++ test_regress/t/t_var_dotted_dup_bad.v | 20 ++++ test_regress/t/t_var_nonamebegin__log.out | 12 +- test_regress/t/t_var_notfound_bad.out | 2 +- 20 files changed, 390 insertions(+), 39 deletions(-) create mode 100644 test_regress/t/t_gen_genblk.out create mode 100755 test_regress/t/t_gen_genblk.pl create mode 100644 test_regress/t/t_gen_genblk.v create mode 100755 test_regress/t/t_gen_genblk_noinl.pl create mode 100644 test_regress/t/t_var_dotted2.v create mode 100755 test_regress/t/t_var_dotted2_inl0.pl create mode 100755 test_regress/t/t_var_dotted2_inl1.pl create mode 100644 test_regress/t/t_var_dotted_dup_bad.out create mode 100755 test_regress/t/t_var_dotted_dup_bad.pl create mode 100644 test_regress/t/t_var_dotted_dup_bad.v diff --git a/Changes b/Changes index 1b8aedeb5..986c8a9a8 100644 --- a/Changes +++ b/Changes @@ -9,6 +9,8 @@ The contributors that suggested a given feature are shown in []. Thanks! *** Add check for assertOn for asserts, #2162. [Tobias Wölfel] +*** Fix genblk naming with directly nested generate blocks, #2176. [Alexander Grobman] + **** Fix undeclared VL_SHIFTR_WWQ, #2114. [Alex Solomatnikov] diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 835758651..127026a30 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -1282,6 +1282,7 @@ void AstBegin::dump(std::ostream& str) const { if (unnamed()) str<<" [UNNAMED]"; if (generate()) str<<" [GEN]"; if (genforp()) str<<" [GENFOR]"; + if (implied()) str<<" [IMPLIED]"; } void AstCoverDecl::dump(std::ostream& str) const { this->AstNode::dump(str); diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index b7b5fe89e..02d2a316c 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -3736,16 +3736,19 @@ class AstBegin : public AstNode { // Children: statements private: string m_name; // Name of block - bool m_unnamed; // Originally unnamed + bool m_unnamed; // Originally unnamed (name change does not affect this) bool m_generate; // Underneath a generate + bool m_implied; // Not inserted by user public: // Node that simply puts name into the output stream - AstBegin(FileLine* fl, const string& name, AstNode* stmtsp, bool generate=false) + AstBegin(FileLine* fl, const string& name, AstNode* stmtsp, bool generate = false, + bool implied = false) : ASTGEN_SUPER(fl) , m_name(name) { addNOp1p(stmtsp); - m_unnamed = (name==""); + m_unnamed = (name == ""); m_generate = generate; + m_implied = implied; } ASTNODE_NODE_FUNCS(Begin) virtual void dump(std::ostream& str) const; @@ -3760,6 +3763,7 @@ public: bool unnamed() const { return m_unnamed; } void generate(bool flag) { m_generate = flag; } bool generate() const { return m_generate; } + bool implied() const { return m_implied; } }; class AstInitial : public AstNode { diff --git a/src/V3Begin.cpp b/src/V3Begin.cpp index 2cb65de2e..77650f7a4 100644 --- a/src/V3Begin.cpp +++ b/src/V3Begin.cpp @@ -125,7 +125,7 @@ private: while ((pos=dottedname.find("__DOT__")) != string::npos) { string ident = dottedname.substr(0, pos); dottedname = dottedname.substr(pos+strlen("__DOT__")); - if (!nodep->unnamed()) { + if (nodep->name() != "") { if (m_namedScope=="") m_namedScope = ident; else m_namedScope = m_namedScope + "__DOT__"+ident; } diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index 17db13e72..557e2769c 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -914,20 +914,24 @@ class LinkDotFindVisitor : public AstNVisitor { } } } - int oldNum = m_beginNum; - AstBegin* oldbegin = m_beginp; - VSymEnt* oldCurSymp = m_curSymp; - { - m_beginNum = 0; - m_beginp = nodep; - m_curSymp = m_statep->insertBlock(m_curSymp, nodep->name(), nodep, m_packagep); - m_curSymp->fallbackp(oldCurSymp); - // Iterate + if (nodep->name() == "") { iterateChildren(nodep); + } else { + int oldNum = m_beginNum; + AstBegin* oldbegin = m_beginp; + VSymEnt* oldCurSymp = m_curSymp; + { + m_beginNum = 0; + m_beginp = nodep; + m_curSymp = m_statep->insertBlock(m_curSymp, nodep->name(), nodep, m_packagep); + m_curSymp->fallbackp(oldCurSymp); + // Iterate + iterateChildren(nodep); + } + m_curSymp = oldCurSymp; + m_beginp = oldbegin; + m_beginNum = oldNum; } - m_curSymp = oldCurSymp; - m_beginp = oldbegin; - m_beginNum = oldNum; } virtual void visit(AstNodeFTask* nodep) VL_OVERRIDE { // NodeTask: Remember its name for later resolution @@ -2449,8 +2453,10 @@ private: checkNoDot(nodep); VSymEnt* oldCurSymp = m_curSymp; { - m_ds.m_dotSymp = m_curSymp = m_statep->getNodeSym(nodep); - UINFO(5," cur=se"<name() != "") { + m_ds.m_dotSymp = m_curSymp = m_statep->getNodeSym(nodep); + UINFO(5," cur=se"<addNext(new AstWhile(fl, condp, newp, incp)); - newp = new AstBegin(nodep->fileline(), "", stmtsp); + newp = new AstBegin(nodep->fileline(), "", stmtsp, false, true); dimension--; } //newp->dumpTree(cout, "-foreach-new:"); @@ -488,6 +488,21 @@ private: virtual void visit(AstBegin* nodep) VL_OVERRIDE { V3Config::applyCoverageBlock(m_modp, nodep); cleanFileline(nodep); + AstNode* backp = nodep->backp(); + // IEEE says directly nested item is not a new block + bool nestedIf = (nodep->implied() // User didn't provide begin/end + && (VN_IS(nodep->stmtsp(), GenIf) + || VN_IS(nodep->stmtsp(), GenCase)) // Has an if/case + && !nodep->stmtsp()->nextp()); // Has only one item + // It's not FOR(BEGIN(...)) but we earlier changed it to BEGIN(FOR(...)) + if (nodep->genforp() && nodep->name() == "") { + nodep->name("genblk"); + } + else if (nodep->generate() && nodep->name() == "" + && (VN_IS(backp, CaseItem) || VN_IS(backp, GenIf)) + && !nestedIf) { + nodep->name("genblk"); + } iterateChildren(nodep); } virtual void visit(AstCase* nodep) VL_OVERRIDE { diff --git a/src/verilog.y b/src/verilog.y index 211e60a0b..f7b9320e7 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -2047,18 +2047,18 @@ generate_block_or_null: // IEEE: generate_block_or_null (called from genc // ';' // is included in // // IEEE: generate_block // // Must always return a BEGIN node, or NULL - see GenFor construction - generate_item { $$ = $1 ? (new AstBegin($1->fileline(),"genblk",$1,true)) : NULL; } + generate_item { $$ = $1 ? (new AstBegin($1->fileline(),"",$1,true,true)) : NULL; } | genItemBegin { $$ = $1; } ; genItemBegin: // IEEE: part of generate_block - yBEGIN ~c~genItemList yEND { $$ = new AstBegin($1,"genblk",$2,true); } + yBEGIN ~c~genItemList yEND { $$ = new AstBegin($1,"",$2,true,false); } | yBEGIN yEND { $$ = NULL; } | id ':' yBEGIN ~c~genItemList yEND endLabelE - { $$ = new AstBegin($1,*$1,$4,true); GRAMMARP->endLabel($6,*$1,$6); } + { $$ = new AstBegin($1,*$1,$4,true,false); GRAMMARP->endLabel($6,*$1,$6); } | id ':' yBEGIN yEND endLabelE { $$ = NULL; GRAMMARP->endLabel($5,*$1,$5); } | yBEGIN ':' idAny ~c~genItemList yEND endLabelE - { $$ = new AstBegin($3,*$3,$4,true); GRAMMARP->endLabel($6,*$3,$6); } + { $$ = new AstBegin($3,*$3,$4,true,false); GRAMMARP->endLabel($6,*$3,$6); } | yBEGIN ':' idAny yEND endLabelE { $$ = NULL; GRAMMARP->endLabel($5,*$3,$5); } ; @@ -2116,11 +2116,11 @@ loop_generate_construct: // ==IEEE: loop_generate_construct { // Convert BEGIN(...) to BEGIN(GENFOR(...)), as we need the BEGIN to hide the local genvar AstBegin* lowerBegp = VN_CAST($9, Begin); UASSERT_OBJ(!($9 && !lowerBegp), $9, "Child of GENFOR should have been begin"); - if (!lowerBegp) lowerBegp = new AstBegin($1,"genblk",NULL,true); // Empty body + if (!lowerBegp) lowerBegp = new AstBegin($1, "genblk", NULL, true, true); // Empty body AstNode* lowerNoBegp = lowerBegp->stmtsp(); if (lowerNoBegp) lowerNoBegp->unlinkFrBackWithNext(); // - AstBegin* blkp = new AstBegin($1,lowerBegp->name(),NULL,true); + AstBegin* blkp = new AstBegin($1, lowerBegp->name(), NULL, true, true); // V3LinkDot detects BEGIN(GENFOR(...)) as a special case AstNode* initp = $3; AstNode* varp = $3; if (VN_IS(varp, Var)) { // Genvar @@ -2793,10 +2793,10 @@ statement_item: // IEEE: statement_item statementFor: // IEEE: part of statement yFOR '(' for_initialization expr ';' for_stepE ')' stmtBlock - { $$ = new AstBegin($1,"",$3); + { $$ = new AstBegin($1, "", $3, false, true); $$->addStmtsp(new AstWhile($1, $4,$8,$6)); } | yFOR '(' for_initialization ';' for_stepE ')' stmtBlock - { $$ = new AstBegin($1,"",$3); + { $$ = new AstBegin($1, "", $3, false, true); $$->addStmtsp(new AstWhile($1, new AstConst($1,AstConst::LogicTrue()),$7,$5)); } ; @@ -4473,7 +4473,7 @@ assertion_item: // ==IEEE: assertion_item deferred_immediate_assertion_item: // ==IEEE: deferred_immediate_assertion_item deferred_immediate_assertion_statement { $$ = $1; } | id/*block_identifier*/ ':' deferred_immediate_assertion_statement - { $$ = new AstBegin($1, *$1, $3); } + { $$ = new AstBegin($1, *$1, $3, false, true); } ; procedural_assertion_statement: // ==IEEE: procedural_assertion_statement @@ -4528,7 +4528,8 @@ deferred_immediate_assertion_statement: // ==IEEE: deferred_immediate_ass concurrent_assertion_item: // IEEE: concurrent_assertion_item concurrent_assertion_statement { $$ = $1; } - | id/*block_identifier*/ ':' concurrent_assertion_statement { $$ = new AstBegin($1, *$1, $3); } + | id/*block_identifier*/ ':' concurrent_assertion_statement + { $$ = new AstBegin($1, *$1, $3, false, true); } // // IEEE: checker_instantiation // // identical to module_instantiation; see etcInst ; diff --git a/test_regress/t/t_gen_genblk.out b/test_regress/t/t_gen_genblk.out new file mode 100644 index 000000000..abaca2eeb --- /dev/null +++ b/test_regress/t/t_gen_genblk.out @@ -0,0 +1,6 @@ +010: exp=top.t.show0 got=top.t.show0 +014: exp=top.t.genblk1.show1 got=top.t.genblk1.show1 +018: exp=top.t.genblk2.show2 got=top.t.genblk2.show2 +023: exp=top.t.genblk3.genblk1.show3 got=top.t.genblk3.genblk1.show3 +029: exp=top.t.x1.x3.show4 got=top.t.x1.x3.show4 +*-* All Finished *-* diff --git a/test_regress/t/t_gen_genblk.pl b/test_regress/t/t_gen_genblk.pl new file mode 100755 index 000000000..e109f0f6c --- /dev/null +++ b/test_regress/t/t_gen_genblk.pl @@ -0,0 +1,20 @@ +#!/usr/bin/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. + +scenarios(simulator => 1); + +compile( + ); + +execute( + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_gen_genblk.v b/test_regress/t/t_gen_genblk.v new file mode 100644 index 000000000..bb28c18be --- /dev/null +++ b/test_regress/t/t_gen_genblk.v @@ -0,0 +1,51 @@ +module t (/*AUTOARG*/ + // Inputs + clk, reset_l + ); + + input clk; + input reset_l; + + generate + show #(`__LINE__, "top.t.show0") show0(); + + if (0) ; + else if (0) ; + else if (1) show #(`__LINE__, "top.t.genblk1.show1") show1(); + + if (0) begin end + else if (0) begin end + else if (1) begin show #(`__LINE__, "top.t.genblk2.show2") show2(); end + + if (0) ; + else begin + if (0) begin end + else if (1) begin show #(`__LINE__, "top.t.genblk3.genblk1.show3") show3(); end + end + + if (0) ; + else begin : x1 + if (0) begin : x2 end + else if (1) begin : x3 show #(`__LINE__, "top.t.x1.x3.show4") show4(); end + end + endgenerate + + int cyc; + + always @ (posedge clk) begin + cyc <= cyc + 1; + if (cyc == 99) begin + $write("*-* All Finished *-*\n"); + $finish; + end + + end +endmodule + +module show #(parameter LINE=0, parameter string EXPT) (); + always @ (posedge t.clk) begin + if (t.cyc == LINE) begin + $display("%03d: exp=%s got=%m", LINE, EXPT); + end + end +endmodule diff --git a/test_regress/t/t_gen_genblk_noinl.pl b/test_regress/t/t_gen_genblk_noinl.pl new file mode 100755 index 000000000..901e0993c --- /dev/null +++ b/test_regress/t/t_gen_genblk_noinl.pl @@ -0,0 +1,23 @@ +#!/usr/bin/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. + +top_filename("t_gen_genblk.v"); + +scenarios(simulator => 1); + +compile( + v_flags2 => ["-Oi"], + ); + +execute( + expect_filename => "t/t_gen_genblk.out", + ); + +ok(1); +1; diff --git a/test_regress/t/t_interface_gen7.v b/test_regress/t/t_interface_gen7.v index dcec13228..c1a0d2a41 100644 --- a/test_regress/t/t_interface_gen7.v +++ b/test_regress/t/t_interface_gen7.v @@ -24,7 +24,7 @@ module t(); generate genvar the_genvar; - begin + begin : ia for (the_genvar = 0; the_genvar < 2; the_genvar++) begin : TestIf begin assign my_intf[the_genvar].val = '1; @@ -36,7 +36,7 @@ module t(); generate genvar the_second_genvar; - begin + begin : ib intf #(.PARAM(1)) my_intf [1:0] (); for (the_second_genvar = 0; the_second_genvar < 2; the_second_genvar++) begin : TestIf begin @@ -49,7 +49,7 @@ module t(); generate genvar the_third_genvar; - begin + begin : ic for (the_third_genvar = 0; the_third_genvar < 2; the_third_genvar++) begin : TestIf begin intf #(.PARAM(1)) my_intf [1:0] (); diff --git a/test_regress/t/t_var_dotted2.v b/test_regress/t/t_var_dotted2.v new file mode 100644 index 000000000..7138dbd86 --- /dev/null +++ b/test_regress/t/t_var_dotted2.v @@ -0,0 +1,131 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty 2020 by Wilson Snyder. + +`ifdef USE_INLINE + `define INLINE_MODULE /*verilator inline_module*/ +`else + `define INLINE_MODULE /*verilator public_module*/ +`endif + +module t (/*AUTOARG*/); + +`define DRAM1(bank) mem.mem_bank[bank].dccm.dccm_bank.ram_core +`define DRAM2(bank) mem.mem_bank2[bank].dccm.dccm_bank.ram_core +`define DRAM3(bank) mem.mem_bank3[bank].dccm.dccm_bank.ram_core +`define DRAM4(bank) mem.sub4.mem_bank4[bank].dccm.dccm_bank.ram_core + + initial begin + `DRAM1(0)[3] = 130; + `DRAM1(1)[3] = 131; + `DRAM2(0)[3] = 230; + `DRAM2(1)[3] = 231; + `DRAM3(0)[3] = 330; + `DRAM3(1)[3] = 331; + `DRAM4(0)[3] = 430; + `DRAM4(1)[3] = 431; + if (`DRAM1(0)[3] !== 130) $stop; + if (`DRAM1(1)[3] !== 131) $stop; + if (`DRAM2(0)[3] !== 230) $stop; + if (`DRAM2(1)[3] !== 231) $stop; + if (`DRAM3(0)[3] !== 330) $stop; + if (`DRAM3(1)[3] !== 331) $stop; + if (`DRAM4(0)[3] !== 430) $stop; + if (`DRAM4(1)[3] !== 431) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end + + eh2_lsu_dccm_mem mem (/*AUTOINST*/); + +endmodule + +module eh2_lsu_dccm_mem +#( + DCCM_INDEX_DEPTH = 8192, + DCCM_NUM_BANKS = 2 + )( +); + `INLINE_MODULE + + // 8 Banks, 16KB each (2048 x 72) + for (genvar i=0; i 1); + +top_filename("t/t_var_dotted2.v"); + +compile( + v_flags2 => ['+define+NOUSE_INLINE',], + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_var_dotted2_inl1.pl b/test_regress/t/t_var_dotted2_inl1.pl new file mode 100755 index 000000000..eba0adacc --- /dev/null +++ b/test_regress/t/t_var_dotted2_inl1.pl @@ -0,0 +1,23 @@ +#!/usr/bin/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. + +scenarios(simulator => 1); + +top_filename("t/t_var_dotted2.v"); + +compile( + v_flags2 => ['+define+USE_INLINE',], + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_var_dotted_dup_bad.out b/test_regress/t/t_var_dotted_dup_bad.out new file mode 100644 index 000000000..bcda5f416 --- /dev/null +++ b/test_regress/t/t_var_dotted_dup_bad.out @@ -0,0 +1,7 @@ +%Error: t/t_var_dotted_dup_bad.v:13: Duplicate declaration of cell: 'dccm_bank' + eh2_ram dccm_bank (.*); + ^~~~~~~~~ + t/t_var_dotted_dup_bad.v:10: ... Location of original declaration + eh2_ram dccm_bank (.*); + ^~~~~~~~~ +%Error: Exiting due to diff --git a/test_regress/t/t_var_dotted_dup_bad.pl b/test_regress/t/t_var_dotted_dup_bad.pl new file mode 100755 index 000000000..e142ae153 --- /dev/null +++ b/test_regress/t/t_var_dotted_dup_bad.pl @@ -0,0 +1,18 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2005 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. + +scenarios(vlt => 1); + +lint( + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_var_dotted_dup_bad.v b/test_regress/t/t_var_dotted_dup_bad.v new file mode 100644 index 000000000..e22f6e514 --- /dev/null +++ b/test_regress/t/t_var_dotted_dup_bad.v @@ -0,0 +1,20 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty 2020 by Wilson Snyder. + +module t (/*AUTOARG*/); + + generate + begin + eh2_ram dccm_bank (.*); + end + begin + eh2_ram dccm_bank (.*); // Error: duplicate + end + endgenerate + +endmodule + +module eh2_ram (); +endmodule diff --git a/test_regress/t/t_var_nonamebegin__log.out b/test_regress/t/t_var_nonamebegin__log.out index e9277860e..441195db4 100644 --- a/test_regress/t/t_var_nonamebegin__log.out +++ b/test_regress/t/t_var_nonamebegin__log.out @@ -1,9 +1,9 @@ ingen: {mod}.genblk1 top.t.genblk1 -d3a: {mod}.d3nameda top.t.d3nameda -b2: {mod} top.t -b3n: {mod}.b3named: top.t.b3named -b3: {mod} top.t -b4: {mod} top.t +d3a: {mod}.d3nameda top.t.unnamedblk1.d3nameda +b2: {mod} top.t.unnamedblk2 +b3n: {mod}.b3named: top.t.unnamedblk2.b3named +b3: {mod} top.t.unnamedblk2.unnamedblk3 +b4: {mod} top.t.unnamedblk2.unnamedblk3.unnamedblk4 t1 {mod}.tsk top.t -t2 {mod}.tsk top.t +t2 {mod}.tsk top.t.unnamedblk7 *-* All Finished *-* diff --git a/test_regress/t/t_var_notfound_bad.out b/test_regress/t/t_var_notfound_bad.out index cad5a6c9f..44bae4abf 100644 --- a/test_regress/t/t_var_notfound_bad.out +++ b/test_regress/t/t_var_notfound_bad.out @@ -13,7 +13,7 @@ : ... Suggested alternative: 'notfuncs' i = sub.nofuncs(); ^~~~~~~ - ... Known scopes under 'nofuncs': + ... Known scopes under 'nofuncs': sub %Error: t/t_var_notfound_bad.v:21: Can't find definition of task/function: 'notask' : ... Suggested alternative: 'nottask' notask(); From 6131bcdbb08a8d8fd07171860adb0980de0c5132 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Thu, 27 Feb 2020 07:12:50 -0500 Subject: [PATCH 13/16] Verilator_gantt: Fix CPU count in report. --- bin/verilator_gantt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/verilator_gantt b/bin/verilator_gantt index 252b98c9a..574d01567 100755 --- a/bin/verilator_gantt +++ b/bin/verilator_gantt @@ -123,13 +123,13 @@ sub report { } my $nthreads = scalar keys %Threads; - $Global{cpus}{cpu_time} = {}; + $Global{cpus} = {}; foreach my $thread (keys %Threads) { # Make potentially multiple characters per column foreach my $start (keys %{$Threads{$thread}}) { my $cpu = $Threads{$thread}{$start}{cpu}; my $elapsed = $Threads{$thread}{$start}{end} - $start; - $Global{cpus}{cpu_time}{$cpu} += $elapsed; + $Global{cpus}{$cpu}{cpu_time} += $elapsed; } } From c06a97a2214cfe096e55b4535f3f9f69eba20e6b Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Thu, 27 Feb 2020 07:18:15 -0500 Subject: [PATCH 14/16] Remove OBJCACHE_HOSTS; dead code. --- Makefile.in | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Makefile.in b/Makefile.in index d79d0b2ff..6f7441743 100644 --- a/Makefile.in +++ b/Makefile.in @@ -208,12 +208,6 @@ EXAMPLES_FIRST = \ EXAMPLES = $(EXAMPLES_FIRST) $(filter-out $(EXAMPLES_FIRST), $(sort $(wildcard examples/*))) -ifeq ($(OBJCACHE_JOBS),) -ifneq ($(OBJCACHE_HOSTS),) -export OBJCACHE_JOBS := -j $(shell objcache --jobs "$(OBJCACHE_HOSTS)") -endif -endif - # See uninstall also - don't put wildcards in this variable, it might uninstall other stuff VL_INST_MAN_FILES = verilator.1 verilator_coverage.1 verilator_gantt.1 verilator_profcfunc.1 From 991d81cd0a0f92487d38beb6f946c0d12c71c272 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Thu, 27 Feb 2020 07:46:34 -0500 Subject: [PATCH 15/16] Recommend -Os. --- Changes | 2 ++ bin/verilator | 9 ++++----- examples/cmake_tracing_c/CMakeLists.txt | 2 +- examples/cmake_tracing_sc/CMakeLists.txt | 2 +- examples/make_protect_lib/Makefile | 2 +- examples/make_tracing_c/Makefile | 2 +- examples/make_tracing_c/Makefile_obj | 2 +- examples/make_tracing_sc/Makefile | 2 +- examples/make_tracing_sc/Makefile_obj | 2 +- include/verilated.mk.in | 2 +- test_regress/driver.pl | 4 ++-- 11 files changed, 16 insertions(+), 15 deletions(-) diff --git a/Changes b/Changes index 986c8a9a8..e912ff79f 100644 --- a/Changes +++ b/Changes @@ -11,6 +11,8 @@ The contributors that suggested a given feature are shown in []. Thanks! *** Fix genblk naming with directly nested generate blocks, #2176. [Alexander Grobman] +**** Use gcc -Os in examples instead of -O2 for better average performance. + **** Fix undeclared VL_SHIFTR_WWQ, #2114. [Alex Solomatnikov] diff --git a/bin/verilator b/bin/verilator index 0a3beebf0..1ec4e722c 100755 --- a/bin/verilator +++ b/bin/verilator @@ -1964,19 +1964,18 @@ OPT, OPT_FAST, or OPT_SLOW lib/verilated.mk. Or, use the -CFLAGS and/or the compiler or linker. Or, just for one run, pass them on the command line to make: - make OPT_FAST="-O2 -fno-stack-protector" -f Vour.mk Vour__ALL.a + make OPT_FAST="-Os -fno-stack-protector" -f Vour.mk Vour__ALL.a OPT_FAST specifies optimizations for those programs that are part of the fast path, mostly code that is executed every cycle. OPT_SLOW specifies optimizations for slow-path files (plus tracing), which execute only rarely, yet take a long time to compile with optimization on. OPT specifies overall optimization and affects all compiles, including those -OPT_FAST and OPT_SLOW control. For best results, use OPT="-O2", and link +OPT_FAST and OPT_SLOW control. For best results, use OPT="-Os", and link with "-static". Nearly the same results can be had with much better compile times with OPT_FAST="-O1 -fstrict-aliasing". Higher optimization -such as "-O3" may help, but gcc compile times may be excessive under O3 on -even medium sized designs. Alternatively, some larger designs report -better performance using "-Os". +such as "-O2" or "-O3" may help, but gcc compile times may be excessive +under O3 on even medium sized designs. Unfortunately, using the optimizer with SystemC files can result in compiles taking several minutes. (The SystemC libraries have many little diff --git a/examples/cmake_tracing_c/CMakeLists.txt b/examples/cmake_tracing_c/CMakeLists.txt index ab5236b3c..d26932609 100644 --- a/examples/cmake_tracing_c/CMakeLists.txt +++ b/examples/cmake_tracing_c/CMakeLists.txt @@ -34,5 +34,5 @@ add_executable(example ../make_tracing_c/sim_main.cpp) # Add the Verilated circuit to the target verilate(example COVERAGE TRACE INCLUDE_DIRS "../make_tracing_c" - VERILATOR_ARGS -f ../make_tracing_c/input.vc -O2 -x-assign 0 + VERILATOR_ARGS -f ../make_tracing_c/input.vc -Os -x-assign 0 SOURCES ../make_tracing_c/top.v) diff --git a/examples/cmake_tracing_sc/CMakeLists.txt b/examples/cmake_tracing_sc/CMakeLists.txt index f2490eb6c..20a55c06e 100644 --- a/examples/cmake_tracing_sc/CMakeLists.txt +++ b/examples/cmake_tracing_sc/CMakeLists.txt @@ -41,7 +41,7 @@ add_executable(example ../make_tracing_sc/sc_main.cpp) # Add the Verilated circuit to the target verilate(example SYSTEMC COVERAGE TRACE INCLUDE_DIRS "../make_tracing_sc" - VERILATOR_ARGS -f ../make_tracing_sc/input.vc -O2 -x-assign 0 + VERILATOR_ARGS -f ../make_tracing_sc/input.vc -Os -x-assign 0 SOURCES ../make_tracing_sc/top.v) verilator_link_systemc(example) diff --git a/examples/make_protect_lib/Makefile b/examples/make_protect_lib/Makefile index 0f9f2e1a3..b2e33a283 100644 --- a/examples/make_protect_lib/Makefile +++ b/examples/make_protect_lib/Makefile @@ -34,7 +34,7 @@ VERILATOR_FLAGS = # Generate C++ VERILATOR_FLAGS += -cc # Optimize -VERILATOR_FLAGS += -O2 -x-assign 0 +VERILATOR_FLAGS += -Os -x-assign 0 # Warn abount lint issues; may not want this on less solid designs VERILATOR_FLAGS += -Wall # Make waveforms diff --git a/examples/make_tracing_c/Makefile b/examples/make_tracing_c/Makefile index f5a3b497e..a6acdc445 100644 --- a/examples/make_tracing_c/Makefile +++ b/examples/make_tracing_c/Makefile @@ -38,7 +38,7 @@ VERILATOR_FLAGS += -cc --exe # Generate makefile dependencies (not shown as complicates the Makefile) #VERILATOR_FLAGS += -MMD # Optimize -VERILATOR_FLAGS += -O2 -x-assign 0 +VERILATOR_FLAGS += -Os -x-assign 0 # Warn abount lint issues; may not want this on less solid designs VERILATOR_FLAGS += -Wall # Make waveforms diff --git a/examples/make_tracing_c/Makefile_obj b/examples/make_tracing_c/Makefile_obj index c3eb5291d..6d07eb427 100644 --- a/examples/make_tracing_c/Makefile_obj +++ b/examples/make_tracing_c/Makefile_obj @@ -38,7 +38,7 @@ endif # SystemC takes minutes to optimize, thus it is off by default. OPT_SLOW = # Fast path optimizations. Most time is spent in these classes. -OPT_FAST = -O2 -fstrict-aliasing +OPT_FAST = -Os -fstrict-aliasing #OPT_FAST = -O #OPT_FAST = diff --git a/examples/make_tracing_sc/Makefile b/examples/make_tracing_sc/Makefile index e70053c53..ded101604 100644 --- a/examples/make_tracing_sc/Makefile +++ b/examples/make_tracing_sc/Makefile @@ -38,7 +38,7 @@ VERILATOR_FLAGS += -sc --exe # Generate makefile dependencies (not shown as complicates the Makefile) #VERILATOR_FLAGS += -MMD # Optimize -VERILATOR_FLAGS += -O2 -x-assign 0 +VERILATOR_FLAGS += -Os -x-assign 0 # Warn abount lint issues; may not want this on less solid designs VERILATOR_FLAGS += -Wall # Make waveforms diff --git a/examples/make_tracing_sc/Makefile_obj b/examples/make_tracing_sc/Makefile_obj index 7c52ba627..03ffe389b 100644 --- a/examples/make_tracing_sc/Makefile_obj +++ b/examples/make_tracing_sc/Makefile_obj @@ -46,7 +46,7 @@ endif # SystemC takes minutes to optimize, thus it is off by default. OPT_SLOW = # Fast path optimizations. Most time is spent in these classes. -OPT_FAST = -O2 -fstrict-aliasing +OPT_FAST = -Os -fstrict-aliasing #OPT_FAST = -O #OPT_FAST = diff --git a/include/verilated.mk.in b/include/verilated.mk.in index e64de5e97..b00b58272 100644 --- a/include/verilated.mk.in +++ b/include/verilated.mk.in @@ -86,7 +86,7 @@ LDLIBS += $(VM_USER_LDLIBS) # SystemC takes minutes to optimize, thus it is off by default. #OPT_SLOW = # Fast path optimizations. Most time is spent in these classes. -#OPT_FAST = -O2 -fstrict-aliasing +#OPT_FAST = -Os -fstrict-aliasing #OPT_FAST = -O #OPT_FAST = diff --git a/test_regress/driver.pl b/test_regress/driver.pl index 24640f9c0..6e10dd113 100755 --- a/test_regress/driver.pl +++ b/test_regress/driver.pl @@ -1048,7 +1048,7 @@ sub compile { "-DTEST_VERBOSE=\"".($self->{verbose} ? 1 : 0)."\"", "-DTEST_SYSTEMC=\"" .($self->sc ? 1 : 0). "\"", "-DCMAKE_PREFIX_PATH=\"".(($ENV{SYSTEMC_INCLUDE}||$ENV{SYSTEMC}||'')."/..\""), - "-DTEST_OPT_FAST=\"" . ($param{benchmark}?"-O2":"") . "\"", + "-DTEST_OPT_FAST=\"" . ($param{benchmark} ? "-Os" : "") . "\"", "-DTEST_VERILATION=\"" . $::Opt_Verilation . "\"", ]); return 1 if $self->errors || $self->skips || $self->unsupporteds; @@ -1066,7 +1066,7 @@ sub compile { "TEST_OBJ_DIR=$self->{obj_dir}", "CPPFLAGS_DRIVER=-D".uc($self->{name}), ($self->{verbose} ? "CPPFLAGS_DRIVER2=-DTEST_VERBOSE=1":""), - ($param{benchmark}?"OPT_FAST=-O2":""), + ($param{benchmark} ? "OPT_FAST=-Os" : ""), "$self->{VM_PREFIX}", # bypass default rule, as we don't need archive ($param{make_flags}||""), ]); From c6b755a12ee6b953808cacd449a6551d04e3aaf1 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Thu, 27 Feb 2020 19:35:49 -0500 Subject: [PATCH 16/16] Commentary --- README.adoc | 4 ++-- bin/verilator | 14 +++++++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/README.adoc b/README.adoc index 395302e64..3e2eb528b 100644 --- a/README.adoc +++ b/README.adoc @@ -25,7 +25,7 @@ endif::[] ^.^| *Welcome to Verilator, the fastest free Verilog HDL simulator.* +++
+++ • Accepts synthesizable Verilog or SystemVerilog +++
+++ • Performs lint code-quality checks -+++
+++ • Compiles into multithreaded {cpp}, SystemC, or (soon) {cpp}-under-Python ++++
+++ • Compiles into multithreaded {cpp}, or SystemC +++
+++ • Creates XML to front-end your own tools <.^|image:https://www.veripool.org/img/verilator_256_200_min.png[Logo,256,200] @@ -81,7 +81,7 @@ touch of {cpp} code, Verilator is the tool for you. Verilator does not simply convert Verilog HDL to {cpp} or SystemC. Rather than only translate, Verilator compiles your code into a much faster optimized and optionally thread-partitioned model, which is in turn wrapped -inside a {cpp}/SystemC/Python module. The results are a compiled Verilog +inside a {cpp}/SystemC/{cpp}-under-Python module. The results are a compiled Verilog model that executes even on a single-thread over 10x faster than standalone SystemC, and on a single thread is about 100 times faster than interpreted Verilog simulators such as http://iverilog.icarus.com[Icarus diff --git a/bin/verilator b/bin/verilator index 1ec4e722c..bb3dbb69b 100755 --- a/bin/verilator +++ b/bin/verilator @@ -1981,9 +1981,10 @@ Unfortunately, using the optimizer with SystemC files can result in compiles taking several minutes. (The SystemC libraries have many little inlined functions that drive the compiler nuts.) -For best results, use GCC 3.3 or newer. GCC 3.2 and earlier have -optimization bugs around pointer aliasing detection, which can result in 2x -performance losses. +For best results, use the latest clang compiler (about 10% faster than +GCC). Note the now fairly old GCC 3.2 and earlier have optimization bugs +around pointer aliasing detection, which can result in 2x performance +losses. If you will be running many simulations on a single compile, investigate feedback driven compilation. With GCC, using -fprofile-arcs, then @@ -1994,6 +1995,9 @@ especially if you link in DPI code. To enable LTO on GCC, pass "-flto" in both compilation and link. Note LTO may cause excessive compile times on large designs. +Using profile driven compiler optimization, with feedback from a real +design, can yield up to30% improvements. + If you are using your own makefiles, you may want to compile the Verilated code with -DVL_INLINE_OPT=inline. This will inline functions, however this requires that all cpp files be compiled in a single compiler run. @@ -2004,7 +2008,7 @@ either oprofile or gprof to see where in the C++ code the time is spent. Run the gprof output through verilator_profcfunc and it will tell you what Verilog line numbers on which most of the time is being spent. -When done, please let the author know the results. I like to keep tabs on +When done, please let the author know the results. We like to keep tabs on how Verilator compares, and may be able to suggest additional improvements. @@ -2079,7 +2083,7 @@ After running Make, the C++ compiler may produce the following: A generic Linux/OS variable specifying what directories have shared object (.so) files. This path should include SystemC and any other shared objects -needed at simultion runtime. +needed at simulation runtime. =item OBJCACHE