Fix cross-hierarchy tristate drivers of interface nets (#7339) (#7801)

Closes #7339.
This commit is contained in:
Yilou Wang 2026-06-19 01:07:37 +02:00 committed by GitHub
parent 51022d6152
commit 129cfc19c0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 270 additions and 2 deletions

View File

@ -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<int, int>
bitPulls; // Per-bit pull: bit_index -> direction (1=up, 0=down)
};
@ -439,6 +446,7 @@ class TristateVisitor final : public TristateBaseVisitor {
int m_unique = 0;
bool m_alhs = false; // On LHS of assignment
bool m_inAlias = false; // Inside alias statement
bool m_processedIfaces = false; // Interface modules already processed (interfaces-first pass)
VStrength m_currentStrength = VStrength::STRONG; // Current strength of assignment,
// Used only on LHS of assignment
const AstNode* m_logicp = nullptr; // Current logic being built
@ -755,6 +763,9 @@ class TristateVisitor final : public TristateBaseVisitor {
}
}
} else if (isIfaceTri) {
// Mark here too, so a net made tristate only by an external 'z driver
// still captures a plain cross-hierarchy driver processed later.
m_varAux(invarp).ifaceTristate = true;
// Interface tristate vars: drivers from different interface instances
// (different VarXRef dotted paths) must be processed separately.
// E.g. io_ifc.d and io_ifc_local.d both target the same AstVar d in
@ -780,7 +791,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 +2095,15 @@ class TristateVisitor final : public TristateBaseVisitor {
if (m_graphing) {
if (nodep->access().isWriteOrRW()) associateLogic(nodep, nodep->varp());
if (nodep->access().isReadOrRW()) associateLogic(nodep->varp(), nodep);
// Only interface tristate nets need this: a plain (non-Z) cross-hierarchy
// driver has no Z in its own module's graph, so mark the net tristate here to
// collect the driver as a contribution. The ifaceTristate flag is only set for
// interface nets; a non-interface cross-module tri driver does nothing here and
// is instead rejected (E_UNSUPPORTED) later in insertTristates.
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);
@ -2160,6 +2184,9 @@ class TristateVisitor final : public TristateBaseVisitor {
}
void visit(AstNodeModule* nodep) override {
// Interfaces are processed first in the constructor; skip the duplicate visit
// during the later iterateChildrenBackwardsConst() pass over all modules.
if (m_processedIfaces && VN_IS(nodep, Iface)) return;
UINFO(8, dbgState() << nodep);
VL_RESTORER(m_modp);
VL_RESTORER(m_graphing);
@ -2296,6 +2323,18 @@ public:
// CONSTRUCTORS
explicit TristateVisitor(AstNetlist* netlistp) {
m_tgraph.clearAndCheck();
// Process interface modules first, so an interface tristate net is recorded
// (AuxAstVar::ifaceTristate) before any module that drives it across hierarchy is
// graphed. A module driving such a net with a plain (non-Z) assign has no Z in its
// own graph to mark the net tristate, so without this its driver would be dropped.
// Collect only the interfaces (reverse declaration order to match the pass below).
std::vector<AstNodeModule*> ifacesp;
for (AstNode* modp = netlistp->modulesp(); modp; modp = modp->nextp()) {
if (VN_IS(modp, Iface)) ifacesp.push_back(VN_AS(modp, NodeModule));
}
for (auto it = ifacesp.rbegin(); it != ifacesp.rend(); ++it) iterate(*it);
m_processedIfaces = true;
// Then the rest in the historical order; interfaces are skipped (already done).
iterateChildrenBackwardsConst(netlistp);
// Combine interface tristate contributions after all modules processed

View File

@ -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()

View File

@ -0,0 +1,70 @@
// 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);
`ifdef verilator
`define no_optimize(v) $c(v)
`else
`define no_optimize(v) (v)
`endif
// 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 = `no_optimize(1'b1);
val = `no_optimize(8'hA5);
top_oe = `no_optimize(1'b0);
topval = `no_optimize(8'h00);
#1;
`checkh(ifc0.data, 8'hA5);
// top drives, dut releases.
oe = `no_optimize(1'b0);
top_oe = `no_optimize(1'b1);
topval = `no_optimize(8'h3C);
#1;
`checkh(ifc0.data, 8'h3C);
// Neither drives: net floats to Z.
oe = `no_optimize(1'b0);
top_oe = `no_optimize(1'b0);
#1;
`checkh(ifc0.data, 8'hzz);
$write("*-* All Finished *-*\n");
$finish;
end
endmodule

View File

@ -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()

View File

@ -0,0 +1,104 @@
// 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);
`ifdef verilator
`define no_optimize(v) $c(v)
`else
`define no_optimize(v) (v)
`endif
// 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. no_optimize keeps the driver
// values from being constant-folded, so the resolution runs at run time.
logic en;
logic [1:0] wint;
assign en = `no_optimize(1'b0);
assign wint = `no_optimize(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
// Interface net made tristate only by an external 'z driver (no internal driver).
interface ifc_tri;
tri [1:0] w;
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 driver_tri (
ifc_tri io,
input logic [1:0] din
);
assign io.w = din;
endmodule
module zdriver (
ifc_tri io
);
assign io.w = 2'bzz;
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 ();
// u_e: net is tristate only via zdriver's external 'z; the plain driver must
// still win (the interface itself has no internal driver).
ifc_tri u_e ();
logic [1:0] va, vb, vc, ve;
assign va = `no_optimize(2'b01);
assign vb = `no_optimize(2'b10);
assign vc = `no_optimize(2'b11);
assign ve = `no_optimize(2'b01);
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
driver_tri u_pdrv (
.io(u_e),
.din(ve)
); // plain driver of an externally-tristated net
zdriver u_zdrv (.io(u_e)); // external 'z driver
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);
// The plain driver must win over an external-only 'z (no internal driver).
`checkh(u_e.w, 2'b01);
$write("*-* All Finished *-*\n");
$finish;
end
endmodule

View File

@ -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()