Support default value on module input (#5358) (#5373)

This commit is contained in:
Drew Ranck 2024-08-15 10:04:07 -04:00 committed by GitHub
parent 563faeb33f
commit 48c71ef76c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
22 changed files with 524 additions and 27 deletions

View File

@ -787,6 +787,7 @@ public:
bool isAny() const { return m_e != NONE; }
// Looks like inout - "ish" because not identical to being an INOUT
bool isInoutish() const { return m_e == INOUT; }
bool isInput() const { return m_e == INPUT; }
bool isNonOutput() const {
return m_e == INPUT || m_e == INOUT || m_e == REF || m_e == CONSTREF;
}

View File

@ -2014,6 +2014,7 @@ public:
bool isContinuously() const { return m_isContinuously; }
bool isDeclTyped() const { return m_declTyped; }
bool isInoutish() const { return m_direction.isInoutish(); }
bool isInput() const { return m_direction.isInput(); }
bool isNonOutput() const { return m_direction.isNonOutput(); }
bool isReadOnly() const VL_MT_SAFE { return m_direction.isReadOnly(); }
bool isConstRef() const VL_MT_SAFE { return m_direction.isConstRef(); }

View File

@ -398,6 +398,7 @@ class LinkCellsVisitor final : public VNVisitor {
for (AstNode* portnodep = nodep->modp()->stmtsp(); portnodep;
portnodep = portnodep->nextp()) {
if (const AstPort* const portp = VN_CAST(portnodep, Port)) {
if (ports.find(portp->name()) == ports.end()
&& ports.find("__pinNumber" + cvtToStr(portp->pinNum())) == ports.end()) {
if (pinStar) {
@ -411,15 +412,49 @@ class LinkCellsVisitor final : public VNVisitor {
newp->svImplicit(true);
nodep->addPinsp(newp);
} else { // warn on the CELL that needs it, not the port
nodep->v3warn(PINMISSING, "Cell has missing pin: "
<< portp->prettyNameQ() << '\n'
<< nodep->warnContextPrimary() << '\n'
<< portp->warnOther()
<< "... Location of port declaration\n"
<< portp->warnContextSecondary());
AstPin* const newp
= new AstPin{nodep->fileline(), 0, portp->name(), nullptr};
nodep->addPinsp(newp);
// We *might* not want to warn on this port, if it happened to be
// an input with a default value in the module declaration. Our
// AstPort* (portp) doesn't have that information, but the Module
// (nodep->modp()) statements do that have information in an AstVar*
// with the same name() as the port. We'll look for that in-line here,
// if a port is missing on this instance.
// Get the AstVar for this AstPort, if it exists, using this
// inefficient O(n) lookup to match the port name.
const AstVar* portp_varp = nullptr;
for (AstNode* module_stmtsp = nodep->modp()->stmtsp(); module_stmtsp;
module_stmtsp = module_stmtsp->nextp()) {
if (const AstVar* const varp = VN_CAST(module_stmtsp, Var)) {
if (!varp->isParam() && varp->name() == portp->name()) {
// not a parameter, same name, break, this is our varp
// (AstVar*)
portp_varp = varp;
break;
}
}
}
// Is the matching Module port: an INPUT, with default value (in
// valuep):
if (portp_varp && portp_varp->isInput() && portp_varp->valuep()) {
// Do not warn
// Create b/c not already connected, and it does exist.
AstPin* const newp
= new AstPin{nodep->fileline(), 0, portp->name(), nullptr};
nodep->addPinsp(newp);
} else {
nodep->v3warn(PINMISSING,
"Cell has missing pin: "
<< portp->prettyNameQ() << '\n'
<< nodep->warnContextPrimary() << '\n'
<< portp->warnOther()
<< "... Location of port declaration\n"
<< portp->warnContextSecondary());
AstPin* const newp
= new AstPin{nodep->fileline(), 0, portp->name(), nullptr};
nodep->addPinsp(newp);
}
}
}
}

View File

@ -62,8 +62,11 @@ class LinkLValueVisitor final : public VNVisitor {
if (m_setForcedByCode) {
nodep->varp()->setForcedByCode();
} else if (!nodep->varp()->isFuncLocal() && nodep->varp()->isReadOnly()) {
nodep->v3warn(ASSIGNIN,
"Assigning to input/const variable: " << nodep->prettyNameQ());
// This is allowed with IEEE 1800-2009 module input with default value.
// the checking now happens in V3Width::visit(AstNodeVarRef*)
// If you were to check here, it would fail on module inputs with default value,
// because Inputs are isReadOnly()=true, and we don't yet have visibility into
// it being an Initial style procedure.
}
}
iterateChildren(nodep);

View File

@ -371,9 +371,12 @@ class LinkParseVisitor final : public VNVisitor {
FileLine* const fl = nodep->valuep()->fileline();
if (nodep->isParam() || (m_ftaskp && nodep->isNonOutput())) {
// 1. Parameters and function inputs: It's a default to use if not overridden
} else if (!m_ftaskp && !VN_IS(m_modp, Class) && nodep->isNonOutput()) {
nodep->v3warn(E_UNSUPPORTED, "Unsupported: Default value on module input: "
<< nodep->prettyNameQ());
} else if (!m_ftaskp && !VN_IS(m_modp, Class) && nodep->isNonOutput()
&& !nodep->isInput()) {
// Module inout/ref/constref: const default to use
nodep->v3warn(E_UNSUPPORTED,
"Unsupported: Default value on module inout/ref/constref: "
<< nodep->prettyNameQ());
nodep->valuep()->unlinkFrBack()->deleteTree();
} // 2. Under modules/class, it's an initial value to be loaded at time 0 via an
// AstInitial

View File

@ -2343,6 +2343,12 @@ class WidthVisitor final : public VNVisitor {
// if (debug() >= 9) nodep->dumpTree("- VRout: ");
if (nodep->access().isWriteOrRW() && nodep->varp()->direction() == VDirection::CONSTREF) {
nodep->v3error("Assigning to const ref variable: " << nodep->prettyNameQ());
} else if (!nodep->varp()->isForced() && nodep->access().isWriteOrRW()
&& nodep->varp()->isInput() && !nodep->varp()->isFuncLocal()
&& nodep->varp()->isReadOnly() && (!m_ftaskp || !m_ftaskp->isConstructor())
&& !VN_IS(m_procedurep, InitialAutomatic)
&& !VN_IS(m_procedurep, InitialStatic)) {
nodep->v3warn(ASSIGNIN, "Assigning to input/const variable: " << nodep->prettyNameQ());
} else if (nodep->access().isWriteOrRW() && nodep->varp()->isConst() && !m_paramsOnly
&& (!m_ftaskp || !m_ftaskp->isConstructor())
&& !VN_IS(m_procedurep, InitialAutomatic)

View File

@ -1,5 +0,0 @@
%Error-UNSUPPORTED: t/t_lint_input_eq_bad.v:10:15: Unsupported: Default value on module input: 'i2'
10 | input wire i2 = i
| ^~
... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest
%Error: Exiting due to

View File

@ -0,0 +1,16 @@
#!/usr/bin/env perl
if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2008 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 => 1);
lint();
ok(1);
1;

View File

@ -6,8 +6,8 @@
module t
(
input wire i,
input wire i2 = i // BAD
input wire i,
input wire i2 = i // Good under IEEE 1800-2009
);
endmodule

View File

@ -0,0 +1,21 @@
#!/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(
);
execute(
check_finished => 1,
);
ok(1);
1;

View File

@ -0,0 +1,179 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty, 2024 by Andrew Ranck
// SPDX-License-Identifier: CC0-1.0
// Test for Issue#5358: Support default value on module input.
// This test is not expected to fail. There are 3 DUTs using various defaulted (and not) input values,
// with expected checks over a few cycles.
module dut_default_input0
(
input logic required_input,
input logic i = (1'b0 && 1'b0), // 0
output logic o
);
assign o = i;
endmodule
module dut_default_input1
(
input logic i = 1'b1,
input logic required_input,
output logic o
);
assign o = i;
endmodule
module dut_default_input_logic32
#(
parameter bit [31:0] DefaultValueI = 32'h12345678
)
(
input logic [31:0] i = DefaultValueI,
output logic [31:0] o
);
assign o = i;
endmodule
module t
(/*AUTOARG*/
// Inputs
clk
);
input clk;
int cyc = 0;
wire logic1 = 1'b1;
function automatic logic logic0_from_some_function();
return 1'b0;
endfunction : logic0_from_some_function
// 1800-2009, a few flavors to test:
// 1. Port omitted from port list on instance (uses default value, NOT implicit net)
// 2. Port included on instance and left open (uses default value)
// 3. Port included on instance and overridden.
// 1. DUT instances with default values and port omitted
// instance names are u_dut*_default
logic dut0_o_default;
dut_default_input0 u_dut0_default
(.required_input(1),
/*.i(),*/
.o(dut0_o_default));
logic dut1_o_default;
dut_default_input1 u_dut1_default
(/*.i(),*/
.o(dut1_o_default),
.required_input(1));
logic [31:0] dut_logic32_o_default;
dut_default_input_logic32 u_dut_logic32_default
(/*.i(),*/
.o(dut_logic32_o_default));
// 2. DUT instances with default values and port open
// instance names are u_dut*_open
logic dut0_o_open;
dut_default_input0 u_dut0_open
(.required_input(1),
.i(), // open
.o(dut0_o_open));
logic dut1_o_open;
dut_default_input1 u_dut1_open
(.i(), // open
.o(dut1_o_open),
.required_input(1));
logic [31:0] dut_logic32_o_open;
dut_default_input_logic32 u_dut_logic32_open
(.i(), // open
.o(dut_logic32_o_open));
// 3. DUT instances with overriden values
// instance names are u_dut*_overriden
// Have u_dut0_overriden get its overriden value from a signal
logic dut0_o_overriden;
dut_default_input0 u_dut0_overriden
(.required_input(1),
.i(logic1), // from wire
.o(dut0_o_overriden));
// Have u_dut1_overriden get its overriden value from a function.
logic dut1_o_overriden;
dut_default_input1 u_dut1_overriden
(.i(logic0_from_some_function()), // from function
.o(dut1_o_overriden),
.required_input(1));
logic [31:0] dut_logic32_o_overriden;
logic [31:0] dut_logic32_want_overriden;
dut_default_input_logic32
#(.DefaultValueI(32'h2222_3333) // dontcare, we're overriding on input
)
u_dut_logic32_overriden
(.i(32'h6789_2345 + 32'(cyc)), // from inline logic
.o(dut_logic32_o_overriden));
assign dut_logic32_want_overriden = 32'h6789_2345 + 32'(cyc); // expected value i --> o
always @(posedge clk) begin : main
cyc <= cyc + 1;
if (cyc > 2) begin
// check these for a few cycles to make sure it's constant
$display("%t %m: outputs - defaults got {%0d %0d %0x}, want {0 1 12345678}",
$time,
dut0_o_default, dut1_o_default, dut_logic32_o_default);
if (dut0_o_default != 0) $error;
if (dut1_o_default != 1) $error;
if (dut_logic32_o_default != 32'h1234_5678) $error;
$display("%t %m: outputs - open got {%0d %0d %0x}, want {0 1 12345678}",
$time,
dut0_o_open, dut1_o_open, dut_logic32_o_open);
if (dut0_o_open != 0) $error;
if (dut1_o_open != 1) $error;
if (dut_logic32_o_open != 32'h1234_5678) $error;
// despite the port map override. At least the parameter goes through?
$display("%t %m: outputs - overrides got {%0d %0d %0x} want {1 0 %0x}",
$time,
dut0_o_overriden, dut1_o_overriden, dut_logic32_o_overriden,
dut_logic32_want_overriden);
if (dut0_o_overriden != 1) $error;
if (dut1_o_overriden != 0) $error;
if (dut_logic32_o_overriden != dut_logic32_want_overriden) $error;
end
if (cyc == 10) begin
// done checking various DUTs and finish
$display("%t %m: cyc=%0d", $time, cyc);
$write("*-* All Finished *-*\n");
$finish();
end
end
endmodule : t

View File

@ -0,0 +1,6 @@
%Error-ASSIGNIN: t/t_module_input_default_value_1_bad.v:16:10: Assigning to input/const variable: 'i'
: ... note: In instance 't.u_dut_should_fail_compile1'
16 | assign i = 1'b0;
| ^
... For error description see https://verilator.org/warn/ASSIGNIN?v=latest
%Error: Exiting due to

View File

@ -2,7 +2,7 @@
if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2008 by Wilson Snyder. This program is free software; you
# 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.
@ -10,7 +10,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di
scenarios(vlt => 1);
lint(
compile(
fails => 1,
expect_filename => $Self->{golden_filename},
);

View File

@ -0,0 +1,51 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty, 2024 by Andrew Ranck
// SPDX-License-Identifier: CC0-1.0
// Test for Issue#5358: Support default value on module input.
// This test *is* expected to not compile, and must match .out file.
module dut_should_fail_compile1
(
input logic i = 1'b1,
output logic o
);
assign i = 1'b0; // bad, should fail post link in V3Width
assign o = i;
endmodule
module t
(/*AUTOARG*/
// Inputs
clk
);
input clk;
int cyc = 0;
// 1800-2009, a few flavors to test:
// We should have some DUT instances that fail to compile,
// if you tried having a default value on port output.
logic dut_should_fail_o;
dut_should_fail_compile1 u_dut_should_fail_compile1
(.i(1'b0),
.o(dut_should_fail_o)
);
always @(posedge clk) begin : main
cyc <= cyc + 1;
if (cyc == 10) begin
// done checking various DUTs and finish
$display("%t %m: cyc=%0d", $time, cyc);
$write("*-* All Finished *-*\n");
$finish();
end
end
endmodule : t

View File

@ -0,0 +1,6 @@
%Error-ASSIGNIN: t/t_module_input_default_value_2_bad.v:17:5: Assigning to input/const variable: 'i'
: ... note: In instance 't.u_dut_should_fail_compile1'
17 | i = 1'b0;
| ^
... For error description see https://verilator.org/warn/ASSIGNIN?v=latest
%Error: Exiting due to

View File

@ -0,0 +1,19 @@
#!/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(vlt => 1);
compile(
fails => 1,
expect_filename => $Self->{golden_filename},
);
ok(1);
1;

View File

@ -0,0 +1,53 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty, 2024 by Andrew Ranck
// SPDX-License-Identifier: CC0-1.0
// Test for Issue#5358: Support default value on module input.
// This test *is* expected to not compile, and must match .out file.
module dut_should_fail_compile2
(
input logic i = 1'b1,
output logic o
);
always_comb begin
i = 1'b0; // bad, should fail post link in V3Width
end
assign o = i;
endmodule
module t
(/*AUTOARG*/
// Inputs
clk
);
input clk;
int cyc = 0;
// 1800-2009, a few flavors to test:
// We should have some DUT instances that fail to compile,
// if you tried having a default value on port output.
logic dut_should_fail_o;
dut_should_fail_compile2 u_dut_should_fail_compile1
(.i(1'b0),
.o(dut_should_fail_o)
);
always @(posedge clk) begin : main
cyc <= cyc + 1;
if (cyc == 10) begin
// done checking various DUTs and finish
$display("%t %m: cyc=%0d", $time, cyc);
$write("*-* All Finished *-*\n");
$finish();
end
end
endmodule : t

View File

@ -0,0 +1,6 @@
%Error-ASSIGNIN: t/t_module_input_default_value_3_bad.v:16:11: Assigning to input/const variable: 'i'
: ... note: In instance 't.u_dut_should_fail_compile1'
16 | initial i = 1'b0;
| ^
... For error description see https://verilator.org/warn/ASSIGNIN?v=latest
%Error: Exiting due to

View File

@ -0,0 +1,19 @@
#!/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(vlt => 1);
compile(
fails => 1,
expect_filename => $Self->{golden_filename},
);
ok(1);
1;

View File

@ -0,0 +1,51 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty, 2024 by Andrew Ranck
// SPDX-License-Identifier: CC0-1.0
// Test for Issue#5358: Support default value on module input.
// This test *is* expected to not compile, and must match .out file.
module dut_should_fail_compile1
(
input logic i = 1'b1,
output logic o
);
initial i = 1'b0; // bad, should fail post link in V3Width
assign o = i;
endmodule
module t
(/*AUTOARG*/
// Inputs
clk
);
input clk;
int cyc = 0;
// 1800-2009, a few flavors to test:
// We should have some DUT instances that fail to compile,
// if you tried having a default value on port output.
logic dut_should_fail_o;
dut_should_fail_compile1 u_dut_should_fail_compile1
(.i(1'b0),
.o(dut_should_fail_o)
);
always @(posedge clk) begin : main
cyc <= cyc + 1;
if (cyc == 10) begin
// done checking various DUTs and finish
$display("%t %m: cyc=%0d", $time, cyc);
$write("*-* All Finished *-*\n");
$finish();
end
end
endmodule : t

View File

@ -0,0 +1,24 @@
#!/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);
top_filename("t/t_module_input_default_value.v");
compile(
v_flags2 => ["-fno-inline"],
);
execute(
check_finished => 1,
);
ok(1);
1;

View File

@ -1,8 +1,10 @@
%Error-ASSIGNIN: t/t_var_in_assign_bad.v:12:16: Assigning to input/const variable: 'value'
12 | assign value = 4'h0;
| ^~~~~
... For error description see https://verilator.org/warn/ASSIGNIN?v=latest
%Error-ASSIGNIN: t/t_var_in_assign_bad.v:21:16: Assigning to input/const variable: 'valueSub'
: ... note: In instance 't.sub'
21 | assign valueSub = 4'h0;
| ^~~~~~~~
... For error description see https://verilator.org/warn/ASSIGNIN?v=latest
%Error-ASSIGNIN: t/t_var_in_assign_bad.v:12:16: Assigning to input/const variable: 'value'
: ... note: In instance 't'
12 | assign value = 4'h0;
| ^~~~~
%Error: Exiting due to