From 5c0a7e06dc56c8e440a9178ae1535c87117906a6 Mon Sep 17 00:00:00 2001 From: Ryszard Rozak Date: Tue, 19 Mar 2024 11:43:06 +0100 Subject: [PATCH] Fix inout ports of unpacked stuct type (#5000) --- src/V3Tristate.cpp | 20 +++++---------- src/V3Unknown.cpp | 8 +++--- test_regress/t/t_tri_array.out | 17 +------------ test_regress/t/t_tri_struct.out | 34 -------------------------- test_regress/t/t_tri_struct.pl | 4 +-- test_regress/t/t_tri_struct.v | 16 +----------- test_regress/t/t_tri_struct_packed.out | 2 ++ test_regress/t/t_tri_struct_packed.pl | 23 +++++++++++++++++ test_regress/t/t_tri_struct_packed.v | 29 ++++++++++++++++++++++ 9 files changed, 66 insertions(+), 87 deletions(-) delete mode 100644 test_regress/t/t_tri_struct.out create mode 100644 test_regress/t/t_tri_struct_packed.out create mode 100755 test_regress/t/t_tri_struct_packed.pl create mode 100644 test_regress/t/t_tri_struct_packed.v diff --git a/src/V3Tristate.cpp b/src/V3Tristate.cpp index f0dcc3db09..455de671f8 100644 --- a/src/V3Tristate.cpp +++ b/src/V3Tristate.cpp @@ -660,20 +660,19 @@ class TristateVisitor final : public TristateBaseVisitor { for (auto it = beginStrength; it != endStrength; it++) { AstVarRef* refp = it->m_varrefp; - const int w = varp->width(); // create the new lhs driver for this var AstVar* const newLhsp = new AstVar{varp->fileline(), VVarType::MODULETEMP, varp->name() + "__out" + cvtToStr(m_unique), - VFlagBitPacked{}, w}; // 2-state ok; sep enable + varp}; // 2-state ok; sep enable UINFO(9, " newout " << newLhsp << endl); nodep->addStmtsp(newLhsp); refp->varp(newLhsp); // create a new var for this drivers enable signal - AstVar* const newEnLhsp = new AstVar{varp->fileline(), VVarType::MODULETEMP, - varp->name() + "__en" + cvtToStr(m_unique++), - VFlagBitPacked{}, w}; // 2-state ok + AstVar* const newEnLhsp + = new AstVar{varp->fileline(), VVarType::MODULETEMP, + varp->name() + "__en" + cvtToStr(m_unique++), envarp}; // 2-state ok UINFO(9, " newenlhsp " << newEnLhsp << endl); nodep->addStmtsp(newEnLhsp); @@ -710,12 +709,6 @@ class TristateVisitor final : public TristateBaseVisitor { ++m_statTriSigs; m_tgraph.didProcess(invarp); - const AstBasicDType* const basicp = VN_CAST(invarp->dtypep()->skipRefp(), BasicDType); - if (!basicp || basicp->isOpaque()) - invarp->v3warn(E_UNSUPPORTED, - "Unsupported: Inout/tristate with non-bit/logic data type: " - << invarp->dtypep()->prettyTypeName()); - // If the lhs var is a port, then we need to create ports for // the output (__out) and output enable (__en) signals. The // original port gets converted to an input. Don't tristate expand @@ -745,7 +738,6 @@ class TristateVisitor final : public TristateBaseVisitor { AstNodeExpr* orp = nullptr; AstNodeExpr* enp = nullptr; - const int w = lhsp->width(); std::sort(refsp->begin(), refsp->end(), [](RefStrength a, RefStrength b) { return a.m_strength > b.m_strength; }); @@ -762,13 +754,13 @@ class TristateVisitor final : public TristateBaseVisitor { // var__strength variable AstVar* varStrengthp = new AstVar{fl, VVarType::MODULETEMP, strengthVarName, - VFlagBitPacked{}, w}; // 2-state ok; sep enable; + invarp}; // 2-state ok; sep enable; UINFO(9, " newstrength " << varStrengthp << endl); nodep->addStmtsp(varStrengthp); // var__strength__en variable AstVar* enVarStrengthp = new AstVar{fl, VVarType::MODULETEMP, strengthVarName + "__en", - VFlagBitPacked{}, w}; // 2-state ok; + invarp}; // 2-state ok; UINFO(9, " newenstrength " << enVarStrengthp << endl); nodep->addStmtsp(enVarStrengthp); diff --git a/src/V3Unknown.cpp b/src/V3Unknown.cpp index b2efbe97e1..43617e72ca 100644 --- a/src/V3Unknown.cpp +++ b/src/V3Unknown.cpp @@ -439,11 +439,9 @@ class UnknownVisitor final : public VNVisitor { int declElements = -1; AstNodeDType* const dtypep = nodep->fromp()->dtypep()->skipRefp(); UASSERT_OBJ(dtypep, nodep, "Select of non-selectable type"); - if (const AstNodeArrayDType* const adtypep = VN_CAST(dtypep, NodeArrayDType)) { - declElements = adtypep->elementsConst(); - } else { - nodep->v3error("Select from non-array " << dtypep->prettyTypeName()); - } + const AstNodeArrayDType* const adtypep = VN_CAST(dtypep, NodeArrayDType); + UASSERT_OBJ(adtypep, nodep, "Select from non-array " << dtypep->prettyTypeName()); + declElements = adtypep->elementsConst(); if (debug() >= 9) nodep->dumpTree("- arraysel_old: "); // If value MODDIV constant, where constant <= declElements, known ok diff --git a/test_regress/t/t_tri_array.out b/test_regress/t/t_tri_array.out index 23d94a39bc..c18ab6ffb5 100644 --- a/test_regress/t/t_tri_array.out +++ b/test_regress/t/t_tri_array.out @@ -7,19 +7,4 @@ : ... note: In instance 't' 28 | Pad pad0 (.pad(pad[g]), | ^ -%Error-UNSUPPORTED: t/t_tri_array.v:19:17: Unsupported: Inout/tristate with non-bit/logic data type: UNPACKARRAYDTYPE - : ... note: In instance 't' - 19 | tri pad [NPAD-1:0]; - | ^~~ -%Error: t/t_tri_array.v:25:25: Select from non-array BASICDTYPE 'bit' - : ... note: In instance 't' - 25 | Pad pad1 (.pad(pad[g]), - | ^ -%Error: t/t_tri_array.v:28:25: Select from non-array BASICDTYPE 'bit' - : ... note: In instance 't' - 28 | Pad pad0 (.pad(pad[g]), - | ^ -%Error: Internal Error: t/t_tri_array.v:25:22: ../V3Dfg.h:#: DfgVertex is not of expected type, but instead has type 'VARPACKED' - 25 | Pad pad1 (.pad(pad[g]), - | ^~~ - ... See the manual at https://verilator.org/verilator_doc.html for more assistance. +%Error: Exiting due to diff --git a/test_regress/t/t_tri_struct.out b/test_regress/t/t_tri_struct.out deleted file mode 100644 index a8f607c639..0000000000 --- a/test_regress/t/t_tri_struct.out +++ /dev/null @@ -1,34 +0,0 @@ -%Error-UNSUPPORTED: t/t_tri_struct.v:20:31: Unsupported: Inout/tristate with non-bit/logic data type: REFDTYPE 'u_struct_t' - : ... note: In instance 't.u_mh' - 20 | module u_mh (inout u_struct_t u_i, inout u_struct_t u_o); - | ^~~ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest -%Error-UNSUPPORTED: t/t_tri_struct.v:20:53: Unsupported: Inout/tristate with non-bit/logic data type: REFDTYPE 'u_struct_t' - : ... note: In instance 't.u_mh' - 20 | module u_mh (inout u_struct_t u_i, inout u_struct_t u_o); - | ^~~ -%Error-UNSUPPORTED: t/t_tri_struct.v:15:31: Unsupported: Inout/tristate with non-bit/logic data type: REFDTYPE 'p_struct_t' - : ... note: In instance 't.p_mh' - 15 | module p_mh (inout p_struct_t p_i, inout p_struct_t p_o); - | ^~~ -%Error-UNSUPPORTED: t/t_tri_struct.v:15:53: Unsupported: Inout/tristate with non-bit/logic data type: REFDTYPE 'p_struct_t' - : ... note: In instance 't.p_mh' - 15 | module p_mh (inout p_struct_t p_i, inout p_struct_t p_o); - | ^~~ -%Error-UNSUPPORTED: t/t_tri_struct.v:26:15: Unsupported: Inout/tristate with non-bit/logic data type: REFDTYPE 'p_struct_t' - : ... note: In instance 't' - 26 | p_struct_t p_i, p_o; - | ^~~ -%Error-UNSUPPORTED: t/t_tri_struct.v:26:20: Unsupported: Inout/tristate with non-bit/logic data type: REFDTYPE 'p_struct_t' - : ... note: In instance 't' - 26 | p_struct_t p_i, p_o; - | ^~~ -%Error-UNSUPPORTED: t/t_tri_struct.v:27:15: Unsupported: Inout/tristate with non-bit/logic data type: REFDTYPE 'u_struct_t' - : ... note: In instance 't' - 27 | u_struct_t u_i, u_o; - | ^~~ -%Error-UNSUPPORTED: t/t_tri_struct.v:27:20: Unsupported: Inout/tristate with non-bit/logic data type: REFDTYPE 'u_struct_t' - : ... note: In instance 't' - 27 | u_struct_t u_i, u_o; - | ^~~ -%Error: Exiting due to diff --git a/test_regress/t/t_tri_struct.pl b/test_regress/t/t_tri_struct.pl index 376f009647..69c1098d47 100755 --- a/test_regress/t/t_tri_struct.pl +++ b/test_regress/t/t_tri_struct.pl @@ -12,13 +12,11 @@ compile( verilator_flags2 => ['--exe --main --timing'], - fails => $Self->{vlt_all}, - expect_filename => $Self->{golden_filename}, ); execute( check_finished => 1, - ) if !$Self->{vlt}; + ); ok(1); 1; diff --git a/test_regress/t/t_tri_struct.v b/test_regress/t/t_tri_struct.v index 2700e08436..e2aba4d5f7 100644 --- a/test_regress/t/t_tri_struct.v +++ b/test_regress/t/t_tri_struct.v @@ -1,39 +1,25 @@ // DESCRIPTION: Verilator: Verilog Test module // // This file ONLY is placed under the Creative Commons Public Domain, for -// any use, without warranty, 2024 by Ryszard Rozak. +// any use, without warranty, 2024 by Antmicro Ltd. // SPDX-License-Identifier: CC0-1.0 -typedef struct packed { - bit x; -} p_struct_t; - typedef struct { bit x; } u_struct_t; -module p_mh (inout p_struct_t p_i, inout p_struct_t p_o); - // OK: module p_mh (input p_struct_t p_i, output p_struct_t p_o); - assign p_o.x = p_i.x; -endmodule - module u_mh (inout u_struct_t u_i, inout u_struct_t u_o); - // OK: module u_mh (input u_struct_t u_i, output u_struct_t u_o); assign u_o.x = u_i.x; endmodule module t; - p_struct_t p_i, p_o; u_struct_t u_i, u_o; - p_mh p_mh(p_i, p_o); u_mh u_mh(u_i, u_o); initial begin - p_i.x = 1; u_i.x = 1; #1; - if (p_o.x != 1'b1) $stop; if (u_o.x != 1'b1) $stop; $write("*-* All Finished *-*\n"); $finish; diff --git a/test_regress/t/t_tri_struct_packed.out b/test_regress/t/t_tri_struct_packed.out new file mode 100644 index 0000000000..0d2704e4ba --- /dev/null +++ b/test_regress/t/t_tri_struct_packed.out @@ -0,0 +1,2 @@ +%Error: t/t_tri_struct_packed.v:25: Verilog $stop +Aborting... diff --git a/test_regress/t/t_tri_struct_packed.pl b/test_regress/t/t_tri_struct_packed.pl new file mode 100755 index 0000000000..680393eab2 --- /dev/null +++ b/test_regress/t/t_tri_struct_packed.pl @@ -0,0 +1,23 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2021 by Wilson Snyder. This program is free software; you +# can redistribute it and/or modify it under the terms of either the GNU +# Lesser General Public License Version 3 or the Perl Artistic License +# Version 2.0. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(simulator => 1); + +compile( + verilator_flags2 => ['--exe --main --timing'], + ); + +execute( + fails => $Self->{vlt_all}, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_tri_struct_packed.v b/test_regress/t/t_tri_struct_packed.v new file mode 100644 index 0000000000..df1f8aa6e4 --- /dev/null +++ b/test_regress/t/t_tri_struct_packed.v @@ -0,0 +1,29 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2024 by Antmicro Ltd. +// SPDX-License-Identifier: CC0-1.0 + +typedef struct packed { + bit x; +} p_struct_t; + +module p_mh (inout p_struct_t p_i, inout p_struct_t p_o); + // OK: module p_mh (input p_struct_t p_i, output p_struct_t p_o); + assign p_o.x = p_i.x; +endmodule + +module t; + p_struct_t p_i, p_o; + + p_mh p_mh(p_i, p_o); + + initial begin + p_i.x = 1; + #1; + // issue #4925 + if (p_o.x != 1'b1) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule