From f6a63db884270ecd96f00ff40722670c2d496273 Mon Sep 17 00:00:00 2001 From: Yilou Wang Date: Thu, 18 Jun 2026 22:27:57 +0200 Subject: [PATCH] Process only interfaces in the first tristate pass; no_optimize test drivers --- src/V3Tristate.cpp | 40 +++++++++---------- test_regress/t/t_interface_inout_tristate.v | 32 +++++++++------ .../t/t_interface_tristate_plain_xhier.v | 21 ++++++---- 3 files changed, 54 insertions(+), 39 deletions(-) diff --git a/src/V3Tristate.cpp b/src/V3Tristate.cpp index c73f9f5e4..5bc72076b 100644 --- a/src/V3Tristate.cpp +++ b/src/V3Tristate.cpp @@ -446,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 @@ -2094,10 +2095,11 @@ 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). + // 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()); @@ -2182,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); @@ -2318,24 +2323,19 @@ public: // CONSTRUCTORS explicit TristateVisitor(AstNetlist* netlistp) { m_tgraph.clearAndCheck(); - // 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; + // 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 ifacesp; 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); + 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 combineIfaceContribs(); diff --git a/test_regress/t/t_interface_inout_tristate.v b/test_regress/t/t_interface_inout_tristate.v index de376e1ee..36b20fea2 100644 --- a/test_regress/t/t_interface_inout_tristate.v +++ b/test_regress/t/t_interface_inout_tristate.v @@ -7,6 +7,11 @@ // 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 @@ -42,20 +47,23 @@ module t; 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); + 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 = 0; - top_oe = 1; - topval = 8'h3C; - #1 `checkh(ifc0.data, 8'h3C); + 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 = 0; - top_oe = 0; - #1 `checkh(ifc0.data, 8'hzz); + oe = `no_optimize(1'b0); + top_oe = `no_optimize(1'b0); + #1; + `checkh(ifc0.data, 8'hzz); $write("*-* All Finished *-*\n"); $finish; end diff --git a/test_regress/t/t_interface_tristate_plain_xhier.v b/test_regress/t/t_interface_tristate_plain_xhier.v index eaf07cb95..d7c7d6ca6 100644 --- a/test_regress/t/t_interface_tristate_plain_xhier.v +++ b/test_regress/t/t_interface_tristate_plain_xhier.v @@ -7,6 +7,11 @@ // 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 @@ -14,10 +19,12 @@ 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; + // 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 wint = 2'b11; + 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. @@ -66,10 +73,10 @@ module t; ifc_tri u_e (); logic [1:0] va, vb, vc, ve; - assign va = 2'b01; - assign vb = 2'b10; - assign vc = 2'b11; - assign ve = 2'b01; + 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