From 125bdb45f51b9520cd0e7ff14f18f3e921ea5e4f Mon Sep 17 00:00:00 2001 From: Nick Brereton <85175726+nbstrike@users.noreply.github.com> Date: Fri, 29 May 2026 19:51:27 -0400 Subject: [PATCH] Support pre/post increment/decrement inside && and || (#7683) --- src/V3LinkInc.cpp | 87 ++++++++++++-- test_regress/t/t_increment_bad.out | 29 ++--- test_regress/t/t_increment_bad.v | 4 + test_regress/t/t_math_postinc.py | 18 +++ test_regress/t/t_math_postinc.v | 182 +++++++++++++++++++++++++++++ 5 files changed, 298 insertions(+), 22 deletions(-) create mode 100755 test_regress/t/t_math_postinc.py create mode 100644 test_regress/t/t_math_postinc.v diff --git a/src/V3LinkInc.cpp b/src/V3LinkInc.cpp index 6c7e058e2..f8e3c6138 100644 --- a/src/V3LinkInc.cpp +++ b/src/V3LinkInc.cpp @@ -54,12 +54,17 @@ VL_DEFINE_DEBUG_FUNCTIONS; //###################################################################### class LinkIncVisitor final : public VNVisitor { + // NODE STATE + // AstLogAnd/AstLogOr::user1() -> bool. True if already lowered + const VNUser1InUse m_inuser1; + // STATE AstNodeFTask* m_ftaskp = nullptr; // Function or task we're inside AstNodeModule* m_modp = nullptr; // Module we're inside int m_modCompoundAssignmentsNum = 0; // Var name counter AstNode* m_insStmtp = nullptr; // Where to insert statement - bool m_unsupportedHere = false; // Used to detect where it's not supported yet + bool m_condEvalContext = false; // ++/-- is in a conditionally-evaluated position + AstNodeExpr* m_incCondp = nullptr; // Gating condition for ++/-- in short-circuit context // METHODS void insertOnTop(AstNode* newp) { @@ -144,8 +149,8 @@ class LinkIncVisitor final : public VNVisitor { } void visit(AstCaseItem* nodep) override { { - VL_RESTORER(m_unsupportedHere); - m_unsupportedHere = true; + VL_RESTORER(m_condEvalContext); + m_condEvalContext = true; iterateAndNextNull(nodep->condsp()); } m_insStmtp = nullptr; // Next thing should be new statement @@ -195,13 +200,64 @@ class LinkIncVisitor final : public VNVisitor { m_insStmtp = nullptr; // Next thing should be new statement } void unsupported_visit(AstNode* nodep) { - VL_RESTORER(m_unsupportedHere); - m_unsupportedHere = true; + VL_RESTORER(m_condEvalContext); + VL_RESTORER(m_incCondp); + m_condEvalContext = true; + // Not rescuable by short-circuit gating; drop the gate so nested ++/-- errors + m_incCondp = nullptr; UINFO(9, "Marking unsupported " << nodep); iterateChildren(nodep); } - void visit(AstLogAnd* nodep) override { unsupported_visit(nodep); } - void visit(AstLogOr* nodep) override { unsupported_visit(nodep); } + // Hoist LHS into a BLOCKTEMP so it is evaluated exactly once; otherwise the + // gated RHS ++/-- could modify a variable the LHS reads. + AstNodeExpr* captureLogicalLhsToTemp(AstNodeBiop* const nodep) { + FileLine* const fl = nodep->fileline(); + AstNodeExpr* const lhsp = nodep->lhsp()->unlinkFrBack(); + const string name = "__VincGate"s + cvtToStr(++m_modCompoundAssignmentsNum); + AstVar* const varp = new AstVar{ + fl, VVarType::BLOCKTEMP, name, VFlagChildDType{}, + new AstRefDType{fl, AstRefDType::FlagTypeOfExpr{}, lhsp->cloneTree(true)}}; + varp->lifetime(VLifetime::AUTOMATIC_EXPLICIT); + if (m_ftaskp) varp->funcLocal(true); + insertOnTop(varp); + AstNode* tempInitp = new AstAssign{fl, new AstVarRef{fl, varp, VAccess::WRITE}, lhsp}; + // Gate by the enclosing condition so a side-effecting LHS isn't run on short-circuit + if (m_incCondp) { + tempInitp = new AstIf{fl, m_incCondp->cloneTreePure(true), tempInitp, nullptr}; + } + insertBeforeStmt(nodep, tempInitp); + nodep->lhsp(new AstVarRef{fl, varp, VAccess::READ}); + return new AstVarRef{fl, varp, VAccess::READ}; + } + void handleShortCircuit(AstNodeBiop* const nodep, bool isOr) { + // insertBeforeStmt retargets the parent iterator; skip on re-visit + if (nodep->user1SetOnce()) return; + VL_RESTORER(m_condEvalContext); + VL_RESTORER(m_incCondp); + iterateAndNextNull(nodep->lhsp()); + m_condEvalContext = true; + AstNodeExpr* ownedCondp = nullptr; + // Gate the RHS by the LHS condition if it evaluates any ++/-- anywhere within + if (nodep->rhsp()->exists([](const AstNode* np) { + return VN_IS(np, PreInc) || VN_IS(np, PreDec) || VN_IS(np, PostInc) + || VN_IS(np, PostDec) || VN_IS(np, AssignCompound); + })) { + // Const LHS or no statement context: clone-and-evaluate-twice is safe + AstNodeExpr* lhsRefp = (!m_insStmtp || VN_IS(nodep->lhsp(), Const)) + ? nodep->lhsp()->cloneTreePure(true) + : captureLogicalLhsToTemp(nodep); + // For ||, RHS runs when LHS is false; gate by !LHS + if (isOr) lhsRefp = new AstLogNot{nodep->fileline(), lhsRefp}; + ownedCondp = m_incCondp ? new AstLogAnd{nodep->fileline(), + m_incCondp->cloneTreePure(true), lhsRefp} + : lhsRefp; + m_incCondp = ownedCondp; + } + iterateAndNextNull(nodep->rhsp()); + if (ownedCondp) VL_DO_DANGLING(ownedCondp->deleteTree(), ownedCondp); + } + void visit(AstLogAnd* nodep) override { handleShortCircuit(nodep, /*isOr=*/false); } + void visit(AstLogOr* nodep) override { handleShortCircuit(nodep, /*isOr=*/true); } void visit(AstLogEq* nodep) override { unsupported_visit(nodep); } void visit(AstLogIf* nodep) override { unsupported_visit(nodep); } void visit(AstCond* nodep) override { unsupported_visit(nodep); } @@ -338,11 +394,12 @@ class LinkIncVisitor final : public VNVisitor { } void prepost_expr_visit(AstNodeUniop* const nodep) { iterateChildren(nodep); - if (m_unsupportedHere) { + if (m_condEvalContext && !m_incCondp) { nodep->v3warn(E_UNSUPPORTED, "Unsupported: Pre/post increment/decrement operator" " within a logical expression (&&, ||, ?:, etc.)"); return; } + const bool needGating = m_condEvalContext && m_incCondp; AstNodeExpr* const readp = nodep->lhsp(); AstNodeExpr* const writep = nodep->lhsp()->cloneTreePure(true); V3LinkLValue::linkLValueSet(readp, false); @@ -365,7 +422,19 @@ class LinkIncVisitor final : public VNVisitor { // Define what operation will we be doing AstNodeExpr* const operp = getOperationp(nodep, readp->cloneTreePure(true), newconstp); - if (VN_IS(nodep, PreInc) || VN_IS(nodep, PreDec)) { + if (needGating) { + // Short-circuit context: mutate the variable only when the gate is true. + // The temp holds the old value, plus the new value for pre-inc/dec. + AstAssign* const assignp = new AstAssign{fl, new AstVarRef{fl, varp, VAccess::WRITE}, + readp->cloneTreePure(true)}; + AstNode* const incp = new AstAssign{fl, writep, operp}; + if (VN_IS(nodep, PreInc) || VN_IS(nodep, PreDec)) { + incp->addNext(new AstAssign{fl, new AstVarRef{fl, varp, VAccess::WRITE}, + readp->cloneTreePure(true)}); + } + assignp->addNextHere(new AstIf{fl, m_incCondp->cloneTreePure(true), incp, nullptr}); + insertBeforeStmt(nodep, assignp); + } else if (VN_IS(nodep, PreInc) || VN_IS(nodep, PreDec)) { // PreInc/PreDec operations // Immediately after declaration - increment it by one AstAssign* const assignp diff --git a/test_regress/t/t_increment_bad.out b/test_regress/t/t_increment_bad.out index 3534f7d84..c2cedb1cb 100644 --- a/test_regress/t/t_increment_bad.out +++ b/test_regress/t/t_increment_bad.out @@ -1,23 +1,26 @@ -%Error-UNSUPPORTED: t/t_increment_bad.v:19:29: Unsupported: Pre/post increment/decrement operator within a logical expression (&&, ||, ?:, etc.) - 19 | if (0 && test_string[pos++] != "e"); - | ^~ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest -%Error-UNSUPPORTED: t/t_increment_bad.v:20:17: Unsupported: Pre/post increment/decrement operator within a logical expression (&&, ||, ?:, etc.) - 20 | if (1 || pos-- != 1); - | ^~ %Error-UNSUPPORTED: t/t_increment_bad.v:22:15: Unsupported: Pre/post increment/decrement operator within a logical expression (&&, ||, ?:, etc.) 22 | if (a <-> --b); | ^~ + ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest %Error-UNSUPPORTED: t/t_increment_bad.v:23:14: Unsupported: Pre/post increment/decrement operator within a logical expression (&&, ||, ?:, etc.) 23 | if (0 -> ++b); | ^~ -%Error-UNSUPPORTED: t/t_increment_bad.v:25:22: Unsupported: Pre/post increment/decrement operator within a logical expression (&&, ||, ?:, etc.) - 25 | pos = (a > 0) ? a++ : --b; +%Error-UNSUPPORTED: t/t_increment_bad.v:26:24: Unsupported: Pre/post increment/decrement operator within a logical expression (&&, ||, ?:, etc.) + 26 | if (1 && (a > 0 ? a++ : --b)); + | ^~ +%Error-UNSUPPORTED: t/t_increment_bad.v:26:29: Unsupported: Pre/post increment/decrement operator within a logical expression (&&, ||, ?:, etc.) + 26 | if (1 && (a > 0 ? a++ : --b)); + | ^~ +%Error-UNSUPPORTED: t/t_increment_bad.v:27:24: Unsupported: Pre/post increment/decrement operator within a logical expression (&&, ||, ?:, etc.) + 27 | if (0 || (a > 0 -> ++b)); + | ^~ +%Error-UNSUPPORTED: t/t_increment_bad.v:29:22: Unsupported: Pre/post increment/decrement operator within a logical expression (&&, ||, ?:, etc.) + 29 | pos = (a > 0) ? a++ : --b; | ^~ -%Error-UNSUPPORTED: t/t_increment_bad.v:25:27: Unsupported: Pre/post increment/decrement operator within a logical expression (&&, ||, ?:, etc.) - 25 | pos = (a > 0) ? a++ : --b; +%Error-UNSUPPORTED: t/t_increment_bad.v:29:27: Unsupported: Pre/post increment/decrement operator within a logical expression (&&, ||, ?:, etc.) + 29 | pos = (a > 0) ? a++ : --b; | ^~ -%Error-UNSUPPORTED: t/t_increment_bad.v:30:36: Unsupported: Pre/post increment/decrement operator within a logical expression (&&, ||, ?:, etc.) - 30 | assert property (@(posedge clk) a++ >= 0); +%Error-UNSUPPORTED: t/t_increment_bad.v:34:36: Unsupported: Pre/post increment/decrement operator within a logical expression (&&, ||, ?:, etc.) + 34 | assert property (@(posedge clk) a++ >= 0); | ^~ %Error: Exiting due to diff --git a/test_regress/t/t_increment_bad.v b/test_regress/t/t_increment_bad.v index 2087b5b18..ea35d766e 100644 --- a/test_regress/t/t_increment_bad.v +++ b/test_regress/t/t_increment_bad.v @@ -22,6 +22,10 @@ module t ( if (a <-> --b); if (0 -> ++b); + // ++/-- nested in ?:/-> inside a supported &&/|| must still error + if (1 && (a > 0 ? a++ : --b)); + if (0 || (a > 0 -> ++b)); + pos = (a > 0) ? a++ : --b; pos = array[0][0]++; diff --git a/test_regress/t/t_math_postinc.py b/test_regress/t/t_math_postinc.py new file mode 100755 index 000000000..8a938befd --- /dev/null +++ b/test_regress/t/t_math_postinc.py @@ -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() + +test.execute() + +test.passes() diff --git a/test_regress/t/t_math_postinc.v b/test_regress/t/t_math_postinc.v new file mode 100644 index 000000000..05a3b9ed6 --- /dev/null +++ b/test_regress/t/t_math_postinc.v @@ -0,0 +1,182 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// Test inline pre/post increment/decrement operators, including +// inside logical operators (&&, ||) with short-circuit semantics. +// +// 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 checkh(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got=%0x exp=%0x (%s !== %s)\n", `__FILE__,`__LINE__, (gotv), (expv), `"gotv`", `"expv`"); `stop; end while(0); +`define checks(gotv,expv) do if ((gotv) != (expv)) begin $write("%%Error: %s:%0d: got='%s' exp='%s'\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0); +// verilog_format: on + +module t; + integer a, b, c; + reg cond; + int side; + + // Impure (side-effecting) function for use as a short-circuit LHS + function automatic int bump(); + side = side + 1; + return 1; + endfunction + + // ++/-- in && / || inside a function/task exercises the function-local temp + function automatic int f_and(int x); + if (x > 0 && x++ < 100) ; + return x; + endfunction + function automatic int f_or(int x); + if (x > 0 || x++ < 100) ; + return x; + endfunction + // LHS reads the gated variable; must see the pre-increment value exactly once + function automatic int f_single(int x); + int taken = 0; + if (x == 4 && x++ < 100) taken = 1; + return taken * 1000 + x; + endfunction + + initial begin + // ---- Basic post/pre increment/decrement ---- + a = 5; b = a++; `checkh(b, 5); `checkh(a, 6); + a = 5; b = ++a; `checkh(a, 6); `checkh(b, 6); + a = 10; b = 5; b = a++ + b; `checkh(b, 15); `checkh(a, 11); + a = 10; b = 5; b = ++a + b; `checkh(b, 16); `checkh(a, 11); + a = 7; b = a++ * 2; `checkh(b, 14); `checkh(a, 8); + a = 7; b = ++a * 2; `checkh(b, 16); `checkh(a, 8); + a = 3; b = 4; c = a++ + b++; `checkh(c, 7); `checkh(a, 4); `checkh(b, 5); + a = 3; b = 4; c = ++a + ++b; `checkh(c, 9); `checkh(a, 4); `checkh(b, 5); + a = 10; b = 3; c = a++ - b--; `checkh(c, 7); `checkh(a, 11); `checkh(b, 2); + a = 5; b = 5; b = a--; `checkh(b, 5); `checkh(a, 4); + a = 5; b = --a; `checkh(b, 4); `checkh(a, 4); + + // ---- Post-inc in shift (5 << 5 = 160) ---- + a = 5; b = 5; b = b << a++; `checkh(b, 160); `checkh(a, 6); + + // ---- Post-inc in paren expr ---- + a = 2; b = (a++ + 1) * 3; `checkh(b, 9); `checkh(a, 3); + + // ---- Post-inc in while with && (constant) ---- + a = 0; while (1 && a++ < 3) begin end `checkh(a, 4); + + // ---- Post-inc in while with && (variable cond, non-short-circuit) ---- + cond = 1; a = 0; while (cond && a++ < 3) begin end `checkh(a, 4); + + // ---- Post-inc in while with && (variable cond, short-circuit) ---- + cond = 0; a = 0; while (cond && a++ < 3) begin end `checkh(a, 0); + + // ---- Post-inc in while with || ---- + a = 0; while (0 || a++ < 3) begin end `checkh(a, 4); + + // ---- && short-circuit ---- + a = 0; if (0 && a++ < 3) begin end `checkh(a, 0); + + // ---- && non-short-circuit ---- + a = 0; if (1 && a++ < 5) begin end `checkh(a, 1); + + // ---- || non-short-circuit ---- + a = 0; if (0 || a++ < 5) begin end `checkh(a, 1); + + // ---- || short-circuit ---- + a = 0; if (1 || a++ < 5) begin end `checkh(a, 0); + + // ---- Pre-inc with && ---- + a = 0; if (1 && ++a < 5) begin end `checkh(a, 1); + + // ---- Pre-inc short-circuit ---- + a = 0; if (0 && ++a < 5) begin end `checkh(a, 0); + + // ---- Pre-inc with || ---- + a = 0; if (0 || ++a < 5) begin end `checkh(a, 1); + a = 0; if (1 || ++a < 5) begin end `checkh(a, 0); + + // ---- Nested && chain ---- + a = 0; if (1 && 1 && a++ < 5) begin end `checkh(a, 1); + + // ---- Post-dec with && ---- + a = 5; if (1 && a-- > 0) begin end `checkh(a, 4); + + // ---- Pre-dec with && short-circuit ---- + a = 5; if (0 && --a > 0) begin end `checkh(a, 5); + + // ---- Post-dec with || ---- + a = 5; if (0 || a-- > 0) begin end `checkh(a, 4); + a = 5; if (1 || a-- > 0) begin end `checkh(a, 5); + + // ---- Pre-dec with || ---- + a = 5; if (0 || --a > 0) begin end `checkh(a, 4); + + // ---- Multiple increments in && chain ---- + a = 0; b = 0; + if (1 && a++ < 5 && b++ < 5) begin end + `checkh(a, 1); `checkh(b, 1); + + // ---- Post-inc on left side of && ---- + a = 0; if (a++ < 5 && 1) begin end `checkh(a, 1); + + // ---- Post-inc on left side of || ---- + a = 0; if (a++ < 5 || 0) begin end `checkh(a, 1); + + // ---- Pre-inc on left side of || ---- + a = 0; if (++a < 5 || 0) begin end `checkh(a, 1); + + // ---- Mixed && and || with post-inc ---- + a = 0; b = 0; + if (1 && a++ < 5 || b++ < 5) begin end + `checkh(a, 1); `checkh(b, 0); + + a = 0; b = 0; + if (0 && a++ < 5 || b++ < 5) begin end + `checkh(a, 0); `checkh(b, 1); + + // ---- Deep nesting (3 levels &&) ---- + a = 0; b = 0; c = 0; + if (1 && 1 && a++ < 5 && b++ < 5 && c++ < 5) begin end + `checkh(a, 1); `checkh(b, 1); `checkh(c, 1); + + a = 0; b = 0; c = 0; + if (1 && 0 && a++ < 5 && b++ < 5 && c++ < 5) begin end + `checkh(a, 0); `checkh(b, 0); `checkh(c, 0); + + // ---- LHS reads variable that gated RHS ++/-- writes (single-eval) ---- + a = 4; if (a == 4 && a++ < 10) begin end `checkh(a, 5); + a = 6; if (a == 4 && a++ < 10) begin end `checkh(a, 6); + a = 0; if (a < 1 && a++ < 5) begin end `checkh(a, 1); + a = 5; if (a > 0 && a-- > 0) begin end `checkh(a, 4); + a = 4; if (a == 4 && ++a < 10) begin end `checkh(a, 5); + + // ---- || mirror: LHS reads variable that gated RHS modifies ---- + a = 4; if (a != 4 || a++ < 10) begin end `checkh(a, 5); + a = 6; if (a != 4 || a++ < 10) begin end `checkh(a, 6); + + // ---- Nested gate: inner && with non-const LHS inside an outer gate ---- + a = 4; if (a == 4 && (a < 10 && a++ < 20)) begin end `checkh(a, 5); + a = 20; if (a == 20 && (a < 10 && a++ < 30)) begin end `checkh(a, 20); + a = 0; if (a == 4 && (a < 10 && a++ < 20)) begin end `checkh(a, 0); + + // ---- ++/-- in && / || inside a function (function-local temp) ---- + `checkh(f_and(5), 6); // 5>0 true, post-inc runs -> 6 + `checkh(f_and(0), 0); // 0>0 false, short-circuit -> unchanged + `checkh(f_or(5), 5); // 5>0 true, short-circuit -> unchanged + `checkh(f_or(0), 1); // 0>0 false, post-inc runs -> 1 + `checkh(f_single(4), 1005); // 4==4 (once) && 4<100 -> taken=1, x=5 + `checkh(f_single(6), 6); // 6==4 false, short-circuit -> taken=0, x=6 + + // ---- Impure (side-effecting) LHS must be evaluated exactly once ---- + side = 0; a = 0; if (bump() > 0 && a++ < 5) begin end `checkh(side, 1); `checkh(a, 1); + side = 0; a = 0; if (bump() < 0 && a++ < 5) begin end `checkh(side, 1); `checkh(a, 0); + + // ---- ++/-- on LHS, side-effecting function on opposite (RHS) side ---- + side = 0; a = 0; if (a++ < 5 && bump() > 0) begin end `checkh(a, 1); `checkh(side, 1); + side = 0; a = 0; if (a++ > 5 && bump() > 0) begin end `checkh(a, 1); `checkh(side, 0); + side = 0; a = 0; if (a++ > 5 || bump() > 0) begin end `checkh(a, 1); `checkh(side, 1); + side = 0; a = 0; if (a++ < 5 || bump() > 0) begin end `checkh(a, 1); `checkh(side, 0); + + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule