From 2215d01d6be892514316a8db5316e05fd319ab1a Mon Sep 17 00:00:00 2001 From: Leela Pakanati <41307800+cachanova@users.noreply.github.com> Date: Thu, 5 Feb 2026 21:15:30 -0600 Subject: [PATCH] Fix hierarchical interface/modport issues (#5941) (#6997) --- src/V3LinkDot.cpp | 46 +++++- test_regress/t/t_interface_modport_hier.py | 21 +++ test_regress/t/t_interface_modport_hier.v | 137 ++++++++++++++++++ .../t/t_interface_modport_hier_noinl.py | 19 +++ 4 files changed, 219 insertions(+), 4 deletions(-) create mode 100755 test_regress/t/t_interface_modport_hier.py create mode 100644 test_regress/t/t_interface_modport_hier.v create mode 100755 test_regress/t/t_interface_modport_hier_noinl.py diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index ec680a11d..9cdc71c78 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -774,13 +774,51 @@ public: } if (!lookupSymp) return nullptr; // Not found } - } else { // Searching for middle submodule, must be a cell name + } else { // Searching for middle path component, must be a cell or interface port VSymEnt* findSymp = findWithAltFlat(lookupSymp, ident, altIdent); if (!findSymp) findSymp = findForkParentAlias(lookupSymp, ident); if (findSymp) { lookupSymp = unwrapForkParent(findSymp, ident); } else { - return nullptr; // Not found + // Try prefixed lookup for interface ports accessed through hierarchy + // (e.g., slave_inst.bus.data where bus is an interface port) + // After inlining, interface ports have prefixed names like + // top__DOT__slave_inst__DOT__bus + UINFO(8, " middle-path: trying prefixed lookup for '" << ident << "'\n"); + string baddot; + findSymp = findSymPrefixed(lookupSymp, ident, baddot, /*fallback=*/true); + UINFO(8, " middle-path: prefixed lookup result: " + << (findSymp ? "found" : "not found") << "\n"); + if (findSymp) { + // Check if this is an interface reference variable + const AstVar* varp = VN_CAST(findSymp->nodep(), Var); + if (!varp) { + if (const AstVarScope* vscp = VN_CAST(findSymp->nodep(), VarScope)) { + varp = vscp->varp(); + } + } + if (varp && varp->isIfaceRef()) { + // Found an interface port - use findSymp directly + // computeScopeAliases has already imported interface members + // into this symbol entry, so we use it instead of redirecting + // to the shared modport definition (which would cause all + // instances to resolve to the same symbols - bug #2656) + lookupSymp = findSymp; + } else { + // Non-interface symbol in middle path must be a cell (module instance) + // to continue hierarchical resolution + if (VN_IS(findSymp->nodep(), Cell)) { + lookupSymp = findSymp; + } else { + // Reject non-cell, non-interface symbols found via fallback + // to prevent accidental resolution to unrelated symbols + UINFO(8, " middle-path: rejecting non-cell symbol\n"); + return nullptr; + } + } + } else { + return nullptr; // Not found + } } } if (lookupSymp) { @@ -2413,8 +2451,8 @@ private: UINFO(5, " Found interface instance: se" << cvtToHex(cellSymp) << " " << cellSymp->nodep()); if (dtypep->modportName() != "") { - VSymEnt* const mpSymp = m_statep->findDotted( - nodep->fileline(), m_modSymp, ifcellname, baddot, okSymp, false); + // Look up the modport within the interface cell's symbol table + VSymEnt* const mpSymp = cellSymp->findIdFallback(dtypep->modportName()); UASSERT_OBJ(mpSymp, nodep, "No symbol for interface modport: " << nodep->prettyNameQ(dtypep->modportName())); diff --git a/test_regress/t/t_interface_modport_hier.py b/test_regress/t/t_interface_modport_hier.py new file mode 100755 index 000000000..3fb25347f --- /dev/null +++ b/test_regress/t/t_interface_modport_hier.py @@ -0,0 +1,21 @@ +#!/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 hierarchical access through modport interface ports +# Related to Issue #5941 and #2656 + +test.compile() + +test.execute() + +test.passes() diff --git a/test_regress/t/t_interface_modport_hier.v b/test_regress/t/t_interface_modport_hier.v new file mode 100644 index 000000000..307ca49ae --- /dev/null +++ b/test_regress/t/t_interface_modport_hier.v @@ -0,0 +1,137 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Leela Pakanati +// SPDX-License-Identifier: CC0-1.0 + +// Test for Issue #5941 and #2656: Modport interface field access via hierarchy +// Tests: +// - Single-level and deep hierarchical access +// - Multiple interface instances (same modport type, different data) +// - Interface arrays + +interface bus_if (input logic clk); + logic [7:0] data; + modport slave (output data, input clk); +endinterface + +// l1 module with the actual logic +module l1_mod (bus_if.slave bus); + always_ff @(posedge bus.clk) + bus.data <= 8'h5A; +endmodule + +// l0 module wrapping l1 module +module l0_mod (bus_if.slave bus); + l1_mod l1_inst (bus); +endmodule + +// Modules for testing multiple instances with same modport +module mod_aa (bus_if.slave bus); + assign bus.data = 8'hAA; +endmodule + +module mod_bb (bus_if.slave bus); + assign bus.data = 8'hBB; +endmodule + +// Module for testing interface arrays +module array_mod (bus_if.slave bus[2]); + always_ff @(posedge bus[0].clk) begin + bus[0].data <= 8'hA0; + bus[1].data <= 8'hA1; + end +endmodule + +module t (/*AUTOARG*/ + // Inputs + clk + ); + + input clk; + integer cyc = 0; + + // Deep hierarchy test + bus_if bus (clk); + l0_mod l0_inst (bus); + + // Multiple instances test + bus_if bus_a (clk); + bus_if bus_b (clk); + mod_aa inst_aa (bus_a); + mod_bb inst_bb (bus_b); + + // Array test + bus_if bus_arr[2] (clk); + array_mod array_inst (bus_arr); + + always @(posedge clk) begin + cyc <= cyc + 1; + if (cyc == 5) begin + // === Deep hierarchy tests === + $display("bus.data = %h (direct)", bus.data); + $display("l0_inst.bus.data = %h (single-level)", l0_inst.bus.data); + $display("l0_inst.l1_inst.bus.data = %h (deep)", l0_inst.l1_inst.bus.data); + + if (bus.data !== 8'h5A) begin + $display("%%Error: bus.data = %h, expected 5A", bus.data); + $stop; + end + if (l0_inst.bus.data !== 8'h5A) begin + $display("%%Error: l0_inst.bus.data = %h, expected 5A", l0_inst.bus.data); + $stop; + end + if (l0_inst.l1_inst.bus.data !== 8'h5A) begin + $display("%%Error: l0_inst.l1_inst.bus.data = %h, expected 5A", l0_inst.l1_inst.bus.data); + $stop; + end + if (l0_inst.bus.clk !== clk) begin + $display("%%Error: l0_inst.bus.clk mismatch"); + $stop; + end + if (l0_inst.l1_inst.bus.clk !== clk) begin + $display("%%Error: l0_inst.l1_inst.bus.clk mismatch"); + $stop; + end + + // === Multiple instances tests (bug #2656) === + $display("inst_aa.bus.data = %h", inst_aa.bus.data); + $display("inst_bb.bus.data = %h", inst_bb.bus.data); + + if (inst_aa.bus.data !== 8'hAA) begin + $display("%%Error: inst_aa.bus.data = %h, expected AA", inst_aa.bus.data); + $stop; + end + if (inst_bb.bus.data !== 8'hBB) begin + $display("%%Error: inst_bb.bus.data = %h, expected BB", inst_bb.bus.data); + $stop; + end + + // === Interface array tests (bug #2656) === + $display("bus_arr[0].data = %h (direct)", bus_arr[0].data); + $display("bus_arr[1].data = %h (direct)", bus_arr[1].data); + $display("array_inst.bus[0].data = %h (hierarchical)", array_inst.bus[0].data); + $display("array_inst.bus[1].data = %h (hierarchical)", array_inst.bus[1].data); + + if (bus_arr[0].data !== 8'hA0) begin + $display("%%Error: bus_arr[0].data = %h, expected A0", bus_arr[0].data); + $stop; + end + if (bus_arr[1].data !== 8'hA1) begin + $display("%%Error: bus_arr[1].data = %h, expected A1", bus_arr[1].data); + $stop; + end + if (array_inst.bus[0].data !== 8'hA0) begin + $display("%%Error: array_inst.bus[0].data = %h, expected A0", array_inst.bus[0].data); + $stop; + end + if (array_inst.bus[1].data !== 8'hA1) begin + $display("%%Error: array_inst.bus[1].data = %h, expected A1", array_inst.bus[1].data); + $stop; + end + + $write("*-* All Finished *-*\n"); + $finish; + end + end +endmodule diff --git a/test_regress/t/t_interface_modport_hier_noinl.py b/test_regress/t/t_interface_modport_hier_noinl.py new file mode 100755 index 000000000..0950605f0 --- /dev/null +++ b/test_regress/t/t_interface_modport_hier_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/t_interface_modport_hier.v" + +test.compile(v_flags2=["-fno-inline"]) + +test.execute() + +test.passes()