From 0c32113fc49b68be9eea386f43e67c3f7c374ea0 Mon Sep 17 00:00:00 2001 From: Yilou Wang Date: Thu, 18 Jun 2026 12:09:07 +0200 Subject: [PATCH] Fix cross-hierarchy tristate drivers of interface nets --- src/V3Tristate.cpp | 42 +++++++++++- test_regress/t/t_interface_inout_tristate.py | 18 +++++ test_regress/t/t_interface_inout_tristate.v | 62 +++++++++++++++++ .../t/t_interface_tristate_plain_xhier.py | 18 +++++ .../t/t_interface_tristate_plain_xhier.v | 68 +++++++++++++++++++ .../t_interface_tristate_plain_xhier_noinl.py | 19 ++++++ 6 files changed, 224 insertions(+), 3 deletions(-) create mode 100755 test_regress/t/t_interface_inout_tristate.py create mode 100644 test_regress/t/t_interface_inout_tristate.v create mode 100755 test_regress/t/t_interface_tristate_plain_xhier.py create mode 100644 test_regress/t/t_interface_tristate_plain_xhier.v create mode 100755 test_regress/t/t_interface_tristate_plain_xhier_noinl.py diff --git a/src/V3Tristate.cpp b/src/V3Tristate.cpp index 6fa174ed0..a0859c99c 100644 --- a/src/V3Tristate.cpp +++ b/src/V3Tristate.cpp @@ -352,7 +352,9 @@ class TristatePinVisitor final : public TristateBaseVisitor { TristateGraph& m_tgraph; const bool m_lvalue; // Flip to be an LVALUE // VISITORS - void visit(AstVarRef* nodep) override { + // AstNodeVarRef, not just AstVarRef: a cross-hierarchy pin expression into an + // interface (.pin(iface.net)) is an AstVarXRef and needs the same access flip. + void visit(AstNodeVarRef* nodep) override { UASSERT_OBJ(!nodep->access().isRW(), nodep, "Tristate unexpected on R/W access flip"); if (m_lvalue && !nodep->access().isWriteOrRW()) { UINFO(9, " Flip-to-LValue " << nodep); @@ -405,6 +407,11 @@ class TristateVisitor final : public TristateBaseVisitor { struct AuxAstVar final { AstPull* pullp = nullptr; // pullup/pulldown direction (whole variable) AstVar* outVarp = nullptr; // output __out var + bool ifaceTristate = false; // Interface var known to be tristate (set when the + // interface module is processed). Lets a module that + // drives this var across hierarchy with a plain (non-Z) + // assign be recognised as a tristate contributor even + // though that module's own graph has no Z on the net. std::unordered_map bitPulls; // Per-bit pull: bit_index -> direction (1=up, 0=down) }; @@ -780,7 +787,11 @@ class TristateVisitor final : public TristateBaseVisitor { } } else if (VN_IS(nodep, Iface) && !invarp->isIO()) { // Local driver in an interface module - use contribution mechanism - // so it can be combined with any external drivers later + // so it can be combined with any external drivers later. Record that + // this interface net is tristate, so a module that drives it across + // hierarchy with a plain (non-Z) assign is also routed through the + // contribution mechanism (its own graph has no Z to mark it tristate). + m_varAux(invarp).ifaceTristate = true; insertTristatesSignal(nodep, invarp, refsp, true, "", "", nullptr); } else { insertTristatesSignal(nodep, invarp, refsp, false, "", "", nullptr); @@ -2080,6 +2091,14 @@ class TristateVisitor final : public TristateBaseVisitor { if (m_graphing) { if (nodep->access().isWriteOrRW()) associateLogic(nodep, nodep->varp()); if (nodep->access().isReadOrRW()) associateLogic(nodep->varp(), nodep); + // A plain (non-Z) cross-hierarchy driver of an interface net that is tristate + // must be treated as tristate here too, so it is collected as a contribution + // and combined with the interface's own drivers (otherwise it is silently + // dropped and the net resolves using only the interface-internal Z driver). + if (nodep->access().isWriteOrRW() && VN_IS(nodep, VarXRef) + && m_varAux(nodep->varp()).ifaceTristate) { + m_tgraph.setTristate(nodep->varp()); + } } else { if (nodep->user2() & U2_NONGRAPH) return; // Processed nodep->user2Or(U2_NONGRAPH); @@ -2296,7 +2315,24 @@ public: // CONSTRUCTORS explicit TristateVisitor(AstNetlist* netlistp) { m_tgraph.clearAndCheck(); - iterateChildrenBackwardsConst(netlistp); + // Process interface modules before any other module, so that interface tristate + // nets are recorded (AuxAstVar::ifaceTristate) before the modules that drive them + // across hierarchy are processed. A module driving an interface tristate net with a + // plain (non-Z) assign has no Z in its own graph to mark the net tristate, so without + // this ordering its driver would be silently dropped. Only modulesp() carries tristate + // logic (filesp/miscsp do not), so restricting the walk to it drops nothing versus the + // historical iterateChildrenBackwardsConst(); iterate each group in reverse declaration + // order to match that order. + std::vector modps; + for (AstNode* modp = netlistp->modulesp(); modp; modp = modp->nextp()) { + modps.push_back(VN_AS(modp, NodeModule)); + } + for (auto it = modps.rbegin(); it != modps.rend(); ++it) { + if (VN_IS(*it, Iface)) iterate(*it); + } + for (auto it = modps.rbegin(); it != modps.rend(); ++it) { + if (!VN_IS(*it, Iface)) iterate(*it); + } // Combine interface tristate contributions after all modules processed combineIfaceContribs(); diff --git a/test_regress/t/t_interface_inout_tristate.py b/test_regress/t/t_interface_inout_tristate.py new file mode 100755 index 000000000..1ddad07d5 --- /dev/null +++ b/test_regress/t/t_interface_inout_tristate.py @@ -0,0 +1,18 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# 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-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('simulator') + +test.compile(timing_loop=True, verilator_flags2=['--timing']) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_interface_inout_tristate.v b/test_regress/t/t_interface_inout_tristate.v new file mode 100644 index 000000000..de376e1ee --- /dev/null +++ b/test_regress/t/t_interface_inout_tristate.v @@ -0,0 +1,62 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 PlanV GmbH +// SPDX-License-Identifier: CC0-1.0 + +// verilog_format: off +`define stop $stop +`define checkh(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0); +// verilog_format: on + +// verilator lint_off MULTIDRIVEN + +interface ifc; + // Tristate net resolved across an inout port connected hierarchically to + // this interface signal (iface.data <-> dut inout port). + wire [7:0] data; +endinterface + +module dut ( + inout wire [7:0] data, + input bit oe, + input logic [7:0] val +); + // The inout port drives the interface net when enabled, releases it (Z) otherwise. + assign data = oe ? val : 8'hzz; +endmodule + +module t; + ifc ifc0 (); + bit oe, top_oe; + logic [7:0] val, topval; + + // A second driver from the top, so resolution is observable from both sides. + assign ifc0.data = top_oe ? topval : 8'hzz; + + dut u ( + .data(ifc0.data), + .oe(oe), + .val(val) + ); + + initial begin + // dut drives the interface net through the inout port. + oe = 1; + val = 8'hA5; + top_oe = 0; + topval = 8'h00; + #1 `checkh(ifc0.data, 8'hA5); + // top drives, dut releases. + oe = 0; + top_oe = 1; + topval = 8'h3C; + #1 `checkh(ifc0.data, 8'h3C); + // Neither drives: net floats to Z. + oe = 0; + top_oe = 0; + #1 `checkh(ifc0.data, 8'hzz); + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_interface_tristate_plain_xhier.py b/test_regress/t/t_interface_tristate_plain_xhier.py new file mode 100755 index 000000000..1ddad07d5 --- /dev/null +++ b/test_regress/t/t_interface_tristate_plain_xhier.py @@ -0,0 +1,18 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# 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-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('simulator') + +test.compile(timing_loop=True, verilator_flags2=['--timing']) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_interface_tristate_plain_xhier.v b/test_regress/t/t_interface_tristate_plain_xhier.v new file mode 100644 index 000000000..3f925d976 --- /dev/null +++ b/test_regress/t/t_interface_tristate_plain_xhier.v @@ -0,0 +1,68 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 PlanV GmbH +// SPDX-License-Identifier: CC0-1.0 + +// verilog_format: off +`define stop $stop +`define checkh(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0); +// verilog_format: on + +// verilator lint_off MULTIDRIVEN + +interface ifc; + wire [1:0] w; + // Self-contained interface-internal tristate driver: 'z while en==0, so any + // external driver must win the net resolution. + bit en = 0; + logic [1:0] wint; + assign wint = 2'b11; + assign w = en ? wint : 2'bzz; + // Read the resolved net from inside the interface (a consumer's view), so the + // check sees the true resolution, not the driving module's own local copy. + function automatic logic [1:0] get_w(); + return w; + endfunction +endinterface + +module driver ( + ifc io, + input logic [1:0] din +); + // Plain (non-Z) cross-hierarchy driver of an interface tristate net. + // This module's own tristate graph has no 'z on io.w, so before the fix this + // contribution was silently dropped and io.w resolved to the interface-internal + // 'z (== 0) only. + assign io.w = din; +endmodule + +module t; + // u_a, u_b: interface nets driven plainly from the top module (depth-0 xref). + // u_c: interface net driven plainly from a child module (depth-1 xref). + ifc u_a (); + ifc u_b (); + ifc u_c (); + + logic [1:0] va, vb, vc; + assign va = 2'b01; + assign vb = 2'b10; + assign vc = 2'b11; + + assign u_a.w = va; // plain top-level driver + assign u_b.w = vb; // plain top-level driver + driver u_drv ( + .io(u_c), + .din(vc) + ); // plain driver in a child module + + initial begin + #1; + // The plain external driver must win over the interface-internal 'z. + `checkh(u_a.get_w(), 2'b01); + `checkh(u_b.get_w(), 2'b10); + `checkh(u_c.get_w(), 2'b11); + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_interface_tristate_plain_xhier_noinl.py b/test_regress/t/t_interface_tristate_plain_xhier_noinl.py new file mode 100755 index 000000000..12aff0c40 --- /dev/null +++ b/test_regress/t/t_interface_tristate_plain_xhier_noinl.py @@ -0,0 +1,19 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# 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-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('simulator') +test.top_filename = "t_interface_tristate_plain_xhier.v" + +test.compile(timing_loop=True, verilator_flags2=['--timing', '-fno-inline']) + +test.execute() + +test.passes()