Skip to content

Commit

Permalink
Fix inout ports of unpacked stuct type (verilator#5000)
Browse files Browse the repository at this point in the history
  • Loading branch information
RRozak authored Mar 19, 2024
1 parent 290b313 commit 5c0a7e0
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 87 deletions.
20 changes: 6 additions & 14 deletions src/V3Tristate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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; });
Expand All @@ -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);

Expand Down
8 changes: 3 additions & 5 deletions src/V3Unknown.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 1 addition & 16 deletions test_regress/t/t_tri_array.out
Original file line number Diff line number Diff line change
Expand Up @@ -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
34 changes: 0 additions & 34 deletions test_regress/t/t_tri_struct.out

This file was deleted.

4 changes: 1 addition & 3 deletions test_regress/t/t_tri_struct.pl
Original file line number Diff line number Diff line change
Expand Up @@ -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;
16 changes: 1 addition & 15 deletions test_regress/t/t_tri_struct.v
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
2 changes: 2 additions & 0 deletions test_regress/t/t_tri_struct_packed.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
%Error: t/t_tri_struct_packed.v:25: Verilog $stop
Aborting...
23 changes: 23 additions & 0 deletions test_regress/t/t_tri_struct_packed.pl
Original file line number Diff line number Diff line change
@@ -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;
29 changes: 29 additions & 0 deletions test_regress/t/t_tri_struct_packed.v
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 5c0a7e0

Please sign in to comment.