Optimize instance port connections by aliasing rather than by assignment (#8427)

Use AstAlias instead of AstAssignW in V3Inst port connection lowering,
the same way as V3Inline would do if the instance is inlined. This
closes some of the gap between -finline and -fno-inline behaviour, and
is also required for moving V3Inline post scope, where it could not
create aliases anymore (hence required for future patch to maintain
current behaviour).

This is also a partial fix to #4698 (only when the connected expression
is a plain VarRef)
This commit is contained in:
Geza Lore
2026-09-22 20:27:21 +01:00
committed by GitHub
parent 0ab3915de6
commit 6eced23e84
8 changed files with 200 additions and 26 deletions
-7
View File
@@ -183,13 +183,6 @@ class DataflowOptimize final {
}
DataflowOptimize(AstNetlist* netlistp) {
// Mark interfaces that might be referenced by a virtual interface
if (v3Global.hasVirtIfaces()) {
netlistp->typeTablep()->foreach([](const AstIfaceRefDType* nodep) {
if (!nodep->isVirtual()) return;
nodep->ifaceViaCellp()->setHasVirtualRef();
});
}
// Mark variables with external references
markExternallyReferencedVariables(netlistp);
// Dump stage stats
+3 -4
View File
@@ -599,8 +599,9 @@ class FsmDetectVisitor final : public VNVisitor {
if (!fsmRegisterWrapperDesc(cellp)) return;
UASSERT_OBJ(lhsVscp->varp()->isInput(), nodep,
"Child-side port alias lhs should be an input");
UASSERT_OBJ(rhsVscp->scopep() == m_scopep, nodep,
"Child input port alias should connect from the parent scope");
// Note the connected variable can live in a scope further up, as V3Inst
// aliases a port connection, merging the port variable of an instance into
// whatever it is connected to
m_cellPortAliases[cellp][lhsVscp->varp()->name()] = rhsVscp;
m_cellPortChildAliases[cellp][lhsVscp->varp()->name()] = lhsVscp;
addWrapperCell(m_scopep, cellp);
@@ -608,8 +609,6 @@ class FsmDetectVisitor final : public VNVisitor {
if (!fsmRegisterWrapperDesc(cellp)) return;
UASSERT_OBJ(rhsVscp->varp()->isWritable(), nodep,
"Child-side port alias rhs should be writable");
UASSERT_OBJ(lhsVscp->scopep() == m_scopep, nodep,
"Child output port alias should connect into the parent scope");
m_cellPortAliases[cellp][rhsVscp->varp()->name()] = lhsVscp;
m_cellPortChildAliases[cellp][rhsVscp->varp()->name()] = rhsVscp;
addWrapperCell(m_scopep, cellp);
+76 -14
View File
@@ -26,6 +26,7 @@
#include "V3Inst.h"
#include "V3Const.h"
#include "V3Control.h"
#include "V3Width.h"
VL_DEFINE_DEBUG_FUNCTIONS;
@@ -48,6 +49,56 @@ class InstVisitor final : public VNVisitor {
// STATE
AstCell* m_cellp = nullptr; // Current cell
// METHODS
// If appropriate, add an AstAlias to connect the given Cell pin to the given expression.
// Returns true if an alias was made, in which case there is nothing else to do for this pin.
bool tryAliasPin(AstPin* nodep, AstNodeExpr* exprp) {
AstVar* const modVarp = nodep->modVarp();
// An interface reference is aliased via AstAliasScope below, not as a variable
if (modVarp->isIfaceRef()) return false;
// Only a whole variable can be aliased, anything else needs the assignment
AstVarRef* const refp = VN_CAST(exprp, VarRef);
if (!refp) return false;
AstVar* const exprVarp = refp->varp();
// A ref port must become an alias
if (!modVarp->direction().isRef()) {
// V3FsmDetect recognizes the registers of an fsm_register_wrapper instance via
// the assignments of its pins, so leave those as assignments
AstNodeModule* const cellModp = m_cellp->modp();
if (V3Control::getFsmRegisterWrapper(cellModp->origName())
|| V3Control::getFsmRegisterWrapper(cellModp->prettyDehashOrigOrName())) {
return false;
}
// A virtual interface method call is dispatched at run time, so the body of an
// interface reached that way must use the signals of the instance it is called
// on, not those of whichever instance this connection happens to be made to
if (const AstIface* const ifacep = VN_CAST(m_cellp->modp(), Iface)) {
if (ifacep->hasVirtualRef()) return false;
}
// Forced signals must keep their own storage, the two sides can be forced separately
if (modVarp->isForced() || exprVarp->isForced()) return false;
// Same for public
if (modVarp->isSigUserRWPublic() || exprVarp->isSigUserRWPublic()) return false;
// V3Tristate resolved the connected net already, and drives it from the
// resolution it built for it, so a port merged into it would be driven by
// the resolution of the instance as well
if (exprVarp->isTristate()) return false;
}
// They will become the same variable, so propagate file-line and attributes
exprVarp->fileline()->modifyStateInherit(modVarp->fileline());
modVarp->fileline()->modifyStateInherit(exprVarp->fileline());
exprVarp->propagateAttrFrom(modVarp);
modVarp->propagateAttrFrom(exprVarp);
// The port is named first, so the net it connects to is the one that survives
refp->access(VAccess::READWRITE);
FileLine* const flp = exprp->fileline();
AstNodeExpr* const itemsp
= new AstVarXRef{flp, modVarp, m_cellp->name(), VAccess::READWRITE};
itemsp->addNext(exprp);
m_cellp->addNextHere(new AstAlias{flp, itemsp});
return true;
}
// VISITORS
void visit(AstCell* nodep) override {
UINFO(4, " CELL " << nodep);
@@ -57,6 +108,7 @@ class InstVisitor final : public VNVisitor {
AstNode::user1ClearTree();
iterateChildren(nodep);
}
void visit(AstPin* nodep) override {
// PIN(p,expr) -> ASSIGNW(VARXREF(p),expr) (if sub's input)
// or ASSIGNW(expr,VARXREF(p)) (if sub's output)
@@ -78,21 +130,25 @@ class InstVisitor final : public VNVisitor {
if (nodep->modVarp()->isInout()) {
nodep->v3fatalSrc("Unsupported: Verilator is a 2-state simulator");
} else if (nodep->modVarp()->isWritable()) {
AstNodeExpr* const rhsp = new AstVarXRef{exprp->fileline(), nodep->modVarp(),
m_cellp->name(), VAccess::READ};
markContinuousLhs(exprp);
AstAssignW* const assp = new AstAssignW{exprp->fileline(), exprp, rhsp};
m_cellp->addNextHere(new AstAlways{assp});
if (!tryAliasPin(nodep, exprp)) {
AstNodeExpr* const rhsp = new AstVarXRef{exprp->fileline(), nodep->modVarp(),
m_cellp->name(), VAccess::READ};
markContinuousLhs(exprp);
AstAssignW* const assp = new AstAssignW{exprp->fileline(), exprp, rhsp};
m_cellp->addNextHere(new AstAlways{assp});
}
} else if (nodep->modVarp()->isNonOutput()) {
// Don't bother moving constants now,
// we'll be pushing the const down to the cell soon enough.
AstVarXRef* const lhsp = new AstVarXRef{exprp->fileline(), nodep->modVarp(),
m_cellp->name(), VAccess::WRITE};
if (!tryAliasPin(nodep, exprp)) {
// Don't bother moving constants now,
// we'll be pushing the const down to the cell soon enough.
AstVarXRef* const lhsp = new AstVarXRef{exprp->fileline(), nodep->modVarp(),
m_cellp->name(), VAccess::WRITE};
markContinuousLhs(lhsp);
AstAssignW* const assp = new AstAssignW{exprp->fileline(), lhsp, exprp};
m_cellp->addNextHere(new AstAlways{assp});
UINFOTREE(9, assp, "", "_new");
markContinuousLhs(lhsp);
AstAssignW* const assp = new AstAssignW{exprp->fileline(), lhsp, exprp};
m_cellp->addNextHere(new AstAlways{assp});
UINFOTREE(9, assp, "", "_new");
}
} else if (nodep->modVarp()->isIfaceRef()
|| (VN_IS(nodep->modVarp()->dtypep()->skipRefp(), UnpackArrayDType)
&& VN_IS(VN_AS(nodep->modVarp()->dtypep()->skipRefp(), UnpackArrayDType)
@@ -127,7 +183,13 @@ class InstVisitor final : public VNVisitor {
public:
// CONSTRUCTORS
explicit InstVisitor(AstNetlist* nodep) { iterate(nodep); }
explicit InstVisitor(AstNetlist* nodep) {
// Modules are level sorted, with the top module first. Visit them in reverse
// order, that is children before parents, so that the warning disables and the
// attributes of a port variable propagate all the way up through a chain of
// aliased port connections (see tryAliasPin).
iterateChildrenBackwardsConst(nodep);
}
~InstVisitor() override = default;
};
+14 -1
View File
@@ -2759,7 +2759,20 @@ private:
UINFOTREE(9, nodep, "", "alias");
AstVarScope* aliasVscp = nullptr;
for (AstNode* itemp = nodep->itemsp(); itemp; itemp = itemp->nextp()) {
AstVarScope* const vscp = VN_AS(itemp, VarRef)->varScopep();
AstVarScope* vscp = nullptr;
if (const AstVarRef* const refp = VN_CAST(itemp, VarRef)) {
vscp = refp->varScopep();
} else {
// Reaches into the scope of an instance, look it up by name
const AstVarXRef* const xrefp = VN_AS(itemp, VarXRef);
const string scopename = xrefp->dotted() + "." + xrefp->name();
string baddot;
VSymEnt* okSymp;
VSymEnt* const symp = m_statep->findDotted(xrefp->fileline(), m_modSymp, scopename,
baddot, okSymp, false);
UASSERT_OBJ(symp, nodep, "No symbol for alias item: " << scopename);
vscp = VN_CAST(symp->nodep(), VarScope);
}
UASSERT_OBJ(vscp, nodep, "VarScope unset");
if (aliasVscp) {
setAliasVarScope(aliasVscp, vscp);
+1
View File
@@ -3853,6 +3853,7 @@ class WidthVisitor final : public VNVisitor {
UINFO(5, " IFACEREF " << nodep);
userIterateChildren(nodep, m_vup);
nodep->dtypep(nodep);
if (nodep->isVirtual()) nodep->ifaceViaCellp()->setHasVirtualRef();
UINFO(4, "dtWidthed " << nodep);
}
void visit(AstNodeUOrStructDType* nodep) override {
+18
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(verilator_flags2=['--binary'])
test.execute()
test.passes()
+68
View File
@@ -0,0 +1,68 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain.
// SPDX-FileCopyrightText: 2026 Wilson Snyder
// SPDX-License-Identifier: CC0-1.0
// verilog_format: off
`define stop $stop
`define checkd(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got=%0d exp=%0d\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0);
// verilog_format: on
// A ref port is another name for the variable it is connected to, so a write at
// either end must be seen at the other.
module sub (
input bit clk,
input int cyc,
ref int y
);
always @(posedge clk) begin
if (cyc == 1) y <= 100;
else if (cyc == 2) `checkd(y, 100)
else if (cyc == 4) `checkd(y, 200) // Written by 't'
end
endmodule
// A ref port read from a task of the instance.
module subtask (
ref int z
);
task static check(int expv);
`checkd(z, expv)
endtask
endmodule
module t;
bit clk = 0;
always #5 clk = ~clk;
int cyc = 0;
// Driven both here and in 'sub' via the ref port - that being the point of this test
// verilator lint_off MULTIDRIVEN
int x;
// verilator lint_on MULTIDRIVEN
int w = 15;
sub s (clk, cyc, x);
subtask st (.z(w));
always @(posedge clk) begin
cyc <= cyc + 1;
if (cyc == 2) begin
`checkd(x, 100) // Written by 's'
end else if (cyc == 3) begin
x <= 200;
end else if (cyc == 4) begin
`checkd(x, 200)
st.check(15);
w = 16;
st.check(16);
end else if (cyc == 5) begin
`checkd(w, 16)
$write("*-* All Finished *-*\n");
$finish;
end
end
endmodule
+20
View File
@@ -0,0 +1,20 @@
#!/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
test.top_filename = "t/t_var_ref_port.v"
import vltest_bootstrap
test.scenarios('vlt')
test.compile(verilator_flags2=['--binary', '-fno-inline'])
test.execute()
test.passes()