diff --git a/Changes b/Changes index 533749ce8..492f904aa 100644 --- a/Changes +++ b/Changes @@ -18,6 +18,7 @@ Verilator 5.025 devel * Fix consecutive zero-delays (#5038). [Krzysztof Bieganski, Antmicro Ltd.] * Fix `$system` with string argument (#5042). * Fix width extension on delays (#5043). +* Fix false ASSIGNIN on functions with explicit port map (#5069). Verilator 5.024 2024-04-05 diff --git a/src/V3LinkLValue.cpp b/src/V3LinkLValue.cpp index c92aa9956..4f2b9ce18 100644 --- a/src/V3LinkLValue.cpp +++ b/src/V3LinkLValue.cpp @@ -22,6 +22,8 @@ #include "V3LinkLValue.h" +#include "V3Task.h" + VL_DEFINE_DEBUG_FUNCTIONS; //###################################################################### @@ -279,23 +281,23 @@ class LinkLValueVisitor final : public VNVisitor { iterateChildren(nodep); } void visit(AstNodeFTaskRef* nodep) override { - AstNode* pinp = nodep->pinsp(); const AstNodeFTask* const taskp = nodep->taskp(); // We'll deal with mismatching pins later if (!taskp) return; - for (AstNode* stmtp = taskp->stmtsp(); stmtp && pinp; stmtp = stmtp->nextp()) { - if (const AstVar* const portp = VN_CAST(stmtp, Var)) { - if (portp->isIO()) { - if (portp->isWritable()) { - m_setRefLvalue = VAccess::WRITE; - iterate(pinp); - m_setRefLvalue = VAccess::NOCHANGE; - } else { - iterate(pinp); - } - // Advance pin - pinp = pinp->nextp(); - } + const V3TaskConnects tconnects + = V3Task::taskConnects(nodep, taskp->stmtsp(), nullptr, false); + for (const auto& tconnect : tconnects) { + const AstVar* const portp = tconnect.first; + const AstArg* const argp = tconnect.second; + if (!argp) continue; + AstNodeExpr* const pinp = argp->exprp(); + if (!pinp) continue; + if (portp->isWritable()) { + m_setRefLvalue = VAccess::WRITE; + iterate(pinp); + m_setRefLvalue = VAccess::NOCHANGE; + } else { + iterate(pinp); } } } diff --git a/src/V3Task.cpp b/src/V3Task.cpp index 8c26a75ff..8f692fa2f 100644 --- a/src/V3Task.cpp +++ b/src/V3Task.cpp @@ -1589,14 +1589,13 @@ public: const char* const V3Task::s_dpiTemporaryVarSuffix = "__Vcvt"; V3TaskConnects V3Task::taskConnects(AstNodeFTaskRef* nodep, AstNode* taskStmtsp, - V3TaskConnectState* statep) { + V3TaskConnectState* statep, bool makeChanges) { // Output list will be in order of the port declaration variables (so // func calls are made right in C) // Missing pin/expr? We return (pinvar, nullptr) // Extra pin/expr? We clean it up UINFO(9, "taskConnects " << nodep << endl); std::map nameToIndex; - std::set argWrap; // Which ports are defaulted, forcing arg wrapper creation V3TaskConnects tconnects; UASSERT_OBJ(nodep->taskp(), nodep, "unlinked"); @@ -1608,7 +1607,7 @@ V3TaskConnects V3Task::taskConnects(AstNodeFTaskRef* nodep, AstNode* taskStmtsp, if (portp->isIO()) { tconnects.emplace_back(portp, static_cast(nullptr)); nameToIndex.emplace(portp->name(), tpinnum); // For name based connections - tpinnum++; + ++tpinnum; if (portp->attrSFormat()) { sformatp = portp; } else if (sformatp) { @@ -1630,17 +1629,19 @@ V3TaskConnects V3Task::taskConnects(AstNodeFTaskRef* nodep, AstNode* taskStmtsp, // By name const auto it = nameToIndex.find(argp->name()); if (it == nameToIndex.end()) { - pinp->v3error("No such argument " << argp->prettyNameQ() << " in function call to " - << nodep->taskp()->prettyTypeName()); - // We'll just delete it; seems less error prone than making a false argument - VL_DO_DANGLING(pinp->unlinkFrBack()->deleteTree(), pinp); + if (makeChanges) { + pinp->v3error("No such argument " << argp->prettyNameQ() + << " in function call to " + << nodep->taskp()->prettyTypeName()); + // We'll just delete it; seems less error prone than making a false argument + VL_DO_DANGLING(pinp->unlinkFrBack()->deleteTree(), pinp); + } } else { - if (tconnects[it->second].second) { + if (tconnects[it->second].second && makeChanges) { pinp->v3error("Duplicate argument " << argp->prettyNameQ() << " in function call to " << nodep->taskp()->prettyTypeName()); } - argp->name(""); // Can forget name as will add back in pin order tconnects[it->second].second = argp; reorganize = true; } @@ -1649,8 +1650,8 @@ V3TaskConnects V3Task::taskConnects(AstNodeFTaskRef* nodep, AstNode* taskStmtsp, if (sformatp) { tconnects.emplace_back(sformatp, static_cast(nullptr)); tconnects[ppinnum].second = argp; - tpinnum++; - } else { + ++tpinnum; + } else if (makeChanges) { pinp->v3error("Too many arguments in function call to " << nodep->taskp()->prettyTypeName()); // We'll just delete it; seems less error prone than making a false argument @@ -1660,10 +1661,13 @@ V3TaskConnects V3Task::taskConnects(AstNodeFTaskRef* nodep, AstNode* taskStmtsp, tconnects[ppinnum].second = argp; } } - ppinnum++; + ++ppinnum; } + if (!makeChanges) return tconnects; + // Connect missing ones + std::set argWrap; // Which ports are defaulted, forcing arg wrapper creation for (int i = 0; i < tpinnum; ++i) { AstVar* const portp = tconnects[i].first; if (!tconnects[i].second || !tconnects[i].second->exprp()) { @@ -1723,6 +1727,11 @@ V3TaskConnects V3Task::taskConnects(AstNodeFTaskRef* nodep, AstNode* taskStmtsp, } } + for (const auto& tconnect : tconnects) { + AstArg* const argp = tconnect.second; + argp->name(""); // Can forget name as will add back in pin order + } + if (reorganize) { // To simplify downstream, put argument list back into pure pinnumber ordering while (nodep->pinsp()) { diff --git a/src/V3Task.h b/src/V3Task.h index f3f7cfacd..f2fa4cf83 100644 --- a/src/V3Task.h +++ b/src/V3Task.h @@ -55,7 +55,8 @@ public: static void taskAll(AstNetlist* nodep) VL_MT_DISABLED; /// Return vector of [port, pin-connects-to] (SLOW) static V3TaskConnects taskConnects(AstNodeFTaskRef* nodep, AstNode* taskStmtsp, - V3TaskConnectState* statep = nullptr) VL_MT_DISABLED; + V3TaskConnectState* statep = nullptr, + bool makeChanges = true) VL_MT_DISABLED; static void taskConnectWrap(AstNodeFTaskRef* nodep, const V3TaskConnects& tconnects, V3TaskConnectState* statep, const std::set& argWrap) VL_MT_DISABLED; diff --git a/test_regress/t/t_func_io_order.pl b/test_regress/t/t_func_io_order.pl new file mode 100755 index 000000000..e64ab41be --- /dev/null +++ b/test_regress/t/t_func_io_order.pl @@ -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 2024 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; diff --git a/test_regress/t/t_func_io_order.v b/test_regress/t/t_func_io_order.v new file mode 100644 index 000000000..97c5d514d --- /dev/null +++ b/test_regress/t/t_func_io_order.v @@ -0,0 +1,75 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2024 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +module t(/*AUTOARG*/ + // Inputs + clk + ); + input clk; + + integer cyc = 0; + reg [63:0] crc; + reg [63:0] sum; + + // Take CRC data and apply to testblock inputs + wire ain = crc[0]; + + /*AUTOWIRE*/ + // Beginning of automatic wires (for undeclared instantiated-module outputs) + logic bout; // From test of Test.v + // End of automatics + + Test test(/*AUTOINST*/ + // Outputs + .bout (bout), + // Inputs + .ain (ain)); + + // Test loop + always @ (posedge clk) begin +`ifdef TEST_VERBOSE + $write("[%0t] cyc==%0d crc=%x result=%x\n", $time, cyc, crc, result); +`endif + cyc <= cyc + 1; + crc <= {crc[62:0], crc[63] ^ crc[2] ^ crc[0]}; + if (cyc == 0) begin + // Setup + crc <= 64'h5aef0c8d_d70a4497; + end + else if (cyc < 10) begin + if (bout != ~ain) $stop; + end + else if (cyc == 99) begin + $write("[%0t] cyc==%0d crc=%x sum=%x\n", $time, cyc, crc, sum); + if (crc !== 64'hc77bb9b3784ea091) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end + end + +endmodule + +module Test(/*AUTOARG*/ + // Outputs + bout, + // Inputs + ain + ); + + input logic ain; + output logic bout; + + function automatic void inv + (input logic w_in, + output logic w_out); + w_out = ~w_in; + endfunction + + always_comb + inv(.w_out(bout), + .w_in(ain)); + +endmodule