From 6984f306ac8d57bf14729819f2f89186a89981b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Chmiel?= Date: Fri, 28 Nov 2025 18:06:46 +0100 Subject: [PATCH 1/2] Prevent inline into trace updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jakub Wasilewski Signed-off-by: Bartłomiej Chmiel --- src/V3Gate.cpp | 9 +++ src/V3Inst.cpp | 13 ++++ test_regress/t/t_trace_no_inline.py | 97 +++++++++++++++++++++++++++++ test_regress/t/t_trace_no_inline.v | 54 ++++++++++++++++ 4 files changed, 173 insertions(+) create mode 100755 test_regress/t/t_trace_no_inline.py create mode 100644 test_regress/t/t_trace_no_inline.v diff --git a/src/V3Gate.cpp b/src/V3Gate.cpp index f6e5a9198..e96c9c9db 100644 --- a/src/V3Gate.cpp +++ b/src/V3Gate.cpp @@ -197,6 +197,7 @@ class GateBuildVisitor final : public VNVisitorConst { AstActive* m_activep = nullptr; // Current active bool m_inClockedActive = false; // Underneath clocked active bool m_inSenItem = false; // Underneath AstSenItem; any varrefs are clocks + bool m_inTraceDecl = false; // Underneath AstTraceDecl // METHODS void checkNode(AstNode* nodep) { @@ -269,6 +270,11 @@ class GateBuildVisitor final : public VNVisitorConst { iterateLogic(nodep, false, nullptr, "senItem"); } } + void visit(AstTraceDecl* nodep) override { + VL_RESTORER(m_inTraceDecl); + m_inTraceDecl = true; + iterateChildrenConst(nodep); + } void visit(AstNodeVarRef* nodep) override { if (!m_logicVertexp) return; @@ -287,6 +293,9 @@ class GateBuildVisitor final : public VNVisitorConst { } } + // To reduce code size, clear reducible for non-primaryIO signals in trace + if (m_inTraceDecl && !vscp->varp()->isPrimaryIO()) vVtxp->clearReducible("TraceDecl"); + // We use weight of one; if we ref the var more than once, when we simplify, // the weight will increase if (nodep->access().isWriteOrRW()) m_graphp->addEdge(m_logicVertexp, vVtxp, 1); diff --git a/src/V3Inst.cpp b/src/V3Inst.cpp index 5b754d13f..3b4c40fbe 100644 --- a/src/V3Inst.cpp +++ b/src/V3Inst.cpp @@ -71,6 +71,12 @@ class InstVisitor final : public VNVisitor { if (nodep->modVarp()->isInout()) { nodep->v3fatalSrc("Unsupported: Verilator is a 2-state simulator"); } else if (nodep->modVarp()->isWritable()) { + // If the destination is a simple VarRef to a primaryIO signal, + // mark the source port as primaryIO too (for trace optimization) + if (const AstNodeVarRef* const dstRefp = VN_CAST(exprp, NodeVarRef)) { + if (dstRefp->varp()->isPrimaryIO()) { nodep->modVarp()->primaryIO(true); } + } + AstNodeExpr* const rhsp = new AstVarXRef{exprp->fileline(), nodep->modVarp(), m_cellp->name(), VAccess::READ}; AstAssignW* const assp = new AstAssignW{exprp->fileline(), exprp, rhsp}; @@ -78,6 +84,13 @@ class InstVisitor final : public VNVisitor { } else if (nodep->modVarp()->isNonOutput()) { // Don't bother moving constants now, // we'll be pushing the const down to the cell soon enough. + + // If the source is a simple VarRef to a primaryIO signal, + // mark the target port as primaryIO too (for trace optimization) + if (const AstNodeVarRef* const srcRefp = VN_CAST(exprp, NodeVarRef)) { + if (srcRefp->varp()->isPrimaryIO()) { nodep->modVarp()->primaryIO(true); } + } + AstAssignW* const assp = new AstAssignW{exprp->fileline(), new AstVarXRef{exprp->fileline(), nodep->modVarp(), diff --git a/test_regress/t/t_trace_no_inline.py b/test_regress/t/t_trace_no_inline.py new file mode 100755 index 000000000..e87521998 --- /dev/null +++ b/test_regress/t/t_trace_no_inline.py @@ -0,0 +1,97 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2025 by Wilson Snyder. 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-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap +import re +import os + +test.scenarios('simulator') + +test.compile(verilator_flags2=['--trace']) + +test.execute() + +# Test that expressions are NOT inlined into trace update functions to avoid code repetition + +trace_full_file = test.obj_dir + "/" + test.vm_prefix + "__Trace__0__Slow.cpp" +trace_chg_file = test.obj_dir + "/" + test.vm_prefix + "__Trace__0.cpp" + +if test.vlt_all: + if os.path.exists(trace_full_file): + with open(trace_full_file, 'r') as f: + trace_content = f.read() + + # Search for inlined expressions, e.g.: bufp->fullBit(oldp+X,((1U & (~ (IData)(vlSelfRef.clk))))); + inlined_not = re.findall(r'fullBit\([^)]*\(\(1U & \(\~', trace_content) + inlined_arithmetic = re.findall(r'fullCData\([^)]*\(\(.*\+.*\)', trace_content) + complex_inlined = re.findall(r'full(?:CData|SData)\([^)]*\([^)]*\+[^)]*\)\s*\*', + trace_content) + deeply_nested_inlined = re.findall(r'full(?:IData|QData)\([^)]*\{[^}]*\{[^}]*\}', + trace_content) + + if len(inlined_not) > 0: + test.error(f"Found {len(inlined_not)} inlined NOT expressions in trace full functions") + + if len(inlined_arithmetic) > 0: + test.error( + f"Found {len(inlined_arithmetic)} inlined arithmetic expressions in trace full functions" + ) + + if len(complex_inlined) > 0: + test.error( + f"Found {len(complex_inlined)} complex inlined multiplication expressions in trace full functions" + ) + + if len(deeply_nested_inlined) > 0: + test.error( + f"Found {len(deeply_nested_inlined)} deeply nested inlined expressions in trace full functions" + ) + + # Search for variable references + var_refs_simple = re.findall( + r'fullBit\(oldp\+\d+,\(vlSelfRef\.t__DOT__.*__DOT__simple_not\)\)', trace_content) + var_refs_add = re.findall( + r'fullCData\(oldp\+\d+,\(vlSelfRef\.t__DOT__.*__DOT__add_result\)', trace_content) + var_refs_mul = re.findall( + r'fullCData\(oldp\+\d+,\(vlSelfRef\.t__DOT__.*__DOT__mul_result\)', trace_content) + + if len(var_refs_simple) == 0: + test.error( + "No variable references found for simple_not signal in trace code - expressions are probably inlined instead" + ) + + if len(var_refs_add) == 0: + test.error( + "No variable references found for add_result signal in trace code - expressions are probably inlined instead" + ) + + if len(var_refs_mul) == 0: + test.error( + "No variable references found for mul_result signal in trace code - expressions are probably inlined instead" + ) + + # Check `chg` functions + if os.path.exists(trace_chg_file): + with open(trace_chg_file, 'r') as f: + trace_chg_content = f.read() + + # Search for inlined expressions, e.g.: chgBit(oldp+X,((1U & (~ ...)))) + chg_inlined_not = re.findall(r'chgBit\([^)]*\(\(1U & \(\~', trace_chg_content) + chg_inlined_arith = re.findall(r'chgCData\([^)]*\(\(.*\+.*\)', trace_chg_content) + + if len(chg_inlined_not) > 0: + test.error( + f"Found {len(chg_inlined_not)} inlined NOT expressions in trace chg functions") + + if len(chg_inlined_arith) > 0: + test.error( + f"Found {len(chg_inlined_arith)} inlined arithmetic expressions in trace chg functions" + ) + +test.passes() diff --git a/test_regress/t/t_trace_no_inline.v b/test_regress/t/t_trace_no_inline.v new file mode 100644 index 000000000..7a6d31bbb --- /dev/null +++ b/test_regress/t/t_trace_no_inline.v @@ -0,0 +1,54 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2025 by Antmicro. +// SPDX-License-Identifier: CC0-1.0 + +parameter INSTANCES = 20; + +module t (clk); + input clk; + + generate + for (genvar i = 0; i < INSTANCES; ++i) sub sub (.*); + endgenerate + + always @(posedge clk) begin + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule + +module sub(input clk); + logic simple_not; + assign simple_not = ~clk; + + logic [7:0] counter; + logic [7:0] add_result; + logic [7:0] mul_result; + assign add_result = counter + 8'd5; + assign mul_result = add_result * 8'd3; + + logic [15:0] complex_a, complex_b; + logic [31:0] complex_result; + assign complex_result = {16'd0, {add_result, mul_result}} ^ + ({{16'd0}, {complex_a[7:0], complex_a[15:8]}} + + {{16'd0}, {complex_b[7:0], complex_b[15:8]}}); + + logic [15:0] conditional_result; + assign conditional_result = (counter > 8'd100) ? + ({add_result, mul_result} & 16'hFF00) : + ({mul_result, add_result} | 16'h00FF); + + // Multi-level logic + logic stage1, stage2, stage3; + assign stage1 = (counter[0] ^ counter[1]) & clk; + assign stage2 = (stage1 | counter[2]) ^ counter[3]; + assign stage3 = stage2 & (counter[4] | ~counter[5]); + + always @(posedge clk) begin + counter <= counter + 1; + complex_a <= complex_a + 16'd7; + complex_b <= complex_b + 16'd13; + end +endmodule From 5ec6570fe73547c1d39afb229c7960ee6c233dd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Chmiel?= Date: Thu, 11 Dec 2025 16:13:58 +0100 Subject: [PATCH 2/2] Fix python lints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bartłomiej Chmiel --- test_regress/t/t_trace_no_inline.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test_regress/t/t_trace_no_inline.py b/test_regress/t/t_trace_no_inline.py index e87521998..a7b54735a 100755 --- a/test_regress/t/t_trace_no_inline.py +++ b/test_regress/t/t_trace_no_inline.py @@ -24,7 +24,7 @@ trace_chg_file = test.obj_dir + "/" + test.vm_prefix + "__Trace__0.cpp" if test.vlt_all: if os.path.exists(trace_full_file): - with open(trace_full_file, 'r') as f: + with open(trace_full_file, 'r', encoding="utf-8") as f: trace_content = f.read() # Search for inlined expressions, e.g.: bufp->fullBit(oldp+X,((1U & (~ (IData)(vlSelfRef.clk))))); @@ -78,7 +78,7 @@ if test.vlt_all: # Check `chg` functions if os.path.exists(trace_chg_file): - with open(trace_chg_file, 'r') as f: + with open(trace_chg_file, 'r', encoding='utf-8') as f: trace_chg_content = f.read() # Search for inlined expressions, e.g.: chgBit(oldp+X,((1U & (~ ...))))