luke
/
verilator
mirror of https://github.com/verilator/verilator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Support busses with mix of pullup/pulldown (#7632)

This commit is contained in:
Lucas Amaral 2026-05-21 15:45:40 -03:00 committed by GitHub
parent a208d17939
commit 20f4eca646
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 458 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
docs/CONTRIBUTORS
View File

@ -172,6 +172,7 @@ Laurent CHARRIER
Leela Pakanati
Liam Braun
Luca Colagrande
Lucas Amaral
Ludwig Rogiers
Lukasz Dalek
M2kar

157
src/V3Tristate.cpp
View File

@ -403,8 +403,10 @@ class TristateVisitor final : public TristateBaseVisitor {
const VNUser4InUse m_inuser4;
struct AuxAstVar final {
AstPull* pullp = nullptr; // pullup/pulldown direction
AstPull* pullp = nullptr; // pullup/pulldown direction (whole variable)
AstVar* outVarp = nullptr; // output __out var
std::unordered_map<int, int>
bitPulls; // Per-bit pull: bit_index -> direction (1=up, 0=down)
};
AstUser3Allocator<AstVar, AuxAstVar> m_varAux;
@ -652,6 +654,35 @@ class TristateVisitor final : public TristateBaseVisitor {
}
}
void setBitPullDirection(AstVar* varp, int bitIndex, int direction) {
// Set pull direction for a specific bit of a bus.
// direction: 1 = pullup, 0 = pulldown
auto& bitPulls = m_varAux(varp).bitPulls;
auto it = bitPulls.find(bitIndex);
if (it != bitPulls.end() && it->second != direction) {
varp->v3warn(E_UNSUPPORTED, "Conflicting pullup/pulldown direction on bit "
<< bitIndex << " of " << varp->prettyNameQ());
}
bitPulls[bitIndex] = direction;
}
AstConst* createPerBitPullConst(AstVar* varp) {
// Create a constant with per-bit pull values.
// Bits without explicit pull default to pulldown (0).
// V3Number::setBit silently no-ops if bitIndex exceeds the variable width.
const auto& bitPulls = m_varAux(varp).bitPulls;
V3Number num{varp, varp->width(), 0}; // Start with all zeros
for (const auto& pair : bitPulls) {
if (pair.second == 1) num.setBit(pair.first, 1);
} // LCOV_EXCL_LINE
return new AstConst{varp->fileline(), num};
}
bool hasPerBitPulls(AstVar* varp) {
// Note: Not const - m_varAux allocates on access
return !m_varAux(varp).bitPulls.empty();
}
void checkUnhandled(AstNode* nodep) {
// Check for unsupported tristate constructs. This is not a 100% check.
// The best way would be to visit the tree again and find any user1p()
@ -679,14 +710,17 @@ class TristateVisitor final : public TristateBaseVisitor {
if (!m_tgraph.isTristate(varp)) continue;
const auto it = m_lhsmap.find(varp);
if (it != m_lhsmap.end()) continue;
// This variable is floating, set output enable to
// always be off on this assign
// This variable is floating, set output enable to always be off on this assign.
// For pullup/pulldown vars, use pull value with en=0 - the final resolution will
// apply the pull formula: final = (driven_value & en) | (~en & pull_value)
UINFO(8, " Adding driver to var " << varp);
AstConst* const constp = newAllZerosOrOnes(varp, false);
const AstPull* const pullp = m_varAux(varp).pullp;
const bool pullValue = pullp && pullp->direction() == 1;
AstConst* const constp = newAllZerosOrOnes(varp, pullValue);
AstVarRef* const varrefp = new AstVarRef{varp->fileline(), varp, VAccess::WRITE};
AstAssignW* const newp = new AstAssignW{varp->fileline(), varrefp, constp};
UINFO(9, " newoev " << newp);
varrefp->user1p(newAllZerosOrOnes(varp, false));
varrefp->user1p(newAllZerosOrOnes(varp, false)); // en=0 for floating/pullup vars
nodep->addStmtsp(new AstAlways{newp});
mapInsertLhsVarRef(varrefp); // insertTristates will convert
// // to a varref to the __out# variable
@ -869,7 +903,12 @@ class TristateVisitor final : public TristateBaseVisitor {
envarp = getCreateEnVarp(invarp, isTopInout); // dir set in visit(AstPin*)
outvarp->user1p(envarp);
m_varAux(outvarp).pullp = m_varAux(invarp).pullp; // AstPull* propagation
m_varAux(outvarp).bitPulls
= m_varAux(invarp).bitPulls; // Per-bit pull propagation
if (m_varAux(invarp).pullp) UINFO(9, "propagate pull to " << outvarp);
if (!m_varAux(invarp).bitPulls.empty()) {
UINFO(9, "propagate per-bit pulls to " << outvarp);
}
} else if (invarp->user1p()) {
envarp = VN_AS(invarp->user1p(), Var); // From CASEEQ, foo === 1'bz
}
@ -1001,29 +1040,41 @@ class TristateVisitor final : public TristateBaseVisitor {
return;
}
if (!outvarp) {
// This is the final pre-forced resolution of the tristate, so we apply
// the pull direction to any undriven pins.
const AstPull* const pullp = m_varAux(lhsp).pullp;
const bool pull1 = pullp && pullp->direction() == 1; // Else default is down
// Apply pull formula: final = orp | (~enp & pull_value)
// Always run at top level (!outvarp) so that the strength-enable expression (enp)
// is consumed into orp even when no pull is present; pull_value is then all-zeros
// so this is semantically a no-op but it links enp into the tree.
// Also run at intermediate levels when there's a pull, so the pull is baked into
// the contribution flowing up the hierarchy.
const bool hasPull = m_varAux(lhsp).pullp || hasPerBitPulls(lhsp);
if (!outvarp || hasPull) {
AstNodeExpr* undrivenp;
if (envarp) {
undrivenp = new AstNot{envarp->fileline(),
new AstVarRef{envarp->fileline(), envarp, VAccess::READ}};
} else if (enp) {
undrivenp = new AstNot{enp->fileline(), enp};
enp = nullptr; // moved into undrivenp
} else {
if (enp) {
undrivenp = new AstNot{enp->fileline(), enp};
} else {
undrivenp = newAllZerosOrOnes(invarp, true);
}
undrivenp = newAllZerosOrOnes(invarp, true); // LCOV_EXCL_LINE
}
undrivenp
= new AstAnd{invarp->fileline(), undrivenp, newAllZerosOrOnes(invarp, pull1)};
orp = new AstOr{invarp->fileline(), orp, undrivenp};
AstConst* pullConstp;
if (hasPerBitPulls(lhsp)) {
pullConstp = createPerBitPullConst(lhsp);
} else {
const AstPull* const pullp = m_varAux(lhsp).pullp;
const bool pull1 = pullp && pullp->direction() == 1;
pullConstp = newAllZerosOrOnes(invarp, pull1);
}
undrivenp = new AstAnd{invarp->fileline(), undrivenp, pullConstp};
orp = orp ? new AstOr{invarp->fileline(), orp, undrivenp} : undrivenp;
}
// Ensure enp is valid when envarp exists (pullup/pulldown only = no active driver)
if (envarp && !enp) enp = newAllZerosOrOnes(invarp, false);
if (envarp) {
AstAssignW* const enAssp = new AstAssignW{
enp->fileline(), new AstVarRef{envarp->fileline(), envarp, VAccess::WRITE}, enp};
@ -1536,7 +1587,27 @@ class TristateVisitor final : public TristateBaseVisitor {
UINFO(9, " enp<-rhs " << nodep->lhsp()->user1p());
m_tgraph.didProcess(nodep);
} else {
m_tgraph.didProcess(nodep, true);
// Non-tristate RHS. For SEL assigns to tristate variables, we still
// need to track which bits are driven by creating a proper enable.
if (AstSel* const selp = VN_CAST(nodep->lhsp(), Sel)) {
if (AstNodeVarRef* const varrefp = VN_CAST(selp->fromp(), NodeVarRef)) {
if (m_tgraph.isTristate(varrefp->varp())) {
// Create an all-1s enable for the SEL width - this will be
// deposited into the correct bit positions by newEnableDeposit
nodep->lhsp()->user1p(newAllZerosOrOnes(nodep->lhsp(), true));
UINFO(9, " enp<-nonTri SEL " << nodep->lhsp()->user1p());
m_tgraph.didProcess(nodep);
} else {
m_tgraph.didProcess(nodep, true);
}
} else {
m_tgraph.didProcess(nodep, true); // LCOV_EXCL_LINE
}
} else {
// Don't set user1p here as there are no handlers for many LHS node types
// (ArraySel, MemberSel, StructSel, etc.) and checkUnhandled() would error.
m_tgraph.didProcess(nodep, true);
}
}
m_alhs = true; // And user1p() will indicate tristate equation, if any
if (AstAssignW* const assignWp = VN_CAST(nodep, AssignW)) {
@ -1939,9 +2010,53 @@ class TristateVisitor final : public TristateBaseVisitor {
// Propagate any pullups/pulldowns upwards if necessary
if (exprrefp) {
if (AstPull* const pullp = m_varAux(nodep->modVarp()).pullp) {
AstPull* const pullp = m_varAux(nodep->modVarp()).pullp;
const auto& srcBitPulls = m_varAux(nodep->modVarp()).bitPulls;
// For part-select port connections (e.g. .out(bus[23:16])) extract
// the parent var, LSB, and width once; reused by both propagations.
AstVar* selVarp = nullptr;
int selLsb = 0;
int selWidth = 0;
if (outAssignp) {
if (AstSel* const selp = VN_CAST(outAssignp->lhsp(), Sel)) {
AstVarRef* const vrefp = VN_CAST(selp->fromp(), VarRef);
AstConst* const lsbp = VN_CAST(selp->lsbp(), Const);
if (vrefp && lsbp) { // LCOV_EXCL_BR_LINE
selVarp = vrefp->varp();
selLsb = lsbp->toSInt();
selWidth = selp->widthConst();
}
}
}
if (pullp) {
UINFO(9, "propagate pull on " << exprrefp);
setPullDirection(exprrefp->varp(), pullp);
// For a part-select target, record per-bit pull direction across
// the SEL range instead of setting pull on the whole variable.
if (selVarp) {
const int direction = pullp->direction();
for (int i = 0; i < selWidth; ++i) {
setBitPullDirection(selVarp, selLsb + i, direction);
}
}
}
// Per-bit pulls: the source bit indices must be offset by the SEL's
// LSB so they land on the correct bits of the parent variable.
if (!srcBitPulls.empty()) {
UINFO(9, "propagate per-bit pulls from " << nodep->modVarp());
for (const auto& pair : srcBitPulls) {
setBitPullDirection(exprrefp->varp(), pair.first, pair.second);
} // LCOV_EXCL_LINE
if (selVarp) {
UINFO(9, "propagate per-bit pulls to SEL target " << selVarp
<< " offset=" << selLsb);
for (const auto& pair : srcBitPulls) {
setBitPullDirection(selVarp, selLsb + pair.first, pair.second);
} // LCOV_EXCL_LINE
}
}
}
// Don't need to visit the created assigns, as it was added at

15
test_regress/t/t_pull_bitsel.py Executable file
View File

@ -0,0 +1,15 @@
#!/usr/bin/env python3
# DESCRIPTION: Verilator: Test pullup/pulldown on bus bit-selects via wrapper
#
# SPDX-FileCopyrightText: 2026 Wilson Snyder
# SPDX-License-Identifier: CC0-1.0
import vltest_bootstrap
test.scenarios('simulator')
test.compile()
test.execute()
test.passes()

87
test_regress/t/t_pull_bitsel.v Normal file
View File

@ -0,0 +1,87 @@
// DESCRIPTION: Verilator: Test pullup/pulldown with bit-select assigns
//
// SPDX-FileCopyrightText: 2026 Lucas Amaral
// SPDX-License-Identifier: CC0-1.0
// Bug: When a module has bit-select assigns (e.g., out[17:0] = in[17:0])
// combined with pullup/pulldown tie cells for other bits, the enable
// for the assigns incorrectly covers all bits, causing the pull constant
// to be optimized away.
// verilator lint_off PINMISSING
// Tie cell with pullup/pulldown (like sky130_fd_sc_hd__conb)
module conb(output HI, output LO);
pullup pu (HI);
pulldown pd (LO);
endmodule
// Wrapper that instantiates tie cell and connects to specific bit
module tiecell_1(output HI, output LO);
conb base (.HI(HI), .LO(LO));
endmodule
// Parameterized tie cell for ranged connections; exercises the multi-bit
// SEL path in V3Tristate's per-bit pull tracking.
module tiecell_n #(parameter N = 1) (output [N-1:0] HI, output [N-1:0] LO);
genvar gi;
generate
for (gi = 0; gi < N; gi = gi + 1) begin : g
conb base (.HI(HI[gi]), .LO(LO[gi]));
end
endgenerate
endmodule
// Submodule mirroring the post-synthesis shape of a module whose outputs are
// constants implemented as tie cells: each output bit driven by exactly one
// tie cell of fixed direction. Per-bit pulls must propagate up to the parent
// net the submodule's output port is connected to.
module mask_col(output [7:0] out);
conb t0 (.HI(out[0])); // bit 0 = 1
conb t1 (.LO(out[1])); // bit 1 = 0
conb t2 (.HI(out[2])); // bit 2 = 1
conb t3 (.LO(out[3])); // bit 3 = 0
conb t4 (.HI(out[4])); // bit 4 = 1
conb t5 (.LO(out[5])); // bit 5 = 0
conb t6 (.HI(out[6])); // bit 6 = 1
conb t7 (.LO(out[7])); // bit 7 = 0
endmodule
module top(input [31:0] in_value, output [31:0] out_value);
assign out_value[7:0] = in_value[7:0];
// Bits 8-15: bit 15 pulled up, rest pulled down via single-bit and ranged cells.
tiecell_1 u_hi (.HI(out_value[15]));
tiecell_n #(.N(4)) u_lo_8_11 (.LO(out_value[11:8]));
tiecell_1 u_lo_8_dup(.LO(out_value[8]));
tiecell_n #(.N(3)) u_lo_12_14(.LO(out_value[14:12]));
// Bits 16-23 driven hierarchically through a part-select port connection.
mask_col u_mask (.out(out_value[23:16]));
assign out_value[31:24] = in_value[31:24];
endmodule
`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);
module t;
// Use wire with assign - values propagate in same delta cycle
wire [31:0] in_value = 32'hDE00_0000;
wire [31:0] out_value;
top dut (.in_value(in_value), .out_value(out_value));
// 0xDE = passthrough[31:24], 0x55 = mask_col HI/LO/HI/LO/HI/LO/HI/LO at [23:16],
// 0x80 = bit 15 pullup + bits 14:8 pulldown, 0x00 = passthrough[7:0].
wire [31:0] expected = 32'hDE55_8000;
initial begin
$display("in_value = %h, out_value = %h, expected = %h", in_value, out_value, expected);
`checkh(out_value, expected);
$write("*-* All Finished *-*\n");
$finish;
end
endmodule

6
test_regress/t/t_pull_bitsel_conflict_bad.out Normal file
View File

@ -0,0 +1,6 @@
%Error-UNSUPPORTED: t/t_pull_bitsel_conflict_bad.v:17:28: Conflicting pullup/pulldown direction on bit 2 of 'bus'
: ... note: In instance 't'
17 | module t(output wire [3:0] bus);
| ^~~
... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest
%Error: Exiting due to

16
test_regress/t/t_pull_bitsel_conflict_bad.py Executable file
View File

@ -0,0 +1,16 @@
#!/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('linter')
test.lint(fails=True, expect_filename=test.golden_filename)
test.passes()

22
test_regress/t/t_pull_bitsel_conflict_bad.v Normal file
View File

@ -0,0 +1,22 @@
// DESCRIPTION: Verilator: Test that conflicting per-bit pull directions on
// the same bit of a bus produce an UNSUPPORTED error.
//
// SPDX-FileCopyrightText: 2026 Lucas Amaral
// SPDX-License-Identifier: CC0-1.0
// verilator lint_off PINMISSING
module pullup_leaf(output wire o);
pullup pu (o);
endmodule
module pulldown_leaf(output wire o);
pulldown pd (o);
endmodule
module t(output wire [3:0] bus);
// Bit 2 of 'bus' is driven by both a pullup AND a pulldown through hierarchy,
// which is an electrical short. Verilator must reject this at compile time.
pullup_leaf pu_inst (.o(bus[2]));
pulldown_leaf pd_inst (.o(bus[2]));
endmodule

15
test_regress/t/t_pull_bitsel_whole.py Executable file
View File

@ -0,0 +1,15 @@
#!/usr/bin/env python3
# DESCRIPTION: Verilator: Test pullup/pulldown bus propagation through whole-vector ports
#
# SPDX-FileCopyrightText: 2026 Wilson Snyder
# SPDX-License-Identifier: CC0-1.0
import vltest_bootstrap
test.scenarios('simulator')
test.compile()
test.execute()
test.passes()

70
test_regress/t/t_pull_bitsel_whole.v Normal file
View File

@ -0,0 +1,70 @@
// DESCRIPTION: Verilator: Test per-bit pullup/pulldown propagation through whole-vector ports
//
// SPDX-FileCopyrightText: 2026 Lucas Amaral
// SPDX-License-Identifier: CC0-1.0
// verilator lint_off PINMISSING
module conb(output HI, output LO);
pullup pu (HI);
pulldown pd (LO);
endmodule
module mask_col(output [7:0] out);
conb t0 (.HI(out[0]));
conb t1 (.LO(out[1]));
conb t2 (.HI(out[2]));
conb t3 (.LO(out[3]));
conb t4 (.HI(out[4]));
conb t5 (.LO(out[5]));
conb t6 (.HI(out[6]));
conb t7 (.LO(out[7]));
endmodule
module pull_hi(output HI);
pullup pu (HI);
endmodule
module top(input [7:0] in_value, output [15:0] out_value, output [7:0] direct_mask,
output direct_pull);
typedef struct packed {
logic [1:0] field;
} pair_t;
wire [7:0] pulled;
pair_t pair;
assign out_value[7:0] = in_value;
assign out_value[15:8] = pulled;
assign pair.field[0] = in_value[0];
// Whole-vector port connection. This exercises propagation of the child
// module's per-bit pulls without the parent connection being a part-select.
mask_col u_mask (.out(pulled));
// Direct whole-vector/scalar output connections cover propagation to the
// assignment target created for parent output ports.
mask_col u_direct_mask (.out(direct_mask));
pull_hi u_direct_pull (.HI(direct_pull));
endmodule
`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);
module t;
wire [7:0] in_value = 8'hA6;
wire [15:0] out_value;
wire [7:0] direct_mask;
wire direct_pull;
top dut (.in_value(in_value), .out_value(out_value), .direct_mask(direct_mask),
.direct_pull(direct_pull));
initial begin
`checkh(out_value, 16'h55A6);
`checkh(direct_mask, 8'h55);
`checkh(direct_pull, 1'b1);
$write("*-* All Finished *-*\n");
$finish;
end
endmodule

15
test_regress/t/t_tri_pullup_bus.py Executable file
View File

@ -0,0 +1,15 @@
#!/usr/bin/env python3
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# SPDX-FileCopyrightText: 2026 Wilson Snyder
# SPDX-License-Identifier: CC0-1.0
import vltest_bootstrap
test.scenarios('simulator')
test.compile(timing_loop=True, verilator_flags2=['--timing'])
test.execute()
test.passes()

75
test_regress/t/t_tri_pullup_bus.v Normal file
View File

@ -0,0 +1,75 @@
// DESCRIPTION: Verilator: Test pullup/pulldown with partial bus assigns
//
// This tests the case where:
// - A bus has some bits driven by pullup/pulldown through hierarchical modules
// - Other bits are driven by regular assigns (partial SEL)
// - The enable tracking must correctly handle the SEL assigns
//
// SPDX-FileCopyrightText: 2026 Lucas Amaral
// SPDX-License-Identifier: CC0-1.0
`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);
`default_nettype none
module pullup_mod(output HI);
pullup pullup0(HI);
endmodule
module pulldown_mod(output LO);
pulldown pulldown0(LO);
endmodule
module top (
input wire [3:0] in_value,
output wire [7:0] out_value
);
// Lower 4 bits driven by input (partial SEL assign)
assign out_value[3:0] = in_value;
// Upper 4 bits: alternating pullup/pulldown through hierarchical modules
// out_value[4] = 1 (pullup)
// out_value[5] = 0 (pulldown)
// out_value[6] = 1 (pullup)
// out_value[7] = 0 (pulldown)
pullup_mod p0(.HI(out_value[4]));
pulldown_mod p1(.LO(out_value[5]));
pullup_mod p2(.HI(out_value[6]));
pulldown_mod p3(.LO(out_value[7]));
endmodule
module t;
reg [3:0] in_value;
wire [7:0] out_value;
top dut(.in_value(in_value), .out_value(out_value));
initial begin
// Test 1: Lower bits = 0xF
in_value = 4'hF;
#1;
// Expected: 0x5F = 0101_1111
// Bits [3:0] = F (from input)
// Bit 4 = 1 (pullup), Bit 5 = 0 (pulldown)
// Bit 6 = 1 (pullup), Bit 7 = 0 (pulldown)
`checkh(out_value, 8'h5F);
// Test 2: Lower bits = 0xA
in_value = 4'hA;
#1;
// Expected: 0x5A = 0101_1010
`checkh(out_value, 8'h5A);
// Test 3: Lower bits = 0x0
in_value = 4'h0;
#1;
// Expected: 0x50 = 0101_0000
`checkh(out_value, 8'h50);
$write("*-* All Finished *-*\n");
$finish;
end
endmodule
`default_nettype wire