From ad2929dff0956787f3190b47acd7268699f7096c Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 30 Oct 2018 20:50:09 -0400 Subject: [PATCH] Support "ref" and "const ref" pins and functions, bug1360. --- Changes | 2 + src/V3Ast.h | 19 +++--- src/V3AstNodes.cpp | 69 ++++++++++++-------- src/V3LinkLevel.cpp | 4 ++ src/V3Width.cpp | 26 ++++++-- src/verilog.y | 8 +-- test_regress/t/t_var_ref.pl | 20 ++++++ test_regress/t/t_var_ref.v | 94 ++++++++++++++++++++++++++++ test_regress/t/t_var_ref_bad1.pl | 24 +++++++ test_regress/t/t_var_ref_bad1.v | 18 ++++++ test_regress/t/t_var_ref_bad2.pl | 25 ++++++++ test_regress/t/t_var_ref_bad2.v | 24 +++++++ test_regress/t/t_var_ref_bad3.pl | 24 +++++++ test_regress/t/t_var_ref_bad3.v | 11 ++++ test_regress/t/t_var_ref_noinline.pl | 23 +++++++ 15 files changed, 350 insertions(+), 41 deletions(-) create mode 100755 test_regress/t/t_var_ref.pl create mode 100644 test_regress/t/t_var_ref.v create mode 100755 test_regress/t/t_var_ref_bad1.pl create mode 100644 test_regress/t/t_var_ref_bad1.v create mode 100755 test_regress/t/t_var_ref_bad2.pl create mode 100644 test_regress/t/t_var_ref_bad2.v create mode 100755 test_regress/t/t_var_ref_bad3.pl create mode 100644 test_regress/t/t_var_ref_bad3.v create mode 100755 test_regress/t/t_var_ref_noinline.pl diff --git a/Changes b/Changes index f94b253f8..42ff6354a 100644 --- a/Changes +++ b/Changes @@ -5,6 +5,8 @@ The contributors that suggested a given feature are shown in []. Thanks! * Verilator 4.007 devel +*** Support "ref" and "const ref" pins and functions, bug1360. [Jake Longo] + *** In --xml-only show the original unmodified names, msg2716. [Kanad Kanhere] **** Fix --trace-lxt2 compile error on MinGW, msg2711. [HyungKi Jeong] diff --git a/src/V3Ast.h b/src/V3Ast.h index 119a9d00c..3254e4689 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -470,7 +470,9 @@ public: NONE, INPUT, OUTPUT, - INOUT + INOUT, + REF, + CONSTREF }; enum en m_e; inline VDirection() : m_e(NONE) {} @@ -480,23 +482,26 @@ public: operator en() const { return m_e; } const char* ascii() const { static const char* const names[] = { - "NONE", "INPUT", "OUTPUT", "INOUT"}; + "NONE", "INPUT", "OUTPUT", "INOUT", "REF", "CONSTREF"}; return names[m_e]; } string verilogKwd() const { static const char* const names[] = { - "", "input", "output", "inout"}; + "", "input", "output", "inout", "ref", "const ref"}; return names[m_e]; } string xmlKwd() const { // For historical reasons no "put" suffix static const char* const names[] = { - "", "in", "out", "inout"}; + "", "in", "out", "inout", "ref", "const ref"}; return names[m_e]; } string prettyName() const { return verilogKwd(); } 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 isNonOutput() const { return m_e == INPUT || m_e == INOUT; } - bool isReadOnly() const { return m_e == INPUT; } - bool isWritable() const { return m_e == OUTPUT || m_e == INOUT; } + bool isNonOutput() const { return m_e == INPUT || m_e == INOUT + || m_e == REF || m_e == CONSTREF; } + bool isReadOnly() const { return m_e == INPUT || m_e == CONSTREF; } + bool isWritable() const { return m_e == OUTPUT || m_e == INOUT + || m_e == REF; } + bool isRefOrConstRef() const { return m_e == REF || m_e == CONSTREF; } }; inline bool operator== (VDirection lhs, VDirection rhs) { return (lhs.m_e == rhs.m_e); } inline bool operator== (VDirection lhs, VDirection::en rhs) { return (lhs.m_e == rhs); } diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 327f5f679..d5899664a 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -208,50 +208,68 @@ string AstVar::verilogKwd() const { string AstVar::vlArgType(bool named, bool forReturn, bool forFunc) const { if (forReturn) named=false; if (forReturn) v3fatalSrc("verilator internal data is never passed as return, but as first argument"); - string arg; - if (isWide() && isReadOnly()) arg += "const "; + string otype; AstBasicDType* bdtypep = basicp(); bool strtype = bdtypep && bdtypep->keyword()==AstBasicDTypeKwd::STRING; if (bdtypep && bdtypep->keyword()==AstBasicDTypeKwd::CHARPTR) { - arg += "const char*"; + otype += "const char*"; } else if (bdtypep && bdtypep->keyword()==AstBasicDTypeKwd::SCOPEPTR) { - arg += "const VerilatedScope*"; + otype += "const VerilatedScope*"; } else if (bdtypep && bdtypep->keyword()==AstBasicDTypeKwd::DOUBLE) { - arg += "double"; + if (forFunc && isReadOnly()) otype += "const "; + otype += "double"; } else if (bdtypep && bdtypep->keyword()==AstBasicDTypeKwd::FLOAT) { - arg += "float"; + if (forFunc && isReadOnly()) otype += "const "; + otype += "float"; } else if (strtype) { - if (isReadOnly()) arg += "const "; - arg += "std::string"; + if (forFunc && isReadOnly()) otype += "const "; + otype += "std::string"; } else if (widthMin() <= 8) { - arg += "CData"; + if (forFunc && isReadOnly()) otype += "const "; + otype += "CData"; } else if (widthMin() <= 16) { - arg += "SData"; + if (forFunc && isReadOnly()) otype += "const "; + otype += "SData"; } else if (widthMin() <= VL_WORDSIZE) { - arg += "IData"; + if (forFunc && isReadOnly()) otype += "const "; + otype += "IData"; } else if (isQuad()) { - arg += "QData"; + if (forFunc && isReadOnly()) otype += "const "; + otype += "QData"; } else if (isWide()) { - arg += "WData"; // []'s added later + if (forFunc && isReadOnly()) otype += "const "; + otype += "WData"; // []'s added later } - if (isDpiOpenArray() || (isWide() && !strtype)) { - arg += " (& "+name()+")"; + + bool mayparen = false; // Need paren, to handle building "(& name)[2]" + string oname; + if (isDpiOpenArray() + || (isWide() && !strtype) + || (forFunc && (isWritable() + || direction()==VDirection::REF + || direction()==VDirection::CONSTREF + || (strtype && isNonOutput())))) { + oname += "&"; + mayparen = true; + } + if (named) oname += " "+name(); + + string oarray; + if (isDpiOpenArray() || direction().isRefOrConstRef()) { for (AstNodeDType* dtp=dtypep(); dtp; ) { dtp = dtp->skipRefp(); // Skip AstRefDType/AstTypedef, or return same node if (AstUnpackArrayDType* adtypep = VN_CAST(dtp, UnpackArrayDType)) { - arg += "["+cvtToStr(adtypep->declRange().elements())+"]"; + if (mayparen) { oname = " ("+oname+")"; mayparen = false; } + oarray += "["+cvtToStr(adtypep->declRange().elements())+"]"; dtp = adtypep->subDTypep(); } else break; } - if (isWide() && !strtype) { - arg += "["+cvtToStr(widthWords())+"]"; - } - } else { - if (forFunc && (isWritable() - || (strtype && isNonOutput()))) arg += "&"; - if (named) arg += " "+name(); } - return arg; + if (isWide() && !strtype) { + if (mayparen) { oname = " ("+oname+")"; mayparen = false; } + oarray += "["+cvtToStr(widthWords())+"]"; + } + return otype+oname+oarray; } string AstVar::vlEnumType() const { @@ -342,7 +360,8 @@ string AstVar::cPubArgType(bool named, bool forReturn) const { arg += " (& "+name(); arg += ")["+cvtToStr(widthWords())+"]"; } else { - if (!forReturn && isWritable()) arg += "&"; + if (!forReturn && (isWritable() + || direction().isRefOrConstRef())) arg += "&"; if (named) arg += " "+name(); } return arg; diff --git a/src/V3LinkLevel.cpp b/src/V3LinkLevel.cpp index 03e0daa6e..217ad4904 100644 --- a/src/V3LinkLevel.cpp +++ b/src/V3LinkLevel.cpp @@ -130,6 +130,10 @@ void V3LinkLevel::wrapTopCell(AstNetlist* rootp) { oldvarp->primaryIO(false); varp->primaryIO(true); } + if (varp->direction().isRefOrConstRef()) { + varp->v3error("Unsupported: ref/const ref as primary input/output: " + <prettyName()); + } if (varp->isIO() && v3Global.opt.systemC()) { varp->sc(true); // User can see trace one level down from the wrapper diff --git a/src/V3Width.cpp b/src/V3Width.cpp index ac6fd06bb..219fa8b2b 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -1287,7 +1287,10 @@ private: if (!nodep->widthMin()) nodep->v3fatalSrc("LHS var should be size complete"); } //if (debug()>=9) nodep->dumpTree(cout," VRout "); - if (nodep->lvalue() && nodep->varp()->isConst() + if (nodep->lvalue() && nodep->varp()->direction() == VDirection::CONSTREF) { + nodep->v3error("Assigning to const ref variable: "<prettyName()); + } + else if (nodep->lvalue() && nodep->varp()->isConst() && !m_paramsOnly && !m_initialp) { // Too loose, but need to allow our generated first assignment // // Move this to a property of the AstInitial block @@ -2319,7 +2322,13 @@ private: } userIterateAndNext(nodep->exprp(), WidthVP(subDTypep,FINAL).p()); } else { - if (nodep->modVarp()->isTristate()) { + if (nodep->modVarp()->direction() == VDirection::REF) { + nodep->v3error("Ref connection '"<modVarp()->prettyName()<<"'" + <<" requires matching types;" + <<" ref requires "<prettyTypeName() + <<" but connection is " + <prettyTypeName()<<"."<modVarp()->isTristate()) { if (pinwidth != conwidth) { nodep->v3error("Unsupported: "<prettyOperatorName()) <<" to inout signal requires "<second; AstNode* pinp = argp->exprp(); if (!pinp) continue; // Argument error we'll find later - if (portp->isWritable() - && pinp->width() != portp->width()) { - pinp->v3error("Unsupported: Function output argument '"<prettyName()<<"'" + if (portp->direction() == VDirection::REF + && !similarDTypeRecurse(portp->dtypep(), pinp->dtypep())) { + pinp->v3error("Ref argument requires matching types;" + <<" port '"<prettyName()<<"'" + <<" requires "<prettyTypeName() + <<" but connection is "<prettyTypeName()<<"."); + } else if (portp->isWritable() + && pinp->width() != portp->width()) { + pinp->v3error("Unsupported: Function output argument '" + <prettyName()<<"'" <<" requires "<width() <<" bits, but connection's "<prettyTypeName() <<" generates "<width()<<" bits."); diff --git a/src/verilog.y b/src/verilog.y index 9cbbe813a..4c88d8692 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -1301,8 +1301,8 @@ port_direction: // ==IEEE: port_direction + tf_port_direction yINPUT { VARIO(INPUT); } | yOUTPUT { VARIO(OUTPUT); } | yINOUT { VARIO(INOUT); } - | yREF { $1->v3error("Unsupported: ref port"); VARIO(INOUT); } - | yCONST__REF yREF { $1->v3error("Unsupported: const ref port"); VARIO(INOUT); } + | yREF { VARIO(REF); } + | yCONST__REF yREF { VARIO(CONSTREF); } ; port_directionReset: // IEEE: port_direction that starts a port_declaraiton @@ -1310,8 +1310,8 @@ port_directionReset: // IEEE: port_direction that starts a port_declaraiton yINPUT { VARRESET_NONLIST(UNKNOWN); VARIO(INPUT); } | yOUTPUT { VARRESET_NONLIST(UNKNOWN); VARIO(OUTPUT); } | yINOUT { VARRESET_NONLIST(UNKNOWN); VARIO(INOUT); } - | yREF { $1->v3error("Unsupported: ref port"); VARRESET_NONLIST(UNKNOWN); VARIO(INOUT); } - | yCONST__REF yREF { $1->v3error("Unsupported: const ref port"); VARRESET_NONLIST(UNKNOWN); VARIO(INOUT); } + | yREF { VARRESET_NONLIST(UNKNOWN); VARIO(REF); } + | yCONST__REF yREF { VARRESET_NONLIST(UNKNOWN); VARIO(CONSTREF); } ; port_declaration: // ==IEEE: port_declaration diff --git a/test_regress/t/t_var_ref.pl b/test_regress/t/t_var_ref.pl new file mode 100755 index 000000000..89a4e77d9 --- /dev/null +++ b/test_regress/t/t_var_ref.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( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_var_ref.v b/test_regress/t/t_var_ref.v new file mode 100644 index 000000000..27c216e0e --- /dev/null +++ b/test_regress/t/t_var_ref.v @@ -0,0 +1,94 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2018 by Wilson Snyder. + +`define checkh(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__,`__LINE__, (gotv), (expv));; end while(0); + +module t(/*AUTOARG*/ + // Inputs + clk + ); + input clk; + + int cyc; + + int vr; + int va[2]; + +`ifdef T_NOINLINE + // verilator no_inline_module +`endif + + //==== + + task fun(ref int r, const ref int c); +`ifdef T_NOINLINE + // verilator no_inline_task +`endif + `checkh(c, 32'h1234); + r = 32'h4567; + endtask + + initial begin + int ci; + int ri; + ci = 32'h1234; + fun(ri, ci); + `checkh(ri, 32'h4567); + end + + //==== + + task fun_array(ref int af[2], const ref int cf[2]); +`ifdef T_NOINLINE + // verilator no_inline_task +`endif + `checkh(cf[0], 32'h1234); + `checkh(cf[1], 32'h2345); + af[0] = 32'h5678; + af[1] = 32'h6789; + endtask + // Not checkint - element of unpacked array + + initial begin + int ca[2]; + int ra[2]; + ca[0] = 32'h1234; + ca[1] = 32'h2345; + fun_array(ra, ca); + `checkh(ra[0], 32'h5678); + `checkh(ra[1], 32'h6789); + end + + //==== + + sub sub(.clk, .vr, .va); + + always @ (posedge clk) begin + cyc <= cyc + 1; + if (cyc == 0) begin + vr <= 32'h789; + va[0] <= 32'h89a; + va[1] <= 32'h9ab; + end + else if (cyc == 2) begin + `checkh(vr, 32'h987); + `checkh(va[0], 32'ha98); + `checkh(va[1], 32'ha9b); + $write("*-* All Finished *-*\n"); + $finish; + end + end + +endmodule + +module sub(input clk, ref int vr, ref int va[2]); + + always @(posedge clk) begin + vr <= 32'h987; + va[0] <= 32'ha98; + va[1] <= 32'ha9b; + end + +endmodule diff --git a/test_regress/t/t_var_ref_bad1.pl b/test_regress/t/t_var_ref_bad1.pl new file mode 100755 index 000000000..c9662e035 --- /dev/null +++ b/test_regress/t/t_var_ref_bad1.pl @@ -0,0 +1,24 @@ +#!/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( + v_flags2 => ["--lint-only"], + make_top_shell => 0, + make_main => 0, + verilator_make_gcc => 0, + fails => 1, + expect => +q{%Error: t/t_var_ref_bad1.v:\d+: Ref connection 'bad_sub_ref' requires matching types; ref requires BASICDTYPE 'real' but connection is BASICDTYPE 'bit'. +.*%Error: Exiting due to.*}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_var_ref_bad1.v b/test_regress/t/t_var_ref_bad1.v new file mode 100644 index 000000000..0d2258a7f --- /dev/null +++ b/test_regress/t/t_var_ref_bad1.v @@ -0,0 +1,18 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2018 by Wilson Snyder. + +// Make sure type errors aren't suppressable +// verilator lint_off WIDTH + +module t(/*AUTOARG*/); + + bit bad_parent; + sub sub + (.bad_sub_ref(bad_parent)); // Type mismatch + +endmodule + +module sub(ref real bad_sub_ref); +endmodule diff --git a/test_regress/t/t_var_ref_bad2.pl b/test_regress/t/t_var_ref_bad2.pl new file mode 100755 index 000000000..a256f2b17 --- /dev/null +++ b/test_regress/t/t_var_ref_bad2.pl @@ -0,0 +1,25 @@ +#!/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( + v_flags2 => ["--lint-only"], + make_top_shell => 0, + make_main => 0, + verilator_make_gcc => 0, + fails => 1, + expect => +q{%Error: t/t_var_ref_bad2.v:\d+: Assigning to const ref variable: bad_const_set +%Error: t/t_var_ref_bad2.v:\d+: Ref argument requires matching types; port 'int_ref' requires VAR 'int_ref' but connection is VARREF 'bad_non_int'. +.*%Error: Exiting due to.*}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_var_ref_bad2.v b/test_regress/t/t_var_ref_bad2.v new file mode 100644 index 000000000..e4d3dc17c --- /dev/null +++ b/test_regress/t/t_var_ref_bad2.v @@ -0,0 +1,24 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2018 by Wilson Snyder. + +// Make sure type errors aren't suppressable +// verilator lint_off WIDTH + +module t(/*AUTOARG*/); + + task checkset(const ref int bad_const_set); + bad_const_set = 32'h4567; // Bad setting const + endtask + + task checkset2(ref int int_ref); + endtask + + initial begin + int i; + byte bad_non_int; + checkset(i); + checkset2(bad_non_int); // Type mismatch + end +endmodule diff --git a/test_regress/t/t_var_ref_bad3.pl b/test_regress/t/t_var_ref_bad3.pl new file mode 100755 index 000000000..34b9a8a59 --- /dev/null +++ b/test_regress/t/t_var_ref_bad3.pl @@ -0,0 +1,24 @@ +#!/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( + v_flags2 => ["--lint-only"], + make_top_shell => 0, + make_main => 0, + verilator_make_gcc => 0, + fails => 1, + expect => +q{%Error: t/t_var_ref_bad3.v:\d+: Unsupported: ref/const ref as primary input/output: bad_primary_ref +.*%Error: Exiting due to.*}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_var_ref_bad3.v b/test_regress/t/t_var_ref_bad3.v new file mode 100644 index 000000000..5fbf8b325 --- /dev/null +++ b/test_regress/t/t_var_ref_bad3.v @@ -0,0 +1,11 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2018 by Wilson Snyder. + +// Make sure type errors aren't suppressable +// verilator lint_off WIDTH + +module t(ref int bad_primary_ref + /*AUTOARG*/); +endmodule diff --git a/test_regress/t/t_var_ref_noinline.pl b/test_regress/t/t_var_ref_noinline.pl new file mode 100755 index 000000000..6e9eff2fa --- /dev/null +++ b/test_regress/t/t_var_ref_noinline.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/t_var_ref.v"); + +scenarios(simulator => 1); + +compile( + v_flags2 => ['+define+T_NOINLINE'], + ); + +execute( + check_finished => 1, + ); + +ok(1); +1;