Do not emit temporaries for atomic assignments.

Added test for a particularly convoluted case requiring fixup in
V3Premit. To help with statistics stability, also prevent V3Premit from
introducing temporaries for assignment where the RHS reads the LHS, but
the assignment is known to be atomic (by emitted C++ semantics).

Also rename `createWideTemp` to `createTemp`, as it is used for non-wide
expressions as well.

Prep for #6216
This commit is contained in:
Geza Lore 2025-07-23 08:13:37 +01:00
parent 344fabf56a
commit 26ffe1ac09
4 changed files with 74 additions and 9 deletions

View File

@ -46,6 +46,7 @@ class PremitVisitor final : public VNVisitor {
// STATE - across all visitors
VDouble0 m_extractedToConstPool; // Statistic tracking
VDouble0 m_temporaryVarsCreated; // Statistic tracking
// STATE - for current visit position (use VL_RESTORER)
AstCFunc* m_cfuncp = nullptr; // Current block
@ -62,12 +63,12 @@ class PremitVisitor final : public VNVisitor {
if (!nodep->isWide()) return; // Not wide
if (m_assignLhs) return; // This is an lvalue!
UASSERT_OBJ(!VN_IS(nodep->firstAbovep(), ArraySel), nodep, "Should have been ignored");
createWideTemp(nodep);
createTemp(nodep);
}
AstVar* createWideTemp(AstNodeExpr* nodep) {
AstVar* createTemp(AstNodeExpr* nodep) {
UASSERT_OBJ(m_stmtp, nodep, "Attempting to create temporary with no insertion point");
UINFO(4, "createWideTemp: " << nodep);
UINFO(4, "createTemp: " << nodep);
VNRelinker relinker;
nodep->unlinkFrBack(&relinker);
@ -93,6 +94,7 @@ class PremitVisitor final : public VNVisitor {
const std::string name = "__Vtemp_" + std::to_string(++m_tmpVarCnt);
varp = new AstVar{flp, VVarType::STMTTEMP, name, nodep->dtypep()};
m_cfuncp->addInitsp(varp);
++m_temporaryVarsCreated;
// Put assignment before the referencing statement
assignp = new AstAssign{flp, new AstVarRef{flp, varp, VAccess::WRITE}, nodep};
@ -221,9 +223,10 @@ class PremitVisitor final : public VNVisitor {
}
}
if (rhsReadsLhs(nodep)) {
// Need to do this even if not wide, as e.g. a select may be on a wide operator
createWideTemp(nodep->rhsp());
// If the RHS reads the LHS, we need a temporary unless the update is atomic
const bool isAtomic = VN_IS(nodep->lhsp(), VarRef) && !nodep->lhsp()->isWide();
if (!isAtomic && rhsReadsLhs(nodep)) {
createTemp(nodep->rhsp());
} else {
iterateAndNextNull(nodep->rhsp());
}
@ -290,7 +293,7 @@ class PremitVisitor final : public VNVisitor {
void visit(AstCvtPackedToArray* nodep) override {
iterateChildren(nodep);
checkNode(nodep);
if (!VN_IS(nodep->backp(), NodeAssign)) createWideTemp(nodep);
if (!VN_IS(nodep->backp(), NodeAssign)) createTemp(nodep);
}
void visit(AstCvtUnpackedToQueue* nodep) override {
iterateChildren(nodep);
@ -330,7 +333,7 @@ class PremitVisitor final : public VNVisitor {
&& !VN_IS(nodep->condp(), VarRef)) {
// We're going to need the expression several times in the expanded code,
// so might as well make it a common expression
createWideTemp(nodep->condp());
createTemp(nodep->condp());
VIsCached::clearCacheTree();
}
checkNode(nodep);
@ -342,7 +345,7 @@ class PremitVisitor final : public VNVisitor {
for (AstNodeExpr *expp = nodep->exprsp(), *nextp; expp; expp = nextp) {
nextp = VN_AS(expp->nextp(), NodeExpr);
if (expp->isString() && !VN_IS(expp, VarRef)) {
AstVar* const varp = createWideTemp(expp);
AstVar* const varp = createTemp(expp);
// Do not remove VarRefs to this in V3Const
varp->noSubst(true);
}
@ -360,6 +363,8 @@ public:
~PremitVisitor() override {
V3Stats::addStat("Optimizations, Prelim extracted value to ConstPool",
m_extractedToConstPool);
V3Stats::addStat("Optimizations, Prelim temporary variables created",
m_temporaryVarsCreated);
}
};

View File

@ -0,0 +1,12 @@
// -*- mode: C++; c-file-style: "cc-mode" -*-
//
// DESCRIPTION: Verilator: Verilog Test module
//
// Copyright 2025 by Geza Lore. 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
//*************************************************************************
extern "C" int identity(int x) { return x; }

View File

@ -0,0 +1,22 @@
#!/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
test.scenarios('simulator')
test.compile(
verilator_flags2=["--exe", "--main", "--timing", "--stats", "t/t_wide_temp_while_cond.cpp"])
if test.vlt or test.vltmt:
test.file_grep(test.stats, r'Optimizations, Prelim temporary variables created\s+(\d+)', 3)
test.execute()
test.passes()

View File

@ -0,0 +1,26 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty, 2025 by Geza Lore.
// SPDX-License-Identifier: CC0-1.0
import "DPI-C" pure function int identity(input int value);
module t;
initial begin
int n = 0;
logic [127:0] val = 128'b1;
logic [15:0] one = 16'b1;
// This condition involves multiple wide temporaries, and an over-width
// shift, all of which requires V3Premit to fix up.
while (|((val[ 7'(one >> identity(32)) +: 96] << n) >> n)) begin
++n;
end
$display("n=%0d", n);
if (n != 96) $stop;
$write("*-* All Finished *-*\n");
$finish;
end
endmodule