From e53d6d9006a75c2632da0e25900d275f652b167a Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Thu, 4 Jun 2026 14:57:36 +0100 Subject: [PATCH 01/32] Improve procedural loop unrolling - Enable unrolling of nested loops when the inner loop updates the outer loop condition - Enable unrolling 'for' loops with break statements --- src/V3Unroll.cpp | 153 +++++++++++++++++++------ test_regress/t/t_unroll_pragma_none.py | 2 +- test_regress/t/t_unroll_stmt.out | 29 +++++ test_regress/t/t_unroll_stmt.py | 4 +- test_regress/t/t_unroll_stmt.v | 18 +++ 5 files changed, 169 insertions(+), 37 deletions(-) diff --git a/src/V3Unroll.cpp b/src/V3Unroll.cpp index d8dd4221e..ce677324b 100644 --- a/src/V3Unroll.cpp +++ b/src/V3Unroll.cpp @@ -64,16 +64,69 @@ struct UnrollStats final { Stat m_nUnrolledIters{"Unrolled iterations"}; }; +//###################################################################### +// Variable bindings +class UnrolllBindings final { + // NODE STATE + // AstVarScope::user1() int: index of the binding in m_bindings + const VNUser1InUse m_inuser1; + + // STATE + // Map from AstVarScope::user1() to current variable value, nullptr if not bound - idx 0 unused + std::vector m_curr{nullptr}; + // Stack of binding checkpoints + std::vector> m_checkpoints; + +public: + // CONSTRUCTOR + UnrolllBindings() = default; + ~UnrolllBindings() = default; + VL_UNCOPYABLE(UnrolllBindings); + VL_UNMOVABLE(UnrolllBindings); + + // METHODS + void clear() { + m_curr.resize(1); + m_checkpoints.clear(); + AstNode::user1ClearTree(); + } + + void checkpoint() { m_checkpoints.push_back(m_curr); } + + void revert() { + m_curr = std::move(m_checkpoints.back()); + m_checkpoints.pop_back(); + } + + void commit() { m_checkpoints.pop_back(); } + + void set(AstVarScope* vscp, AstConst* valp) { + if (!vscp->user1()) { + vscp->user1(m_curr.size()); + m_curr.push_back(nullptr); + } + UINFO(6, "Binding SET " << vscp->name() << " / " << vscp->user1() << " := " << valp); + m_curr[vscp->user1()] = valp; + } + + AstConst* get(AstVarScope* vscp) { + // It's possible after a revert that user1 is set, but the vector is shorter, pad up + if (static_cast(vscp->user1()) >= m_curr.size()) { + m_curr.resize(vscp->user1() + 1, nullptr); + } + AstConst* const valp = vscp->user1() ? m_curr[vscp->user1()] : nullptr; + UINFO(6, "Binding GET " << vscp->name() << " / " << vscp->user1() << " == " << valp); + return valp; + } +}; + //###################################################################### // Unroll one AstLoop class UnrollOneVisitor final : VNVisitor { - // NODE STATE - // AstVarScope::user1p() AstConst: Value of this AstVarScope - const VNUser1InUse m_user1InUse; - // STATE UnrollStats& m_stats; // Statistics tracking + UnrolllBindings& m_bindings; // Variable bindings AstLoop* const m_loopp; // The loop we are trying to unroll AstNode* m_stmtsp = nullptr; // Resulting unrolled statement size_t m_unrolledSize = 0; // Number of nodes in unrolled loop @@ -204,11 +257,11 @@ class UnrollOneVisitor final : VNVisitor { AstVarRef* const refp = VN_CAST(np, VarRef); if (!refp) return; // Ignore if the referenced variable has no binding - AstConst* const valp = VN_AS(refp->varScopep()->user1p(), Const); + AstConst* const valp = m_bindings.get(refp->varScopep()); if (!valp) return; // If writen, remove the binding if (refp->access().isWriteOrRW()) { - refp->varScopep()->user1p(nullptr); + m_bindings.set(refp->varScopep(), nullptr); return; } // Otherwise add it to the list of variables to substitute @@ -219,7 +272,7 @@ class UnrollOneVisitor final : VNVisitor { // Actually substitute the variables that still have bindings for (AstVarRef* const refp : toSubstitute) { // Pick up bound value - AstConst* const valp = VN_AS(refp->varScopep()->user1p(), Const); + AstConst* const valp = m_bindings.get(refp->varScopep()); // Binding might have been removed after adding to 'toSubstitute' if (!valp) continue; // Substitute it @@ -230,8 +283,9 @@ class UnrollOneVisitor final : VNVisitor { } // CONSTRUCTOR - UnrollOneVisitor(UnrollStats& stats, AstLoop* loopp) + UnrollOneVisitor(UnrollStats& stats, UnrolllBindings& bindings, AstLoop* loopp) : m_stats{stats} + , m_bindings{bindings} , m_loopp{loopp} { UASSERT_OBJ(!loopp->contsp(), loopp, "'contsp' only used before LinkJump"); // Do not unroll if we are told not to @@ -239,25 +293,9 @@ class UnrollOneVisitor final : VNVisitor { cantUnroll(loopp, m_stats.m_nPragmaDisabled); return; } - // Gather variable bindings from the preceding statements - for (AstNode *succp = loopp, *currp = loopp->backp(); currp->nextp() == succp; - succp = currp, currp = currp->backp()) { - AstAssign* const assignp = VN_CAST(currp, Assign); - if (!assignp) break; - AstConst* const valp = VN_CAST(V3Const::constifyEdit(assignp->rhsp()), Const); - if (!valp) break; - AstVarRef* const lhsp = VN_CAST(assignp->lhsp(), VarRef); - if (!lhsp) break; - // Don't bind if volatile - if (lhsp->varp()->isForced() || lhsp->varp()->isSigUserRWPublic()) continue; - // Don't overwrite a later binding - if (lhsp->varScopep()->user1p()) continue; - // Set up the binding - lhsp->varScopep()->user1p(valp); - } // Attempt to unroll the loop - const size_t iterLimit - = m_unrollFull ? v3Global.opt.unrollLimit() : v3Global.opt.unrollCount(); + const size_t iterLimit = m_unrollFull ? v3Global.opt.unrollLimit() // + : v3Global.opt.unrollCount(); size_t iterCount = 0; do { if (iterCount > iterLimit) { @@ -316,7 +354,7 @@ class UnrollOneVisitor final : VNVisitor { AstVarRef* const lhsp = VN_CAST(nodep->lhsp(), VarRef); AstConst* const valp = VN_CAST(V3Const::constifyEdit(nodep->rhsp()), Const); if (lhsp && valp && !lhsp->varp()->isForced() && !lhsp->varp()->isSigUserRWPublic()) { - lhsp->varScopep()->user1p(valp); + m_bindings.set(lhsp->varScopep(), valp); return; } // Otherwise just like a generic statement @@ -340,12 +378,32 @@ class UnrollOneVisitor final : VNVisitor { // Remove trailing dead code if (nodep->nextp()) pushDeletep(nodep->nextp()->unlinkFrBackWithNext()); } + void visit(AstLoop* nodep) override { + m_bindings.checkpoint(); + std::pair pair = UnrollOneVisitor::apply(m_stats, m_bindings, nodep); + + // If failed, revert the bindings and process the loop as a generic statement + if (!pair.second) { + m_bindings.revert(); + process(nodep); + return; + } + + // Commit the bindings and replace the loop with the unrolled code + m_bindings.commit(); + if (pair.first) { + nodep->replaceWith(pair.first); + } else { + nodep->unlinkFrBack(); + } + VL_DO_DANGLING(pushDeletep(nodep), nodep); + } public: - // Unroll the given loop. Returns the resulting statements and the number of - // iterations unrolled (0 if unrolling failed); - static std::pair apply(UnrollStats& stats, AstLoop* loopp) { - UnrollOneVisitor visitor{stats, loopp}; + // Unroll the given loop. Returns the resulting statements and a flag indicating success + static std::pair apply(UnrollStats& stats, UnrolllBindings& bindings, + AstLoop* loopp) { + UnrollOneVisitor visitor{stats, bindings, loopp}; // If successfully unrolled, return the resulting list of statements - might be empty if (visitor.m_ok) return {visitor.m_stmtsp, true}; // Otherwise delete intermediate results @@ -358,13 +416,40 @@ public: // Unroll all AstLoop statements class UnrollAllVisitor final : VNVisitor { - // STATE - Statistic tracking - UnrollStats m_stats; + // STATE + UnrollStats m_stats; // Statistic tracking + UnrolllBindings m_bindings; // Variable bindings // VISIT void visit(AstLoop* nodep) override { + // Gather variable bindings from the preceding statements + m_bindings.clear(); + for (AstNode *succp = nodep, *currp = nodep->backp(); true; + succp = currp, currp = currp->backp()) { + // If the current statement is in higher list, proceed carefully + if (currp->nextp() != succp) { + // Jump block is OK, there is only one way to get here + if (VN_IS(currp, JumpBlock)) continue; + // TODO: if? + // Otherwise we dont' know the control flow, give up + break; + } + AstAssign* const assignp = VN_CAST(currp, Assign); + if (!assignp) break; + AstConst* const valp = VN_CAST(V3Const::constifyEdit(assignp->rhsp()), Const); + if (!valp) break; + AstVarRef* const lhsp = VN_CAST(assignp->lhsp(), VarRef); + if (!lhsp) break; + // Don't bind if volatile + if (lhsp->varp()->isForced() || lhsp->varp()->isSigUserRWPublic()) continue; + // Don't overwrite a later binding + if (m_bindings.get(lhsp->varScopep())) continue; + // Set up the binding + m_bindings.set(lhsp->varScopep(), valp); + } + // Attempt to unroll this loop - const std::pair pair = UnrollOneVisitor::apply(m_stats, nodep); + const std::pair pair = UnrollOneVisitor::apply(m_stats, m_bindings, nodep); // If failed, carry on with nested loop if (!pair.second) { diff --git a/test_regress/t/t_unroll_pragma_none.py b/test_regress/t/t_unroll_pragma_none.py index 24144ca3f..ea66b1ba4 100755 --- a/test_regress/t/t_unroll_pragma_none.py +++ b/test_regress/t/t_unroll_pragma_none.py @@ -20,6 +20,6 @@ test.compile(verilator_flags2=['--unroll-count 4 --unroll-stmts 9999 --stats -DT test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled loops\s+(\d+)', 5) test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled iterations\s+(\d+)', 25) test.file_grep(test.stats, - r'Optimizations, Loop unrolling, Failed - reached --unroll-count\s+(\d+)', 2) + r'Optimizations, Loop unrolling, Failed - reached --unroll-count\s+(\d+)', 7) test.passes() diff --git a/test_regress/t/t_unroll_stmt.out b/test_regress/t/t_unroll_stmt.out index 50d32c577..69c4f7630 100644 --- a/test_regress/t/t_unroll_stmt.out +++ b/test_regress/t/t_unroll_stmt.out @@ -31,4 +31,33 @@ loop_6 3 loop_6 4 loop_6 5 stopping loop_6 +loop_7 0 +loop_8 0 +loop_8 1 +loop_8 2 +loop_8 3 +loop_7 1 +loop_8 0 +loop_8 1 +loop_8 2 +loop_8 3 +loop_7 2 +loop_8 0 +loop_8 1 +loop_8 2 +loop_8 3 +loop_7 3 +loop_8 0 +loop_8 1 +loop_8 2 +loop_8 3 +loop_7 4 +loop_8 0 +loop_8 1 +loop_8 2 +loop_8 3 +loop_9 0 +loop_9 1 +loop_9 2 +loop_9 3 *-* All Finished *-* diff --git a/test_regress/t/t_unroll_stmt.py b/test_regress/t/t_unroll_stmt.py index d7b6c0bfe..6d15f7c2e 100755 --- a/test_regress/t/t_unroll_stmt.py +++ b/test_regress/t/t_unroll_stmt.py @@ -26,7 +26,7 @@ test.file_grep(test.stats, test.file_grep(test.stats, r'Optimizations, Loop unrolling, Failed - unknown loop condition\s+(\d+)', 0) test.file_grep(test.stats, r'Optimizations, Loop unrolling, Pragma unroll_disable\s+(\d+)', 0) -test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled loops\s+(\d+)', 6) -test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled iterations\s+(\d+)', 40) +test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled loops\s+(\d+)', 13) +test.file_grep(test.stats, r'Optimizations, Loop unrolling, Unrolled iterations\s+(\d+)', 77) test.passes() diff --git a/test_regress/t/t_unroll_stmt.v b/test_regress/t/t_unroll_stmt.v index 2b651df4b..a40495dfd 100644 --- a/test_regress/t/t_unroll_stmt.v +++ b/test_regress/t/t_unroll_stmt.v @@ -12,6 +12,8 @@ module t; return ++static_loop_cond < 8; endfunction + int nonConst3 = $c("3"); + initial begin // Basic loop for (int i = 0; i < 3; ++i) begin : loop_0 @@ -59,6 +61,22 @@ module t; end end end + // Outer condition updated in inner loop + begin + automatic int i = 0; + while (i < 5) begin : loop_7 + $display("loop_7 %0d", i); + for (int j = 0 ; j < 4; ++j) begin : loop_8 + $display("loop_8 %0d", j); + if (j == 3) ++i; + end + end + end + // Data dependent break + for (int i = 0; i < 5; ++i) begin : loop_9 + $display("loop_9 %0d", i); + if (i == nonConst3) break; + end // Done $write("*-* All Finished *-*\n"); $finish; From ca376d681adb42ca8fc77685129b25b823ab223b Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Fri, 5 Jun 2026 12:56:46 +0100 Subject: [PATCH 02/32] Optimize $onehot in Dfg --- src/V3AstNodeExpr.h | 2 ++ src/V3Dfg.cpp | 4 +++- src/V3DfgCse.cpp | 4 ++++ src/V3DfgPeephole.cpp | 10 ++++++++++ test_regress/t/t_dfg_peephole.v | 6 ++++++ 5 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/V3AstNodeExpr.h b/src/V3AstNodeExpr.h index 7d4a3842d..06cda4604 100644 --- a/src/V3AstNodeExpr.h +++ b/src/V3AstNodeExpr.h @@ -5784,6 +5784,7 @@ public: }; class AstOneHot final : public AstNodeUniop { // True if only single bit set in vector + // @astgen makeDfgVertex public: AstOneHot(FileLine* fl, AstNodeExpr* lhsp) : ASTGEN_SUPER_OneHot(fl, lhsp) { @@ -5801,6 +5802,7 @@ public: }; class AstOneHot0 final : public AstNodeUniop { // True if only single bit, or no bits set in vector + // @astgen makeDfgVertex public: AstOneHot0(FileLine* fl, AstNodeExpr* lhsp) : ASTGEN_SUPER_OneHot0(fl, lhsp) { diff --git a/src/V3Dfg.cpp b/src/V3Dfg.cpp index df68246aa..d51250bc3 100644 --- a/src/V3Dfg.cpp +++ b/src/V3Dfg.cpp @@ -774,7 +774,9 @@ void DfgVertex::typeCheck(const DfgGraph& dfg) const { case VDfgType::LogNot: case VDfgType::RedAnd: case VDfgType::RedOr: - case VDfgType::RedXor: { + case VDfgType::RedXor: + case VDfgType::OneHot: + case VDfgType::OneHot0: { CHECK(dtype() == DfgDataType::packed(1), "Should be 1-bit"); CHECK(inputp(0)->isPacked(), "Operand should be Packed type"); return; diff --git a/src/V3DfgCse.cpp b/src/V3DfgCse.cpp index abce554ba..40aa85190 100644 --- a/src/V3DfgCse.cpp +++ b/src/V3DfgCse.cpp @@ -110,6 +110,8 @@ class V3DfgCse final { case VDfgType::NeqCase: case VDfgType::NeqWild: case VDfgType::Not: + case VDfgType::OneHot: + case VDfgType::OneHot0: case VDfgType::Or: case VDfgType::Pow: case VDfgType::PowSS: @@ -232,6 +234,8 @@ class V3DfgCse final { case VDfgType::NeqCase: case VDfgType::NeqWild: case VDfgType::Not: + case VDfgType::OneHot: + case VDfgType::OneHot0: case VDfgType::Or: case VDfgType::Pow: case VDfgType::PowSS: diff --git a/src/V3DfgPeephole.cpp b/src/V3DfgPeephole.cpp index f8a9a2b46..7c0fc7e25 100644 --- a/src/V3DfgPeephole.cpp +++ b/src/V3DfgPeephole.cpp @@ -143,6 +143,8 @@ template <> void foldOp (V3Number& out, const V3Number& src) { ou template <> void foldOp (V3Number& out, const V3Number& src) { out.opLogNot(src); } template <> void foldOp (V3Number& out, const V3Number& src) { out.opNegate(src); } template <> void foldOp (V3Number& out, const V3Number& src) { out.opNot(src); } +template <> void foldOp (V3Number& out, const V3Number& src) { out.opOneHot(src); } +template <> void foldOp (V3Number& out, const V3Number& src) { out.opOneHot0(src); } template <> void foldOp (V3Number& out, const V3Number& src) { out.opRedAnd(src); } template <> void foldOp (V3Number& out, const V3Number& src) { out.opRedOr(src); } template <> void foldOp (V3Number& out, const V3Number& src) { out.opRedXor(src); } @@ -1325,6 +1327,14 @@ class V3DfgPeephole final : public DfgVisitor { } } + void visit(DfgOneHot* const vtxp) override { + if (foldUnary(vtxp)) return; + } + + void visit(DfgOneHot0* const vtxp) override { + if (foldUnary(vtxp)) return; + } + void visit(DfgRedOr* const vtxp) override { if (optimizeReduction(vtxp)) return; } diff --git a/test_regress/t/t_dfg_peephole.v b/test_regress/t/t_dfg_peephole.v index aa15acf60..9e5c64da4 100644 --- a/test_regress/t/t_dfg_peephole.v +++ b/test_regress/t/t_dfg_peephole.v @@ -62,6 +62,12 @@ module t ( //verilator lint_on WIDTH `signal(FOLD_UNARY_Extend, tmp_FOLD_UNARY_Extend); `signal(FOLD_UNARY_ExtendS, tmp_FOLD_UNARY_ExtendS); + `signal(FOLD_UNARY_OneHot, $onehot(const_a)); + `signal(FOLD_UNARY_OneHot0, $onehot0(const_a)); + `signal(FOLD_UNARY_OneHot_A, $onehot(const_a[0])); + `signal(FOLD_UNARY_OneHot_B, $onehot(~const_a[0])); + `signal(FOLD_UNARY_OneHot0_A, $onehot0(const_a[0])); + `signal(FOLD_UNARY_OneHot0_B, $onehot0(~const_a[0])); `signal(FOLD_BINARY_Add, const_a + const_b); `signal(FOLD_BINARY_And, const_a & const_b); From e35b2429ff9579756fa911d02198f9cd4ba41abb Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Fri, 5 Jun 2026 13:00:31 +0100 Subject: [PATCH 03/32] Optimize $countones constant in Dfg --- src/V3DfgPeephole.cpp | 5 +++++ test_regress/t/t_dfg_peephole.v | 1 + 2 files changed, 6 insertions(+) diff --git a/src/V3DfgPeephole.cpp b/src/V3DfgPeephole.cpp index 7c0fc7e25..8181dcfab 100644 --- a/src/V3DfgPeephole.cpp +++ b/src/V3DfgPeephole.cpp @@ -138,6 +138,7 @@ template <> const DfgDataType& resultDType (const DfgVertex* lhsp // Unary constant folding template void foldOp(V3Number& out, const V3Number& src); +template <> void foldOp (V3Number& out, const V3Number& src) { out.opCountOnes(src); } template <> void foldOp (V3Number& out, const V3Number& src) { out.opAssign(src); } template <> void foldOp (V3Number& out, const V3Number& src) { out.opExtendS(src, src.width()); } template <> void foldOp (V3Number& out, const V3Number& src) { out.opLogNot(src); } @@ -1195,6 +1196,10 @@ class V3DfgPeephole final : public DfgVisitor { // DfgVertexUnary //========================================================================= + void visit(DfgCountOnes* const vtxp) override { + if (foldUnary(vtxp)) return; + } + void visit(DfgExtend* const vtxp) override { if (foldUnary(vtxp)) return; diff --git a/test_regress/t/t_dfg_peephole.v b/test_regress/t/t_dfg_peephole.v index 9e5c64da4..3c9ce7ae8 100644 --- a/test_regress/t/t_dfg_peephole.v +++ b/test_regress/t/t_dfg_peephole.v @@ -62,6 +62,7 @@ module t ( //verilator lint_on WIDTH `signal(FOLD_UNARY_Extend, tmp_FOLD_UNARY_Extend); `signal(FOLD_UNARY_ExtendS, tmp_FOLD_UNARY_ExtendS); + `signal(FOLD_UNARY_CountOnes, $countones(const_a)); `signal(FOLD_UNARY_OneHot, $onehot(const_a)); `signal(FOLD_UNARY_OneHot0, $onehot0(const_a)); `signal(FOLD_UNARY_OneHot_A, $onehot(const_a[0])); From 629266a9886fe19917f45f054960b6a61243348a Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 6 Jun 2026 10:46:02 -0400 Subject: [PATCH 04/32] Fix whitespace --- test_regress/t/t_forceable_unpacked.v | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test_regress/t/t_forceable_unpacked.v b/test_regress/t/t_forceable_unpacked.v index 5016bdbef..5e0048563 100644 --- a/test_regress/t/t_forceable_unpacked.v +++ b/test_regress/t/t_forceable_unpacked.v @@ -4,13 +4,16 @@ // SPDX-FileCopyrightText: 2026 Nikolai Kumar // SPDX-License-Identifier: CC0-1.0 +// verilog_format: off `define checkh(g,e) do if ((g) !==(e)) begin $write("%%Error: %s:%0d: got=%x exp=%x\n", `__FILE__,`__LINE__, (g),(e)); $stop; end while(0) `ifdef CMT - `define FORCEABLE /*verilator forceable*/ + `define FORCEABLE /*verilator forceable*/ `else - `define FORCEABLE + `define FORCEABLE `endif +// verilog_format: on + module t (input wire clk, output reg [31:0] cyc); initial cyc = 0; always @(posedge clk) cyc <= cyc + 1; From 85d9c38ebf1465c90f6cb3bb9d0ce7a6a4c357ab Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 6 Jun 2026 10:46:18 -0400 Subject: [PATCH 05/32] Commentary: Changes update --- Changes | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Changes b/Changes index 4ce7cff30..e1e9d3d46 100644 --- a/Changes +++ b/Changes @@ -44,6 +44,7 @@ Verilator 5.049 devel * Support generic interface arrays (#7604). [Krzysztof Bieganski, Antmicro Ltd.] * Support FSM detection in primitive wrappers (#7607). [Yogish Sekhar] * Support busses with mix of pullup/pulldown (#7632). [Lucas Amaral] +* Support forceable on unpacked array variables (#7677) (#7678). [Nikolai Kumar] * Support pre/post increment/decrement inside && and || (#7683). [Nick Brereton] * Support streaming into 2-D unpacked arrays (#7686) (#7687). [Paul Swirhun] * Support if/if-else in properties (#7692). [Artur Bieniek, Antmicro Ltd.] @@ -68,6 +69,8 @@ Verilator 5.049 devel * Optimize formatting functions (#7701) (#7702) (#7703). [Geza Lore, Testorrent USA, Inc.] * Optimize DPI import argument passing (#7704). [Geza Lore, Testorrent USA, Inc.] * Optimize runtime assertOn() checks (#7707). [Geza Lore, Testorrent USA, Inc.] +* Optimize $countones and $onehot in DFG. [Geza Lore, Testorrent USA, Inc.] +* Optimize procedural loop unrolling. [Geza Lore, Testorrent USA, Inc.] * Fix TSP variable ordering for mtasks (#5342) (#7610). [Muzaffer Kal] * Fix inlining static initializer in V3Gate (#5381) (#7503). [Andrew Nolte] [Geza Lore, Testorrent USA, Inc.] * Fix segmentation fault when using --trace with --lib-create (#7299) (#7518). [anonkey] From 680ef8dda931d540446bb0b486afdeb85239325a Mon Sep 17 00:00:00 2001 From: em2machine <92717390+em2machine@users.noreply.github.com> Date: Sat, 6 Jun 2026 17:55:47 +0200 Subject: [PATCH 06/32] Fix for HIERPARAM - relax checking (#7570) (#7690) --- src/V3AstAttr.h | 18 ++++++ src/V3Param.cpp | 29 ++++++---- test_regress/t/t_interface_hierparam_bits.py | 18 ++++++ test_regress/t/t_interface_hierparam_bits.v | 61 ++++++++++++++++++++ 4 files changed, 115 insertions(+), 11 deletions(-) create mode 100755 test_regress/t/t_interface_hierparam_bits.py create mode 100644 test_regress/t/t_interface_hierparam_bits.v diff --git a/src/V3AstAttr.h b/src/V3AstAttr.h index e912bae20..c12627886 100644 --- a/src/V3AstAttr.h +++ b/src/V3AstAttr.h @@ -370,6 +370,24 @@ public: // clang-format on return names[m_e]; } + // True for attributes that read an operand's type rather than its value, such as $bits. + bool isTypeQuery() const { + switch (m_e) { + case DIM_BITS: + case DIM_BITS_OR_NUMBER: + case DIM_DIMENSIONS: + case DIM_HIGH: + case DIM_INCREMENT: + case DIM_LEFT: + case DIM_LOW: + case DIM_RIGHT: + case DIM_SIZE: + case DIM_UNPK_DIMENSIONS: + case TYPEID: + case TYPENAME: return true; + default: return false; + } + } VAttrType() : m_e{ILLEGAL} {} // cppcheck-suppress noExplicitConstructor diff --git a/src/V3Param.cpp b/src/V3Param.cpp index fe2273d98..063c55d3a 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -2722,10 +2722,14 @@ class ParamVisitor final : public VNVisitor { // LCOV_EXCL_STOP } - void checkParamNotHier(AstNode* valuep) { - if (!valuep) return; - valuep->foreachAndNext([&](const AstNodeExpr* exprp) { - if (const AstVarXRef* const refp = VN_CAST(exprp, VarXRef)) { + // Flag hierarchical refs in a parameter value. Single top-down pass. + void checkParamNotHierRecurse(AstNode* nodep, bool underTypeQuery = false) { + for (; nodep; nodep = nodep->nextp()) { + // Refs read only for their type ($bits etc.) are allowed + const AstAttrOf* const attrp = VN_CAST(nodep, AttrOf); + const bool childUnderQuery + = underTypeQuery || (attrp && attrp->attrType().isTypeQuery()); + if (const AstVarXRef* const refp = VN_CAST(nodep, VarXRef)) { // Allow hierarchical ref to interface params through interface/modport ports // or local interface instances bool isIfaceRef = false; @@ -2735,18 +2739,21 @@ class ParamVisitor final : public VNVisitor { = !refname.empty() && (m_ifacePortNames.count(refname) || m_ifaceInstCells.count(refname)); } - - if (!isIfaceRef) { + if (!isIfaceRef && !underTypeQuery) { refp->v3warn(HIERPARAM, "Parameter values cannot use hierarchical values" " (IEEE 1800-2023 6.20.2)"); } - } else if (const AstNodeFTaskRef* refp = VN_CAST(exprp, NodeFTaskRef)) { + } else if (const AstNodeFTaskRef* const refp = VN_CAST(nodep, NodeFTaskRef)) { if (refp->dotted() != "") { refp->v3error("Parameter values cannot call hierarchical functions" " (IEEE 1800-2023 6.20.2)"); } } - }); + checkParamNotHierRecurse(nodep->op1p(), childUnderQuery); + checkParamNotHierRecurse(nodep->op2p(), childUnderQuery); + checkParamNotHierRecurse(nodep->op3p(), childUnderQuery); + checkParamNotHierRecurse(nodep->op4p(), childUnderQuery); + } } // Deparameterize and constify nested interface cells within ifaceModp. @@ -2888,7 +2895,7 @@ class ParamVisitor final : public VNVisitor { iterateChildren(nodep); } void visit(AstCell* nodep) override { - checkParamNotHier(nodep->paramsp()); + checkParamNotHierRecurse(nodep->paramsp()); if (VN_IS(nodep->modp(), Iface)) m_ifaceInstCells.emplace(nodep->name(), nodep); visitCellOrClassRef(nodep, VN_IS(nodep->modp(), Iface)); } @@ -2896,7 +2903,7 @@ class ParamVisitor final : public VNVisitor { if (nodep->ifacep()) visitCellOrClassRef(nodep, true); } void visit(AstClassRefDType* nodep) override { - checkParamNotHier(nodep->paramsp()); + checkParamNotHierRecurse(nodep->paramsp()); visitCellOrClassRef(nodep, false); } void visit(AstClassOrPackageRef* nodep) override { @@ -2913,7 +2920,7 @@ class ParamVisitor final : public VNVisitor { if (nodep->isIfaceRef()) { m_ifacePortNames.insert(nodep->name()); } iterateChildren(nodep); if (nodep->isParam()) { - checkParamNotHier(nodep->valuep()); + checkParamNotHierRecurse(nodep->valuep()); if (!nodep->valuep() && !VN_IS(m_modp, Class)) { nodep->v3error("Parameter without default value is never given value" << " (IEEE 1800-2023 6.20.1): " << nodep->prettyNameQ()); diff --git a/test_regress/t/t_interface_hierparam_bits.py b/test_regress/t/t_interface_hierparam_bits.py new file mode 100755 index 000000000..46d1fe4c0 --- /dev/null +++ b/test_regress/t/t_interface_hierparam_bits.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(verilator_flags2=['--binary']) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_interface_hierparam_bits.v b/test_regress/t/t_interface_hierparam_bits.v new file mode 100644 index 000000000..47e58e753 --- /dev/null +++ b/test_regress/t/t_interface_hierparam_bits.v @@ -0,0 +1,61 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// $bits() of interface member signals used as a child module parameter value. +// $bits reads the type, not the hierarchical value, so it is legal in a +// parameter (IEEE 1800-2023 6.20.2 forbids hierarchical values only). +// +// 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='h%x exp='h%x\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0) +// verilog_format: on + +interface axi_if #( + parameter int ID_W = 8, + parameter int ADDR_W = 32 +); + logic [ID_W-1:0] AWID; + logic [ADDR_W-1:0] AWADDR; + logic [7:0] AWLEN; +endinterface + +module chkmod #( + parameter int PAYLOAD_WIDTH = 1, + parameter int EXPECT = 1 +); + initial `checkh(PAYLOAD_WIDTH, EXPECT); +endmodule + +module bridge #( + parameter int EXPECT = 1 +) ( + axi_if axi +); + // $bits of a concat of interface members + chkmod #( + .PAYLOAD_WIDTH($bits({axi.AWID, axi.AWADDR, axi.AWLEN})), + .EXPECT(EXPECT) + ) u_concat (); + // $bits of a single interface member + chkmod #( + .PAYLOAD_WIDTH($bits(axi.AWADDR)), + .EXPECT(EXPECT - 12 - 8) + ) u_single (); +endmodule + +module t; + axi_if #(.ID_W(12), .ADDR_W(64)) if0 (); // 12 + 64 + 8 = 84 + axi_if #(.ID_W(12), .ADDR_W(16)) if1 (); // 12 + 16 + 8 = 36 + + bridge #(.EXPECT(84)) dut0 (.axi(if0)); + bridge #(.EXPECT(36)) dut1 (.axi(if1)); + + initial begin + #1; + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule From 067cd6c9c6b6061c280f128effd7940e78cbec6a Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sat, 6 Jun 2026 10:19:21 +0100 Subject: [PATCH 07/32] CI: Show summary tables in 'pr: rtlmeter' results --- .github/workflows/coverage.yml | 2 +- .github/workflows/pages.yml | 6 +- .github/workflows/rtlmeter-pr-results.yml | 48 ------- .github/workflows/rtlmeter.yml | 111 ++++---------- Makefile.in | 1 + ci/ci-pages-notify.bash | 8 +- ci/ci-pages.bash | 168 ++++++++++++++++++++-- ci/ci-rtlmeter-pr-report.bash | 135 +++++++++++++++++ ci/ci-rtlmeter-pr-report.py | 103 +++++++++++++ test_regress/t/t_dist_whitespace.py | 2 +- 10 files changed, 428 insertions(+), 156 deletions(-) delete mode 100644 .github/workflows/rtlmeter-pr-results.yml create mode 100755 ci/ci-rtlmeter-pr-report.bash create mode 100644 ci/ci-rtlmeter-pr-report.py diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 1724ebf18..439143c85 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -182,7 +182,7 @@ jobs: uses: actions/upload-artifact@v7 with: path: repo/notification - name: coverage-pr-notification + name: pr-notification # Create GitHub issue for failed scheduled jobs # This should always be the last job (we want an issue if anything breaks) diff --git a/.github/workflows/pages.yml b/.github/workflows/pages.yml index 8b53d95b6..658134075 100644 --- a/.github/workflows/pages.yml +++ b/.github/workflows/pages.yml @@ -10,7 +10,7 @@ on: paths: ["ci/**", ".github/workflows"] workflow_dispatch: workflow_run: - workflows: ["Code coverage"] + workflows: ["Code coverage", "RTLMeter"] types: [completed] # Sets permissions of the GITHUB_TOKEN to allow deployment to GitHub Pages @@ -35,7 +35,7 @@ jobs: name: Build content runs-on: ubuntu-24.04 outputs: - coverage-pr-run-ids: ${{ steps.build.outputs.coverage-pr-run-ids }} + pr-run-ids: ${{ steps.build.outputs.pr-run-ids }} steps: - name: Checkout uses: actions/checkout@v6 @@ -83,5 +83,5 @@ jobs: - name: Comment on PR env: GH_TOKEN: ${{ steps.generate-token.outputs.token }} - COVERAGE_PR_RUN_IDS: ${{ needs.build.outputs.coverage-pr-run-ids }} + PR_RUN_IDS: ${{ needs.build.outputs.pr-run-ids }} run: bash -x ./ci/ci-pages-notify.bash diff --git a/.github/workflows/rtlmeter-pr-results.yml b/.github/workflows/rtlmeter-pr-results.yml deleted file mode 100644 index 2a8bbb2a4..000000000 --- a/.github/workflows/rtlmeter-pr-results.yml +++ /dev/null @@ -1,48 +0,0 @@ ---- -# DESCRIPTION: Github actions config -# This name is key to badges in README.rst, so we use the name build -# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 - -name: RTLMeter PR results - -on: - workflow_run: - workflows: [RTLMeter] - types: [completed] - -jobs: - publish: - name: Publish - runs-on: ubuntu-latest - if: ${{ github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion == 'success' }} - permissions: - actions: read - pull-requests: write - steps: - - name: Download report - uses: actions/download-artifact@v8 - with: - name: rtlmeter-pr-results - run-id: ${{ github.event.workflow_run.id }} - github-token: ${{ secrets.GITHUB_TOKEN }} - - name: Download PR number - uses: actions/download-artifact@v8 - with: - name: pr-number - run-id: ${{ github.event.workflow_run.id }} - github-token: ${{ secrets.GITHUB_TOKEN }} - # Use the Verilator CI app to post the comment - - name: Generate access token - id: generate-token - uses: actions/create-github-app-token@v3.2.0 - with: - app-id: ${{ vars.VERILATOR_CI_ID }} - private-key: ${{ secrets.VERILATOR_CI_KEY }} - permission-pull-requests: write - - name: Comment on PR - env: - GH_TOKEN: ${{ steps.generate-token.outputs.token }} - run: |- - ls -la - cat report.txt - gh pr --repo ${{ github.repository }} comment $(cat pr-number.txt) --body-file report.txt diff --git a/.github/workflows/rtlmeter.yml b/.github/workflows/rtlmeter.yml index 372df70ae..626bb994c 100644 --- a/.github/workflows/rtlmeter.yml +++ b/.github/workflows/rtlmeter.yml @@ -286,106 +286,45 @@ jobs: with: repository: "verilator/rtlmeter" path: rtlmeter + - name: Setup RTLMeter venv working-directory: rtlmeter run: make venv - - name: Download combined results - uses: actions/download-artifact@v8 + + - name: Checkout Verilator + uses: actions/checkout@v6 with: - pattern: all-results-* - path: all-results - merge-multiple: true - - name: Get scheduled run info - id: scheduled-info - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - ID=$(gh run --repo ${{ github.repository }} list --workflow RTLMeter --event schedule --status success --limit 1 --json databaseId --jq ".[0].databaseId") - echo "id=$ID" >> $GITHUB_OUTPUT - URL=$(gh run --repo ${{ github.repository }} view $ID --json url --jq ".url") - echo "url=$URL" >> $GITHUB_OUTPUT - NUM=$(gh run --repo ${{ github.repository }} view $ID --json number --jq ".number") - echo "num=$NUM" >> $GITHUB_OUTPUT - DATE=$(gh run --repo ${{ github.repository }} view $ID --json createdAt --jq ".createdAt") - echo "date=$DATE" >> $GITHUB_OUTPUT - - name: Download scheduled run results - uses: actions/download-artifact@v8 - with: - name: published-results - path: nightly-results - run-id: ${{ steps.scheduled-info.outputs.id }} - github-token: ${{ secrets.GITHUB_TOKEN }} - - name: Compare results - working-directory: rtlmeter - run: | - for tag in gcc clang gcc-hier; do - ADATA=../nightly-results/all-results-${tag}.json - BDATA=../all-results/all-results-${tag}.json - touch ../verilate-${tag}.txt - touch ../execute-${tag}.txt - touch ../cppbuild-${tag}.txt - if [[ ! -e $ADATA ]]; then - continue - fi - ./rtlmeter compare --cases '* !Example:* !*:hello' --steps "verilate" --metrics "elapsed memory" $ADATA $BDATA > ../verilate-${tag}.txt - cat ../verilate-${tag}.txt - ./rtlmeter compare --cases '* !Example:* !*:hello' --steps "execute" --metrics "speed memory elapsed" $ADATA $BDATA > ../execute-${tag}.txt - cat ../execute-${tag}.txt - ./rtlmeter compare --cases '* !Example:* !*:hello' --steps "cppbuild" --metrics "elapsed memory cpu codeSize" $ADATA $BDATA > ../cppbuild-${tag}.txt - cat ../cppbuild-${tag}.txt - done + path: verilator + - name: Create report + working-directory: verilator env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - set -x - NUM=$(gh run --repo ${{ github.repository }} view ${{ github.run_id }} --json number --jq ".number") - URL=$(gh run --repo ${{ github.repository }} view ${{ github.run_id }} --json url --jq ".url") - echo -n "Performance metrics for PR workflow [#$NUM]($URL) (B) compared to scheduled run" > report.txt - echo -n " [#${{ steps.scheduled-info.outputs.num }}](${{ steps.scheduled-info.outputs.url }}) (A)" >> report.txt - echo " from ${{ steps.scheduled-info.outputs.date }}" >> report.txt - for tag in gcc clang gcc-hier; do - echo "" >> report.txt - if [[ $tag == "gcc" ]]; then - echo "
" >> report.txt - else - echo "
" >> report.txt - fi - echo -n "" >> report.txt - jq -rj ".[0].runName" all-results/all-results-${tag}.json >> report.txt - echo "" >> report.txt - awk -v RS= -v tag=${tag} '{print > sprintf("frag-%02d-verilate-%s.txt",NR,tag)}' verilate-${tag}.txt - awk -v RS= -v tag=${tag} '{print > sprintf("frag-%02d-execute-%s.txt" ,NR,tag)}' execute-${tag}.txt - awk -v RS= -v tag=${tag} '{print > sprintf("frag-$02d-cppbuild-%s.txt",NR,tag)}' cppbuild-${tag}.txt - for f in $(ls -1 frag-*-verilate-${tag}.txt | sort) $(ls -1 frag-*-execute-${tag}.txt | sort) $(ls -1 frag-*-cppbuild-${tag}.txt | sort); do - if [[ $f == frag-01-verilate-${tag}.txt || $f == frag-01-execute-${tag}.txt ]]; then - echo "
" >> report.txt - else - echo "
" >> report.txt - fi - echo -n "" >> report.txt - head -n 1 $f | tr -d '\n' >> report.txt - echo "" >> report.txt - echo '
' >> report.txt
-              tail -n +2 $f >> report.txt
-              echo '
' >> report.txt - echo "
" >> report.txt - done - echo "
" >> report.txt - done - cat report.txt + ln -s ../rtlmeter rtlmeter + gh repo set-default ${{ github.repository }} + # Compare to last successful scheduled run + REF_ID=$(gh run list --workflow RTLMeter --event schedule --status success --limit 1 --json databaseId --jq ".[0].databaseId") + NEW_ID=${{ github.run_id }} + ci/ci-rtlmeter-pr-report.bash $REF_ID $NEW_ID gcc clang gcc-hier + mv rtlmeter-pr-report/report.txt ../ + cat ../report.txt + # Generate notification comment content + mkdir -p ../notification + mv rtlmeter-pr-report/notification.txt ../notification/body.txt + echo ${{ github.event.number }} > ../notification/pr-number.txt + - name: Upload report uses: actions/upload-artifact@v7 with: path: report.txt - name: rtlmeter-pr-results - - name: Save PR number - run: echo ${{ github.event.number }} > pr-number.txt - - name: Upload PR number + name: rtlmeter-pr-report + + - name: Upload notification uses: actions/upload-artifact@v7 with: - path: pr-number.txt - name: pr-number + path: notification + name: pr-notification # Create GitHub issue for failed scheduled jobs # This should always be the last job (we want an issue if anything breaks) diff --git a/Makefile.in b/Makefile.in index 9d92056ae..c2b60d004 100644 --- a/Makefile.in +++ b/Makefile.in @@ -535,6 +535,7 @@ PY_PROGRAMS = \ # Python files, subject to format but not lint PY_FILES = \ $(PY_PROGRAMS) \ + ci/*.py \ test_regress/t/*.py \ # Python files, test_regress tests diff --git a/ci/ci-pages-notify.bash b/ci/ci-pages-notify.bash index 05bb92fe6..e5dd599b6 100755 --- a/ci/ci-pages-notify.bash +++ b/ci/ci-pages-notify.bash @@ -4,7 +4,7 @@ # SPDX-FileCopyrightText: 2025 Geza Lore # SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 -# Notify PRs via comment that their coverage reports are available +# Notify PRs via comment that their workflow reports are available # Get the current repo URL - might differ on a fork readonly REPO_URL=$(gh repo view --json url --jq .url) @@ -13,7 +13,7 @@ readonly REPO_URL=$(gh repo view --json url --jq .url) ARTIFACTS_ROOT=artifacts mkdir -p ${ARTIFACTS_ROOT} -for RUN_ID in ${COVERAGE_PR_RUN_IDS//,/ }; do +for RUN_ID in ${PR_RUN_IDS//,/ }; do echo "@@@ Processing run ${RUN_ID}" # Create workflow artifacts directory @@ -21,7 +21,7 @@ for RUN_ID in ${COVERAGE_PR_RUN_IDS//,/ }; do mkdir -p ${ARTIFACTS_DIR} # Download artifact of this run, if exists - gh run download ${RUN_ID} --name coverage-pr-notification --dir ${ARTIFACTS_DIR} || true + gh run download ${RUN_ID} --name pr-notification --dir ${ARTIFACTS_DIR} || true ls -lsha ${ARTIFACTS_DIR} # Move on if no notification is required @@ -35,7 +35,7 @@ for RUN_ID in ${COVERAGE_PR_RUN_IDS//,/ }; do gh pr comment $(cat ${ARTIFACTS_DIR}/pr-number.txt) --body-file ${ARTIFACTS_DIR}/body.txt # Get the artifact ID - ARTIFACT_ID=$(gh api "repos/{owner}/{repo}/actions/runs/${RUN_ID}/artifacts" --jq '.artifacts[] | select(.name == "coverage-pr-notification") | .id') + ARTIFACT_ID=$(gh api "repos/{owner}/{repo}/actions/runs/${RUN_ID}/artifacts" --jq '.artifacts[] | select(.name == "pr-notification") | .id') # Delete it, so we only notify once gh api --method DELETE "repos/{owner}/{repo}/actions/artifacts/${ARTIFACT_ID}" diff --git a/ci/ci-pages.bash b/ci/ci-pages.bash index ba9013621..44dc3f6d1 100755 --- a/ci/ci-pages.bash +++ b/ci/ci-pages.bash @@ -5,8 +5,7 @@ # SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 # This scipt build the content of the GitHub Pages for the repository. -# Currently this only hosts code coverage reports, but it would be possible to -# add any other contents to the page in parallel here. +# Currently this hosts code coverage reports, and PR RTLMeter detailed reports. # Developer note: You should be able to run this script in your local checkout # if you have GitHub CLI (command 'gh') setup, authenticated ('gh auth login'), @@ -25,6 +24,9 @@ if [[ -z "$GITHUB_OUTPUT" ]]; then GITHUB_OUTPUT=github-output.txt fi +# Run IDs of PR jobs processed +PR_RUN_IDS="" + # Populates ${PAGES_ROOT}/coverage-reports compile_coverage_reports() { # We will process all runs up to and including this date. This is chosen to be @@ -53,9 +55,6 @@ compile_coverage_reports() { local CONTENTS=contents.tmp echo > ${CONTENTS} - # Run IDs of PR jobs processed - local PR_RUN_IDS="" - # Iterate over all unique event types that triggered the workflows for EVENT in $(jq -r 'map(.event) | sort | unique | .[]' completedRuns.json); do echo "@@@ Processing '${EVENT}' runs" @@ -73,11 +72,7 @@ compile_coverage_reports() { # Record run ID of PR job if [[ $EVENT == "pull_request" ]]; then - if [[ -z "$PR_RUN_IDS" ]]; then - PR_RUN_IDS="$RUN_ID" - else - PR_RUN_IDS="$PR_RUN_IDS,$RUN_ID" - fi + PR_RUN_IDS="$PR_RUN_IDS $RUN_ID" fi # Create workflow artifacts directory @@ -148,7 +143,7 @@ CONTENTS_TEMPLATE $(cat ${CONTENTS}) -

Assembled $(date --iso-8601=minutes --utc)

+

Assembled $(date --iso-8601=minutes --utc)

@@ -156,12 +151,159 @@ INDEX_TEMPLATE # Report size du -shc ${COVERAGE_ROOT}/* +} - # Set output - echo "coverage-pr-run-ids=${PR_RUN_IDS}" >> $GITHUB_OUTPUT +# Populates ${PAGES_ROOT}/rtlmeter-reports +compile_rtlmeter_reports() { + # We will process all runs up to and including this date. This is chosen to be + # slightly less than the artifact retention period for simplicity. + local OLDEST=$(date --date="28 days ago" --iso-8601=date) + + # Gather all RTLMeter workflow runs within the time window + gh run list -w rtlmeter.yml --limit 1000 --created ">=${OLDEST}" --json "databaseId,event,status,conclusion,createdAt,number" > recentRuns.json + echo @@@ Recent runs: + jq "." recentRuns.json + + # Select completd runs that were not cancelled or skipped, sort by descending run number + jq 'sort_by(-.number) | map(select(.status == "completed" and (.conclusion == "success" or .conclusion == "failure")))' recentRuns.json > completedRuns.json + echo @@@ Completed with success or failure: + jq "." completedRuns.json + + # Create artifacts root directory + local ARTIFACTS_ROOT=artifacts + mkdir -p ${ARTIFACTS_ROOT} + + # Create rtlmeter reports root directory + local RTLMETER_ROOT=${PAGES_ROOT}/rtlmeter-reports + mkdir -p ${RTLMETER_ROOT} + + # Create index page contents fragment + local CONTENTS=contents.tmp + echo > ${CONTENTS} + + # Iterate over all unique event types that triggered the workflows + for EVENT in $(jq -r 'map(.event) | sort | unique | .[]' completedRuns.json); do + echo "@@@ Processing '${EVENT}' runs" + + # Emit section header if a report exists with this event type + EMIT_SECTION_HEADER=1 + + # For each worfklow run that was triggered by this event type + for RUN_ID in $(jq ".[] | select(.event == \"${EVENT}\") |.databaseId" completedRuns.json); do + echo "@@@ Processing run ${RUN_ID}" + + # Extract the info of this run + jq ".[] | select(.databaseId == $RUN_ID)" completedRuns.json > workflow.json + jq "." workflow.json + + # Record run ID of PR job + if [[ $EVENT == "pull_request" ]]; then + PR_RUN_IDS="$PR_RUN_IDS $RUN_ID" + fi + + # Create workflow artifacts directory + local ARTIFACTS_DIR=${ARTIFACTS_ROOT}/${RUN_ID} + mkdir -p ${ARTIFACTS_DIR} + + # Download artifacts of this run, if exists + gh run download ${RUN_ID} --name rtlmeter-pr-report --dir ${ARTIFACTS_DIR} || true + ls -lsha ${ARTIFACTS_DIR} + + # Move on if no RTLMeter report is available + if [ ! -f ${ARTIFACTS_DIR}/report.txt ]; then + echo "No RTLMeter report found" + continue + fi + echo "RTLMeter report found" + + # Emit section header + if [[ -n $EMIT_SECTION_HEADER ]]; then + unset EMIT_SECTION_HEADER + echo "

RTLMeter reports for '${EVENT}' runs:

" >> ${CONTENTS} + fi + + # Extract run metadata + local WORKFLOW_CREATED=$(jq -r '.createdAt' workflow.json) + local WOFKRLOW_NUMBER=$(jq -r '.number' workflow.json) + + # Wrap the report into an HTML page. The report content is already HTML + # (produced by ci-rtlmeter-pr-report.bash), so we just embed it in the + # page body. + cat > ${RTLMETER_ROOT}/${RUN_ID}.html < + + + Verilator RTLMeter report #${WOFKRLOW_NUMBER} + + + + +$(cat ${ARTIFACTS_DIR}/report.txt) + + + +REPORT_TEMPLATE + + # Add index page content + cat >> ${CONTENTS} <#${WOFKRLOW_NUMBER} + | GitHub: ${RUN_ID} + | started at: ${WORKFLOW_CREATED} +CONTENTS_TEMPLATE + echo "
" >> ${CONTENTS} + done + + # Section break + if [[ -z "$EMIT_SECTION_HEADER" ]]; then + echo "
" >> ${CONTENTS} + fi + done + + # Write rtlmeter report index.html + cat > ${RTLMETER_ROOT}/index.html < + + + Verilator CI RTLMeter reports + + + + + $(cat ${CONTENTS}) +

Assembled $(date --iso-8601=minutes --utc)

+ + + +INDEX_TEMPLATE + + # Report size + du -shc ${RTLMETER_ROOT}/* } # Compilie coverage reports compile_coverage_reports; +# Compile RTLMeter reports +compile_rtlmeter_reports; + # You can build any other content here to be put under ${PAGES_ROOT} + +# Set output to all affected unique PR run IDs (comma separated, no trailing comma) +IDS=$(echo ${PR_RUN_IDS} | xargs -n1 | sort -u | paste -sd,) +echo "pr-run-ids=${IDS}" >> $GITHUB_OUTPUT diff --git a/ci/ci-rtlmeter-pr-report.bash b/ci/ci-rtlmeter-pr-report.bash new file mode 100755 index 000000000..f44f074e2 --- /dev/null +++ b/ci/ci-rtlmeter-pr-report.bash @@ -0,0 +1,135 @@ +#!/usr/bin/env bash +# DESCRIPTION: Verilator: CI script for 'rtlmeter.yml' PR results +# +# SPDX-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +# This scipt builds the content of the response comment posten on PRs +# at the end of RTLMeter runs. + +# Developer note: You should be able to run this script in your local checkout +# if you have GitHub CLI (command 'gh') setup, authenticated ('gh auth login'), +# and have set a default repository ('gh repo set-default'). + +# Trace when running in the CI +[ "$GITHUB_ACTIONS" != "true" ] || set -x + +# Arguments: +# 1. reference run ID +# 2. new run ID +# rest: run tags +REF_ID=$1; shift +NEW_ID=$1; shift +RUNS="$@" + +# $VERILATOR_CHECKOUT/ci directory +SCRIPT_DIR=$(readlink -f $(dirname ${BASH_SOURCE[0]})) + +# Move into a temporary directory +rm -rf rtlmeter-pr-report +mkdir rtlmeter-pr-report +pushd rtlmeter-pr-report &> /dev/null +TMP_DIR=$(readlink -f .) + +# Artifacts to download +DOWNLOAD_ARTIFACTS="" +for r in $RUNS; do + DOWNLOAD_ARTIFACTS="$DOWNLOAD_ARTIFACTS --name all-results-$r" +done + +# Download reference run results +mkdir ref +REF_DIR=$(readlink -f ref) +gh run download ${REF_ID} $DOWNLOAD_ARTIFACTS --dir $REF_DIR +mv $REF_DIR/*/*.json $REF_DIR/ +find $REF_DIR -mindepth 1 -type d -delete + +# Download current run results +mkdir new +NEW_DIR=$(readlink -f new) +gh run download ${NEW_ID} $DOWNLOAD_ARTIFACTS --dir $NEW_DIR +mv $NEW_DIR/*/*.json $NEW_DIR/ +find $NEW_DIR -mindepth 1 -type d -delete + +# Get Some metadata about the runs +REF_URL=$(gh run view $REF_ID --json url --jq ".url") +REF_NUM=$(gh run view $REF_ID --json number --jq ".number") +REF_DATE=$(gh run view $REF_ID --json createdAt --jq ".createdAt") +NEW_URL=$(gh run view $NEW_ID --json url --jq ".url") +NEW_NUM=$(gh run view $NEW_ID --json number --jq ".number") + +# Go back to root directory +popd &> /dev/null +# Go to RTLMeter directory +cd rtlmeter + +# Compare results +SUMMARY_ARGS=() +for r in $RUNS; do + CMP_JSON=$TMP_DIR/cmp-$r.json + # Gather args for summary script + SUMMARY_ARGS+=($REF_DIR/all-results-$r.json $CMP_JSON) + # Create JSON + ./rtlmeter compare --format json --steps "*" --metrics "*" \ + $REF_DIR/all-results-$r.json $NEW_DIR/all-results-$r.json > $CMP_JSON + # Also create detailed tables + ./rtlmeter compare --format ascii --steps 'verilate' --metrics '* !system !user' $REF_DIR/all-results-$r.json $NEW_DIR/all-results-$r.json > $TMP_DIR/verilate-$r.txt + ./rtlmeter compare --format ascii --steps 'cppbuild' --metrics '* !system !user' $REF_DIR/all-results-$r.json $NEW_DIR/all-results-$r.json > $TMP_DIR/cppbuild-$r.txt + ./rtlmeter compare --format ascii --steps 'execute' --metrics '* !system !user' $REF_DIR/all-results-$r.json $NEW_DIR/all-results-$r.json > $TMP_DIR/execute-$r.txt + # Chop them at new lines, into one table per file + awk -v RS= -v prefix=$TMP_DIR/$r-frag '{print > sprintf("%s-verilate-%02d.txt",prefix,NR)}' $TMP_DIR/verilate-$r.txt + awk -v RS= -v prefix=$TMP_DIR/$r-frag '{print > sprintf("%s-cppbuild-%02d.txt",prefix,NR)}' $TMP_DIR/cppbuild-$r.txt + awk -v RS= -v prefix=$TMP_DIR/$r-frag '{print > sprintf("%s-execute-%02d.txt" ,prefix,NR)}' $TMP_DIR/execute-$r.txt +done + +# Create summary +venv/bin/python3 $SCRIPT_DIR/ci-rtlmeter-pr-report.py ${SUMMARY_ARGS[@]} > $TMP_DIR/summary.txt +# Print it +cat $TMP_DIR/summary.txt + +# Create notification comment content +NOTIFICATION=$TMP_DIR/notification.txt +cat > $NOTIFICATION < + Summary of all runs + 
+$(cat $TMP_DIR/summary.txt)
+
+
+Blah Blah Blah +NOTIFICATION_TEMPLATE + +# Create detailed report +REPORT=$TMP_DIR/report.txt +cat > $REPORT < + Summary of all runs + 
+$(cat $TMP_DIR/summary.txt)
+
+
+SUMMARY_TEMPLATE +echo "
" >> $REPORT +echo " Detailed results" >> $REPORT +for r in $RUNS; do + RUN_NAME=$(jq -rj ".[0].runName" $REF_DIR/all-results-$r.json) + echo "
" >> $REPORT + echo " $RUN_NAME" >> $REPORT + for f in $(ls -1 $TMP_DIR/$r-frag-verilate-*.txt | sort) \ + $(ls -1 $TMP_DIR/$r-frag-cppbuild-*.txt | sort) \ + $(ls -1 $TMP_DIR/$r-frag-execute-*.txt | sort); do + cat >> $REPORT < + $(head -n 1 $f | tr -d '\n') + 
+$(tail -n +2 $f)
+
+
+DETAIL_TAMPLATE + done + echo "
" >> $REPORT +done +echo "" >> $REPORT diff --git a/ci/ci-rtlmeter-pr-report.py b/ci/ci-rtlmeter-pr-report.py new file mode 100644 index 000000000..e743f67da --- /dev/null +++ b/ci/ci-rtlmeter-pr-report.py @@ -0,0 +1,103 @@ +# DESCRIPTION: Verilator: CI script for 'rtlmeter.yml' PR results +# +# SPDX-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +############################################################################### +# This script runs with the venv of **RTLMeter** +############################################################################### + +import sys +import json +import tabulate +from typing import Final, List + +tabulate.PRESERVE_WHITESPACE = True +tabulate.MIN_PADDING = 0 + +_ASCII_TABLE_FORMAT: Final = tabulate.TableFormat( + lineabove=tabulate.Line("╒═", "═", "═╤═", "═╕"), + linebelowheader=tabulate.Line("╞═", "═", "═╪═", "═╡"), + linebelow=tabulate.Line("╘═", "═", "═╧═", "═╛"), + headerrow=tabulate.DataRow("│ ", " │ ", " │"), + datarow=tabulate.DataRow("│ ", " │ ", " │"), + linebetweenrows=None, + padding=0, + with_header_hide=None, +) + + +def printTable(table: List[List[str]], **kwargs) -> None: + print(tabulate.tabulate(table, tablefmt=_ASCII_TABLE_FORMAT, **kwargs)) + + +# fmt: off +stepMetric = ( + ("verilate", "elapsed"), + ("verilate", "memory"), + ("verilate", "cpu"), + ("cppbuild", "elapsed"), + ("cppbuild", "memory"), + ("cppbuild", "cpu"), + ("cppbuild", "codeSize"), + ("execute", "speed"), + ("execute", "clocks"), + ("execute", "memory"), + ("execute", "cpu"), +) +# fmt: on + +table = [] + +for ref, cmp in zip(sys.argv[1::2], sys.argv[2::2]): + with open(ref, "r", encoding="utf-8") as f: + ref_json = json.load(f)[0] + with open(cmp, "r", encoding="utf-8") as f: + cmp_json = json.load(f) + if table: + table.append(tabulate.SEPARATING_LINE) + runName = ref_json["runName"] + for step, metric in stepMetric: + if step not in cmp_json: + continue + data = cmp_json[step] + if metric not in data: + continue + data = data[metric] + minGain = float("inf") + maxGain = float("-inf") + meanGain = 1 + count = 0 + for _, _, _, _, _, g, _ in data["table"]: + minGain = min(minGain, g) + maxGain = max(maxGain, g) + meanGain *= g + count += 1 + meanGain = meanGain**(1 / count) + + if metric == "clocks": + # Clock cycles should match exactly + status = "❌" if minGain != 1 or maxGain != 1 else "✅" + else: + # Otherwise use some arbitrary brackets + status = "❌" + if (meanGain > 0.95): + status = "⚠️" + if (meanGain > 0.98): + status = "✅" + if (meanGain > 1.02): + status = "💡" + if (meanGain > 1.05): + status = "⭐" + + table.append([ + runName, step, ref_json["metrics"][metric]["header"], f"{meanGain:.2f}x {status} ", + f"{minGain:.2f}x", f"{maxGain:.2f}x" + ]) + +printTable( + table, + headers=("Run", "Step", "Metric", "Improvement", "Min", "Max"), + colalign=("left", "left", "left", "right", "right", "right"), + disable_numparse=True, +) diff --git a/test_regress/t/t_dist_whitespace.py b/test_regress/t/t_dist_whitespace.py index c6678f5b6..87f1fe8ef 100755 --- a/test_regress/t/t_dist_whitespace.py +++ b/test_regress/t/t_dist_whitespace.py @@ -13,7 +13,7 @@ test.scenarios('dist') Tabs_Exempt_Re = r'(\.out$)|(/fstcpp)|(Makefile)|(\.mk$)|(\.mk\.in$)|test_regress/t/t_preproc\.v|install-sh' -Unicode_Exempt_Re = r'(Changes$|CONTRIBUTORS$|LICENSES?|contributors.rst$|spelling.txt$)' +Unicode_Exempt_Re = r'(Changes$|CONTRIBUTORS$|LICENSES?|contributors.rst$|spelling.txt$|ci-rtlmeter-pr-report.py)' def get_source_files(): From 4ee19c4065db211277c3c8aa09d9d404a656888e Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sun, 7 Jun 2026 09:40:41 +0100 Subject: [PATCH 08/32] CI: Generate whole RTLMeter report in ci-rtlmeter-report.bash --- .github/workflows/rtlmeter.yml | 22 ++++---- ci/ci-pages.bash | 55 +++++-------------- ...pr-report.bash => ci-rtlmeter-report.bash} | 52 ++++++++++++------ ...ter-pr-report.py => ci-rtlmeter-report.py} | 0 test_regress/t/t_dist_whitespace.py | 2 +- 5 files changed, 63 insertions(+), 68 deletions(-) rename ci/{ci-rtlmeter-pr-report.bash => ci-rtlmeter-report.bash} (84%) rename ci/{ci-rtlmeter-pr-report.py => ci-rtlmeter-report.py} (100%) diff --git a/.github/workflows/rtlmeter.yml b/.github/workflows/rtlmeter.yml index 626bb994c..3c633d9f0 100644 --- a/.github/workflows/rtlmeter.yml +++ b/.github/workflows/rtlmeter.yml @@ -306,24 +306,26 @@ jobs: # Compare to last successful scheduled run REF_ID=$(gh run list --workflow RTLMeter --event schedule --status success --limit 1 --json databaseId --jq ".[0].databaseId") NEW_ID=${{ github.run_id }} - ci/ci-rtlmeter-pr-report.bash $REF_ID $NEW_ID gcc clang gcc-hier - mv rtlmeter-pr-report/report.txt ../ - cat ../report.txt - # Generate notification comment content - mkdir -p ../notification - mv rtlmeter-pr-report/notification.txt ../notification/body.txt - echo ${{ github.event.number }} > ../notification/pr-number.txt + ci/ci-rtlmeter-report.bash $REF_ID $NEW_ID gcc clang gcc-hier + # Create the report artifact + mkdir ../report-artifact + mv rtlmeter-report/report ../report-artifact/ + echo ${{ github.event.number }} > ../report-artifact/pr-number.txt + # Create the notification artifact + mkdir ../notification-artifact + mv rtlmeter-report/notification.txt ../notification-artifact/body.txt + echo ${{ github.event.number }} > ../notification-artifact/pr-number.txt - name: Upload report uses: actions/upload-artifact@v7 with: - path: report.txt - name: rtlmeter-pr-report + path: report-artifact + name: rtlmeter-report - name: Upload notification uses: actions/upload-artifact@v7 with: - path: notification + path: notification-artifact name: pr-notification # Create GitHub issue for failed scheduled jobs diff --git a/ci/ci-pages.bash b/ci/ci-pages.bash index 44dc3f6d1..bd9fcfaa2 100755 --- a/ci/ci-pages.bash +++ b/ci/ci-pages.bash @@ -44,7 +44,7 @@ compile_coverage_reports() { jq "." completedRuns.json # Create artifacts root directory - local ARTIFACTS_ROOT=artifacts + local ARTIFACTS_ROOT=artifacts-coverage mkdir -p ${ARTIFACTS_ROOT} # Create coverage reports root directory @@ -71,9 +71,7 @@ compile_coverage_reports() { jq "." workflow.json # Record run ID of PR job - if [[ $EVENT == "pull_request" ]]; then - PR_RUN_IDS="$PR_RUN_IDS $RUN_ID" - fi + [[ $EVENT != "pull_request" ]] || PR_RUN_IDS="$PR_RUN_IDS $RUN_ID" # Create workflow artifacts directory local ARTIFACTS_DIR=${ARTIFACTS_ROOT}/${RUN_ID} @@ -170,7 +168,7 @@ compile_rtlmeter_reports() { jq "." completedRuns.json # Create artifacts root directory - local ARTIFACTS_ROOT=artifacts + local ARTIFACTS_ROOT=artifacts-rtlmeter mkdir -p ${ARTIFACTS_ROOT} # Create rtlmeter reports root directory @@ -197,20 +195,18 @@ compile_rtlmeter_reports() { jq "." workflow.json # Record run ID of PR job - if [[ $EVENT == "pull_request" ]]; then - PR_RUN_IDS="$PR_RUN_IDS $RUN_ID" - fi + [[ $EVENT != "pull_request" ]] || PR_RUN_IDS="$PR_RUN_IDS $RUN_ID" # Create workflow artifacts directory local ARTIFACTS_DIR=${ARTIFACTS_ROOT}/${RUN_ID} mkdir -p ${ARTIFACTS_DIR} # Download artifacts of this run, if exists - gh run download ${RUN_ID} --name rtlmeter-pr-report --dir ${ARTIFACTS_DIR} || true + gh run download ${RUN_ID} --name rtlmeter-report --dir ${ARTIFACTS_DIR} || true ls -lsha ${ARTIFACTS_DIR} # Move on if no RTLMeter report is available - if [ ! -f ${ARTIFACTS_DIR}/report.txt ]; then + if [ ! -d ${ARTIFACTS_DIR}/report ]; then echo "No RTLMeter report found" continue fi @@ -222,42 +218,21 @@ compile_rtlmeter_reports() { echo "

RTLMeter reports for '${EVENT}' runs:

" >> ${CONTENTS} fi - # Extract run metadata - local WORKFLOW_CREATED=$(jq -r '.createdAt' workflow.json) - local WOFKRLOW_NUMBER=$(jq -r '.number' workflow.json) - - # Wrap the report into an HTML page. The report content is already HTML - # (produced by ci-rtlmeter-pr-report.bash), so we just embed it in the - # page body. - cat > ${RTLMETER_ROOT}/${RUN_ID}.html < - - - Verilator RTLMeter report #${WOFKRLOW_NUMBER} - - - - -$(cat ${ARTIFACTS_DIR}/report.txt) - - - -REPORT_TEMPLATE + # Create pages subdirectory + mv ${ARTIFACTS_DIR}/report ${RTLMETER_ROOT}/${RUN_ID} # Add index page content + local WORKFLOW_CREATED=$(jq -r '.createdAt' workflow.json) + local WOFKRLOW_NUMBER=$(jq -r '.number' workflow.json) cat >> ${CONTENTS} <#${WOFKRLOW_NUMBER} + Run #${WOFKRLOW_NUMBER} | GitHub: ${RUN_ID} | started at: ${WORKFLOW_CREATED} CONTENTS_TEMPLATE + if [ -e ${ARTIFACTS_DIR}/pr-number.txt ]; then + local PRNUMBER=$(cat ${ARTIFACTS_DIR}/pr-number.txt) + echo " | Pull request: #${PRNUMBER}" >> ${CONTENTS} + fi echo "
" >> ${CONTENTS} done diff --git a/ci/ci-rtlmeter-pr-report.bash b/ci/ci-rtlmeter-report.bash similarity index 84% rename from ci/ci-rtlmeter-pr-report.bash rename to ci/ci-rtlmeter-report.bash index f44f074e2..fd231c732 100755 --- a/ci/ci-rtlmeter-pr-report.bash +++ b/ci/ci-rtlmeter-report.bash @@ -26,9 +26,9 @@ RUNS="$@" SCRIPT_DIR=$(readlink -f $(dirname ${BASH_SOURCE[0]})) # Move into a temporary directory -rm -rf rtlmeter-pr-report -mkdir rtlmeter-pr-report -pushd rtlmeter-pr-report &> /dev/null +rm -rf rtlmeter-report +mkdir rtlmeter-report +pushd rtlmeter-report &> /dev/null TMP_DIR=$(readlink -f .) # Artifacts to download @@ -83,7 +83,7 @@ for r in $RUNS; do done # Create summary -venv/bin/python3 $SCRIPT_DIR/ci-rtlmeter-pr-report.py ${SUMMARY_ARGS[@]} > $TMP_DIR/summary.txt +venv/bin/python3 $SCRIPT_DIR/ci-rtlmeter-report.py ${SUMMARY_ARGS[@]} > $TMP_DIR/summary.txt # Print it cat $TMP_DIR/summary.txt @@ -102,22 +102,17 @@ Blah Blah Blah NOTIFICATION_TEMPLATE # Create detailed report -REPORT=$TMP_DIR/report.txt +REPORT=$TMP_DIR/body.html cat > $REPORT < - Summary of all runs - 
+
Summary of all runs

+
 $(cat $TMP_DIR/summary.txt)
-  

-
+
SUMMARY_TEMPLATE -echo "
" >> $REPORT -echo " Detailed results" >> $REPORT +echo "

Detailed results

" >> $REPORT for r in $RUNS; do RUN_NAME=$(jq -rj ".[0].runName" $REF_DIR/all-results-$r.json) - echo "
" >> $REPORT - echo " $RUN_NAME" >> $REPORT + echo "

$RUN_NAME

" >> $REPORT for f in $(ls -1 $TMP_DIR/$r-frag-verilate-*.txt | sort) \ $(ls -1 $TMP_DIR/$r-frag-cppbuild-*.txt | sort) \ $(ls -1 $TMP_DIR/$r-frag-execute-*.txt | sort); do @@ -130,6 +125,29 @@ $(tail -n +2 $f)
DETAIL_TAMPLATE done - echo "
" >> $REPORT done -echo "" >> $REPORT + +# Turn the report into a proper HTML page +mkdir -p ${TMP_DIR}/report +cat > ${TMP_DIR}/report/index.html < + + + Verilator RTLMeter report #${NEW_NUM} + + + + +$(cat ${TMP_DIR}/body.html) + + + +INDEX_TEMPLATE diff --git a/ci/ci-rtlmeter-pr-report.py b/ci/ci-rtlmeter-report.py similarity index 100% rename from ci/ci-rtlmeter-pr-report.py rename to ci/ci-rtlmeter-report.py diff --git a/test_regress/t/t_dist_whitespace.py b/test_regress/t/t_dist_whitespace.py index 87f1fe8ef..a14988064 100755 --- a/test_regress/t/t_dist_whitespace.py +++ b/test_regress/t/t_dist_whitespace.py @@ -13,7 +13,7 @@ test.scenarios('dist') Tabs_Exempt_Re = r'(\.out$)|(/fstcpp)|(Makefile)|(\.mk$)|(\.mk\.in$)|test_regress/t/t_preproc\.v|install-sh' -Unicode_Exempt_Re = r'(Changes$|CONTRIBUTORS$|LICENSES?|contributors.rst$|spelling.txt$|ci-rtlmeter-pr-report.py)' +Unicode_Exempt_Re = r'(Changes$|CONTRIBUTORS$|LICENSES?|contributors.rst$|spelling.txt$|ci-rtlmeter-report.py)' def get_source_files(): From 4e3be5641531e212aed73bd5da455636416d7911 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sun, 7 Jun 2026 11:50:02 +0100 Subject: [PATCH 09/32] CI: Improve RTLMeter reports (#7725) --- ci/ci-rtlmeter-report.bash | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ci/ci-rtlmeter-report.bash b/ci/ci-rtlmeter-report.bash index fd231c732..8fadd8416 100755 --- a/ci/ci-rtlmeter-report.bash +++ b/ci/ci-rtlmeter-report.bash @@ -58,6 +58,13 @@ REF_DATE=$(gh run view $REF_ID --json createdAt --jq ".createdAt") NEW_URL=$(gh run view $NEW_ID --json url --jq ".url") NEW_NUM=$(gh run view $NEW_ID --json number --jq ".number") +# Repository owner and name of the default repository, used to build the +# GitHub Pages URL of the detailed report. The owner is lowercased, as required +# for the '.github.io' Pages domain. Resolved here, while still in the +# default repository's git context (before the 'cd rtlmeter' below). +PAGES_OWNER=$(gh repo view --json owner --jq '.owner.login' | tr '[:upper:]' '[:lower:]') +PAGES_NAME=$(gh repo view --json name --jq '.name') + # Go back to root directory popd &> /dev/null # Go to RTLMeter directory @@ -93,12 +100,13 @@ cat > $NOTIFICATION < - Summary of all runs + Summary of all runs 
 $(cat $TMP_DIR/summary.txt)
-Blah Blah Blah + +Detailed report: [${NEW_ID}](https://${PAGES_OWNER}.github.io/${PAGES_NAME}/rtlmeter-reports/${NEW_ID}/index.html) NOTIFICATION_TEMPLATE # Create detailed report From f20076c4a3a0ca155f34458fe632c8f5e4a31d77 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sun, 7 Jun 2026 08:34:17 -0400 Subject: [PATCH 10/32] Commentary: Changes update --- Changes | 19 ++++++++++--------- test_regress/t/t_interface_hierparam_bits.v | 14 ++++++++++---- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/Changes b/Changes index e1e9d3d46..fcb769eae 100644 --- a/Changes +++ b/Changes @@ -21,6 +21,15 @@ Verilator 5.049 devel **Other:** +* Add `+verilator+log+file` (#4505) (#7645). [Tracy Narine] +* Add peak memory usage to `--stats`. [Geza Lore, Testorrent USA, Inc.] +* Add error on mixed-initialization (#7352) (#7357). +* Add `--coverage-per-instance` (#7636). [Yogish Sekhar] +* Add NOTREDOP error on reduction and negation operators (#7417) (#7623) (#7624). +* Add hierarchy-aware reporting to `verilator_coverage` (#7657). [Yogish Sekhar] +* Improve `--coverage-fsm` (#7490) (#7529) (#7561) (#7573) (#7619). [Yogish Sekhar] +* Change `+verilator+seed` to default to 1, and 0 to randomly select (#7325) (#7516). [Miguel] +* Change JSON to include parameter constant mnemonics for FSM Coverage (#7531). [Yogish Sekhar] * Support printing enum names for %p and %s (#5523) (#7338 repair) (#7521) (#7527). [Nick Brereton] * Support weak `until` / `until_with` property operators (#7290) (#7548) (#7685). [Yilou Wang] * Support `s_eventually` (#7291) (#7508). [Bartłomiej Chmiel, Antmicro Ltd.] @@ -50,15 +59,6 @@ Verilator 5.049 devel * Support if/if-else in properties (#7692). [Artur Bieniek, Antmicro Ltd.] * Support process::self().srand() (#7695). [Igor Zaworski, Antmicro Ltd.] * Support MacOS lldb (#7697). [Tracy Narine] -* Add `+verilator+log+file` (#4505) (#7645). [Tracy Narine] -* Add peak memory usage to `--stats`. [Geza Lore, Testorrent USA, Inc.] -* Add error on mixed-initialization (#7352) (#7357). -* Add `--coverage-per-instance` (#7636). [Yogish Sekhar] -* Add NOTREDOP error on reduction and negation operators (#7417) (#7623) (#7624). -* Add hierarchy-aware reporting to `verilator_coverage` (#7657). [Yogish Sekhar] -* Improve `--coverage-fsm` (#7490) (#7529) (#7561) (#7573) (#7619). [Yogish Sekhar] -* Change `+verilator+seed` to default to 1, and 0 to randomly select (#7325) (#7516). [Miguel] -* Change JSON to include parameter constant mnemonics for FSM Coverage (#7531). [Yogish Sekhar] * Optimize emitting to_string() for compiler speedup (#7468). [Jakub Michalski, Antmicro Ltd.] * Optimize additional DFG peephole cases (#7553). [Varun Koyyalagunta, Testorrent USA, Inc.] * Optimize forced signal handling (#7554 partial) (#7572) (#7594) (#7596). [Krzysztof Bieganski, Artur Bieniek, Antmicro Ltd.] @@ -99,6 +99,7 @@ Verilator 5.049 devel * Fix internal error on consecutive repetition with N > 256 (#7552) (#7603). [Yilou Wang] * Fix inherited rand array with .size + foreach constraint (#7558) (#7650). [Yilou Wang] * Fix biased bit distribution under value < (1 << N) constraints (#7563) (#7684). [Yilou Wang] +* Fix HIERPARAM lint checking (#7570) (#7690). [em2machine] * Fix display of %m in non-first argument (#7574). * Fix floating point compile warning on min/max delays. * Fix force of unpacked arrays (#7579) (#7580). [Zubin Jain] diff --git a/test_regress/t/t_interface_hierparam_bits.v b/test_regress/t/t_interface_hierparam_bits.v index 47e58e753..6064bade7 100644 --- a/test_regress/t/t_interface_hierparam_bits.v +++ b/test_regress/t/t_interface_hierparam_bits.v @@ -17,9 +17,9 @@ interface axi_if #( parameter int ID_W = 8, parameter int ADDR_W = 32 ); - logic [ID_W-1:0] AWID; + logic [ID_W-1:0] AWID; logic [ADDR_W-1:0] AWADDR; - logic [7:0] AWLEN; + logic [7:0] AWLEN; endinterface module chkmod #( @@ -47,8 +47,14 @@ module bridge #( endmodule module t; - axi_if #(.ID_W(12), .ADDR_W(64)) if0 (); // 12 + 64 + 8 = 84 - axi_if #(.ID_W(12), .ADDR_W(16)) if1 (); // 12 + 16 + 8 = 36 + axi_if #( + .ID_W(12), + .ADDR_W(64) + ) if0 (); // 12 + 64 + 8 = 84 + axi_if #( + .ID_W(12), + .ADDR_W(16) + ) if1 (); // 12 + 16 + 8 = 36 bridge #(.EXPECT(84)) dut0 (.axi(if0)); bridge #(.EXPECT(36)) dut1 (.axi(if1)); From d62ebf9e6de59a8a76d92b1367b4d0a4da4c72ea Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sun, 7 Jun 2026 08:40:07 -0400 Subject: [PATCH 11/32] Update github issue templates --- .github/ISSUE_TEMPLATE/issue.md | 20 ++++++++++++++++++-- .github/ISSUE_TEMPLATE/questions.md | 10 +++++++++- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/issue.md b/.github/ISSUE_TEMPLATE/issue.md index 121f9d197..53cfabfe1 100644 --- a/.github/ISSUE_TEMPLATE/issue.md +++ b/.github/ISSUE_TEMPLATE/issue.md @@ -7,18 +7,34 @@ assignees: '' --- + + +Can you please attach an example that shows the issue or missing feature? + + What output from that test indicates it is wrong, and what is the correct or expected output? (Or, please make test self-checking if possible.) + + What 'verilator' command line do we use to run your example? + + What 'verilator --version' are you using? Did you try it with the git master version? Did you try it with other simulators? + + What OS and distribution are you using? + + May we assist you in trying to fix this in Verilator yourself? -(Please avoid attaching screenshots that show text - you can convert images to text using e.g. https://ocr.space) + diff --git a/.github/ISSUE_TEMPLATE/questions.md b/.github/ISSUE_TEMPLATE/questions.md index c162a5ff8..a9ed1beee 100644 --- a/.github/ISSUE_TEMPLATE/questions.md +++ b/.github/ISSUE_TEMPLATE/questions.md @@ -7,6 +7,14 @@ assignees: '' --- + + How may we help - what is your question? -(If reporting a bug or requesting a feature please hit BACK on your browser and use a different issue templates.) + From d9cbc279026dcb801a1c3240e4405df5566b6cd7 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sun, 7 Jun 2026 08:47:00 -0400 Subject: [PATCH 12/32] Disable blank github issues --- .github/ISSUE_TEMPLATE/config.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/config.yml diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 000000000..bd9dfe4ef --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,2 @@ +--- +blank_issues_enabled: false From 83ef69d86621d3ea42896b4ea1bf205bbbb806d8 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sun, 7 Jun 2026 15:36:34 +0100 Subject: [PATCH 13/32] CI: Run RTLMeter on the same machine with target commit (#7726) --- .github/workflows/reusable-rtlmeter-build.yml | 31 ++++- .github/workflows/reusable-rtlmeter-run.yml | 124 ++++++++++++++---- .github/workflows/rtlmeter.yml | 109 +++++++++++++-- ci/ci-rtlmeter-report.bash | 61 +++++---- 4 files changed, 256 insertions(+), 69 deletions(-) diff --git a/.github/workflows/reusable-rtlmeter-build.yml b/.github/workflows/reusable-rtlmeter-build.yml index 85deb9706..321033a72 100644 --- a/.github/workflows/reusable-rtlmeter-build.yml +++ b/.github/workflows/reusable-rtlmeter-build.yml @@ -15,6 +15,14 @@ on: description: "Compiler to use: 'gcc' or 'clang'" type: string required: true + sha: + description: "Git SHA to build" + type: string + required: true + outputs: + archive: + description: "Name of the built installation archive artifact" + value: ${{ jobs.build.outputs.archive }} defaults: run: @@ -26,8 +34,10 @@ env: jobs: build: - runs-on: ${{ inputs.runs-on }} name: Build + runs-on: ${{ inputs.runs-on }} + outputs: + archive: ${{ steps.create-archive.outputs.archive }} steps: - name: Install dependencies run: | @@ -43,13 +53,17 @@ jobs: uses: actions/cache@v5 with: path: ccache - key: rtlmeter-build-ccache-${{ inputs.runs-on }}-${{ inputs.cc }}-${{ github.run_id }}-${{ github.run_attempt }} - restore-keys: rtlmeter-build-ccache-${{ inputs.runs-on }}-${{ inputs.cc }} + key: rtlmeter-build-ccache-${{ inputs.runs-on }}-${{ inputs.cc }}-${{ inputs.sha }}-${{ github.run_id }}-${{ github.run_attempt }} + restore-keys: | + rtlmeter-build-ccache-${{ inputs.runs-on }}-${{ inputs.cc }}-${{ inputs.sha }}-${{ github.run_id }} + rtlmeter-build-ccache-${{ inputs.runs-on }}-${{ inputs.cc }}-${{ inputs.sha }} + rtlmeter-build-ccache-${{ inputs.runs-on }}-${{ inputs.cc }} - name: Checkout uses: actions/checkout@v6 with: path: repo + ref: ${{ inputs.sha }} fetch-depth: 0 # Required for 'git describe' used for 'verilator --version' - name: Configure @@ -67,11 +81,16 @@ jobs: run: make install - name: Tar up installation - run: tar --posix -c -z -f verilator-rtlmeter.tar.gz install + id: create-archive + run: | + SHA=$(git -C repo rev-parse HEAD) + ARCHIVE=verilator-$SHA-rtlmeter-${{ inputs.runs-on }}-${{ inputs.cc }}.tar.gz + tar --posix -c -z -f $ARCHIVE install + echo "archive=$ARCHIVE" >> $GITHUB_OUTPUT - name: Upload Verilator installation archive uses: actions/upload-artifact@v7 with: - path: verilator-rtlmeter.tar.gz - name: verilator-rtlmeter-${{ inputs.runs-on }}-${{ inputs.cc }} + path: ${{ steps.create-archive.outputs.archive }} + name: ${{ steps.create-archive.outputs.archive }} overwrite: true diff --git a/.github/workflows/reusable-rtlmeter-run.yml b/.github/workflows/reusable-rtlmeter-run.yml index 5e163f555..ebbaffef8 100644 --- a/.github/workflows/reusable-rtlmeter-run.yml +++ b/.github/workflows/reusable-rtlmeter-run.yml @@ -19,6 +19,15 @@ on: description: "Compiler to use: 'gcc' or 'clang'" type: string required: true + verilator-archive-new: + description: "Name of the installation archive artifact from reusable-rtlmeter-build, new version" + type: string + required: true + verilator-archive-old: + description: "Name of the installation archive artifact from reusable-rtlmeter-build, old version" + type: string + required: false + default: "" # Note: The combination of 'cases' and 'run-name' must be unique for all # invocations of this workflow within a run of the parent workflow. # These two are used together to generate a unique results file name. @@ -63,24 +72,6 @@ jobs: sudo apt install ccache mold libfl-dev libjemalloc-dev libsystemc-dev || \ sudo apt install ccache mold libfl-dev libjemalloc-dev libsystemc-dev - - name: Download Verilator installation archive - uses: actions/download-artifact@v8 - with: - name: verilator-rtlmeter-${{ inputs.runs-on }}-${{ inputs.cc }} - - - name: Unpack Verilator installation archive - run: | - tar -x -z -f verilator-rtlmeter.tar.gz - echo "${{ github.workspace }}/install/bin" >> $GITHUB_PATH - - - name: Use saved ccache - if: ${{ env.CCACHE_DISABLE == 0 }} - uses: actions/cache@v5 - with: - path: ${{ env.CCACHE_DIR }} - key: rtlmeter-run-ccache-${{ inputs.runs-on }}-${{ inputs.cc }}-${{ inputs.cases }}-${{ inputs.compileArgs }}-${{ github.run_id }}-${{ github.run_attempt }} - restore-keys: rtlmeter-run-ccache-${{ inputs.runs-on }}-${{ inputs.cc }}-${{ inputs.cases }}-${{ inputs.compileArgs }} - - name: Checkout RTLMeter uses: actions/checkout@v6 with: @@ -91,32 +82,59 @@ jobs: working-directory: rtlmeter run: make venv - - name: Compile cases + - name: Use saved ccache + if: ${{ env.CCACHE_DISABLE == 0 }} + uses: actions/cache@v5 + with: + path: ${{ env.CCACHE_DIR }} + key: rtlmeter-run-ccache-${{ inputs.runs-on }}-${{ inputs.cc }}-${{ inputs.cases }}-${{ inputs.compileArgs }}-${{ github.run_id }}-${{ github.run_attempt }} + restore-keys: rtlmeter-run-ccache-${{ inputs.runs-on }}-${{ inputs.cc }}-${{ inputs.cases }}-${{ inputs.compileArgs }} + + ######################################################################## + # Run with new Verilator + ######################################################################## + + - name: Download Verilator installation archive - new + uses: actions/download-artifact@v8 + with: + name: ${{ inputs.verilator-archive-new }} + + - name: Unpack Verilator installation archive - new + run: | + tar -x -z -f ${{ inputs.verilator-archive-new }} + mv install verilator-new + + - name: Compile cases - new working-directory: rtlmeter run: | + export PATH="${{ github.workspace }}/verilator-new/bin:$PATH" ./rtlmeter run --timeout 60 --verbose --cases='${{inputs.cases}}' --compileArgs='${{inputs.compileArgs}}' --executeArgs='${{inputs.executeArgs}}' --nExecute=0 + ccache -svv - - name: Execute cases + - name: Execute cases - new working-directory: rtlmeter continue-on-error: true # Do not fail on error, so we can at least save the successful results run: | + export PATH="${{ github.workspace }}/verilator-new/bin:$PATH" ./rtlmeter run --timeout 60 --verbose --cases='${{inputs.cases}}' --compileArgs='${{inputs.compileArgs}}' --executeArgs='${{inputs.executeArgs}}' - - name: Collate results + - name: Collate results - new id: results working-directory: rtlmeter run: | + export PATH="${{ github.workspace }}/verilator-new/bin:$PATH" # Use 'inputs.cases' and 'inputs.run-name' to generate a unique file name hash=$(md5sum <<< '${{ inputs.cases }} ${{ inputs.run-name }}' | awk '{print $1}') echo "hash=${hash}" >> $GITHUB_OUTPUT ./rtlmeter collate --runName "${{ inputs.run-name }}" > ../results-${hash}.json - - name: Report results + - name: Report results - new working-directory: rtlmeter run: | + export PATH="${{ github.workspace }}/verilator-new/bin:$PATH" ./rtlmeter report --steps '*' --metrics '*' ../results-${{ steps.results.outputs.hash }}.json - - name: Upload results + - name: Upload results - new uses: actions/upload-artifact@v7 with: path: results-${{ steps.results.outputs.hash }}.json @@ -124,7 +142,65 @@ jobs: overwrite: true retention-days: 2 - - name: Report status + - name: Report status - new working-directory: rtlmeter run: |- # This will fail the job if any of the runs failed + export PATH="${{ github.workspace }}/verilator-new/bin:$PATH" ./rtlmeter run --verbose --cases='${{inputs.cases}}' --compileArgs='${{inputs.compileArgs}}' --executeArgs='${{inputs.executeArgs}}' + # Clean up for run with old version + rm -rf work + rm -rf ${{ github.workspace }}/verilator-new + + ######################################################################## + # Run with old Verilator **on same machine for consistency** + ######################################################################## + + - name: Download Verilator installation archive - old + if: ${{ inputs.verilator-archive-old != '' }} + uses: actions/download-artifact@v8 + with: + name: ${{ inputs.verilator-archive-old }} + + - name: Unpack Verilator installation archive - old + if: ${{ inputs.verilator-archive-old != '' }} + run: | + tar -x -z -f ${{ inputs.verilator-archive-old }} + mv install verilator-old + + - name: Compile cases - old + if: ${{ inputs.verilator-archive-old != '' }} + working-directory: rtlmeter + run: | + export PATH="${{ github.workspace }}/verilator-old/bin:$PATH" + ./rtlmeter run --timeout 60 --verbose --cases='${{inputs.cases}}' --compileArgs='${{inputs.compileArgs}}' --executeArgs='${{inputs.executeArgs}}' --nExecute=0 + ccache -svv + + - name: Execute cases - old + if: ${{ inputs.verilator-archive-old != '' }} + working-directory: rtlmeter + run: | + export PATH="${{ github.workspace }}/verilator-old/bin:$PATH" + ./rtlmeter run --timeout 60 --verbose --cases='${{inputs.cases}}' --compileArgs='${{inputs.compileArgs}}' --executeArgs='${{inputs.executeArgs}}' + + - name: Collate results - old + if: ${{ inputs.verilator-archive-old != '' }} + working-directory: rtlmeter + run: | + export PATH="${{ github.workspace }}/verilator-old/bin:$PATH" + ./rtlmeter collate --runName "${{ inputs.run-name }}" > ../reference-${{ steps.results.outputs.hash }}.json + + - name: Report results - old + if: ${{ inputs.verilator-archive-old != '' }} + working-directory: rtlmeter + run: | + export PATH="${{ github.workspace }}/verilator-old/bin:$PATH" + ./rtlmeter report --steps '*' --metrics '*' ../reference-${{ steps.results.outputs.hash }}.json + + - name: Upload results - old + if: ${{ inputs.verilator-archive-old != '' }} + uses: actions/upload-artifact@v7 + with: + path: reference-${{ steps.results.outputs.hash }}.json + name: rtlmeter-${{ inputs.tag }}-reference-${{ steps.results.outputs.hash }} + overwrite: true + retention-days: 2 diff --git a/.github/workflows/rtlmeter.yml b/.github/workflows/rtlmeter.yml index 3c633d9f0..e5d632b9f 100644 --- a/.github/workflows/rtlmeter.yml +++ b/.github/workflows/rtlmeter.yml @@ -40,34 +40,73 @@ jobs: (github.event_name == 'workflow_dispatch') || (github.event_name == 'push') runs-on: ubuntu-24.04 + outputs: + old-sha: ${{ steps.start.outputs.old-sha }} steps: - name: Startup - run: echo + id: start + env: + GH_TOKEN: ${{ github.token }} + run: | + [[ "${{ github.event_name }}" == 'pull_request' ]] || exit 0 + # For a pull request, 'github.sha' is the test merge commit. Its + # first parent is the target branch commit it was merged with for this run. + OLD_SHA="$(gh api "repos/${{ github.repository }}/commits/${{ github.sha }}" --jq '.parents[0].sha')" + echo "old-sha=$OLD_SHA" >> "$GITHUB_OUTPUT" - build-gcc: - name: Build GCC + build-gcc-new: + name: Build New Verilator - GCC needs: start uses: ./.github/workflows/reusable-rtlmeter-build.yml with: runs-on: ubuntu-24.04 cc: gcc + sha: ${{ github.sha }} - build-clang: - name: Build Clang + build-clang-new: + name: Build New Verilator - Clang needs: start uses: ./.github/workflows/reusable-rtlmeter-build.yml with: runs-on: ubuntu-24.04 cc: clang + sha: ${{ github.sha }} + + build-gcc-old: + name: Build Old Verilator - GCC + needs: start + if: ${{ needs.start.outputs.old-sha != '' }} + uses: ./.github/workflows/reusable-rtlmeter-build.yml + with: + runs-on: ubuntu-24.04 + cc: gcc + sha: ${{ needs.start.outputs.old-sha }} + + build-clang-old: + name: Build Old Verilator - Clang + needs: start + if: ${{ needs.start.outputs.old-sha != '' }} + uses: ./.github/workflows/reusable-rtlmeter-build.yml + with: + runs-on: ubuntu-24.04 + cc: clang + sha: ${{ needs.start.outputs.old-sha }} run-gcc: name: Run GCC | ${{ matrix.cases }} - needs: build-gcc + needs: + - build-gcc-new + - build-gcc-old + # Run even if 'build-*-old' was skipped (no old SHA), but not if any + # dependency failed or the workflow was cancelled. + if: ${{ !failure() && !cancelled() }} uses: ./.github/workflows/reusable-rtlmeter-run.yml with: tag: gcc runs-on: ubuntu-24.04 cc: gcc + verilator-archive-new: ${{ needs.build-gcc-new.outputs.archive }} + verilator-archive-old: ${{ needs.build-gcc-old.outputs.archive }} cases: ${{ matrix.cases }} run-name: "gcc" compileArgs: "" @@ -108,12 +147,19 @@ jobs: run-clang: name: Run Clang | ${{ matrix.cases }} - needs: build-clang + needs: + - build-clang-new + - build-clang-old + # Run even if 'build-*-old' was skipped (no old SHA), but not if any + # dependency failed or the workflow was cancelled. + if: ${{ !failure() && !cancelled() }} uses: ./.github/workflows/reusable-rtlmeter-run.yml with: tag: clang runs-on: ubuntu-24.04 cc: clang + verilator-archive-new: ${{ needs.build-clang-new.outputs.archive }} + verilator-archive-old: ${{ needs.build-clang-old.outputs.archive }} cases: ${{ matrix.cases }} run-name: "clang --threads 4" compileArgs: "--threads 4" @@ -154,12 +200,19 @@ jobs: run-gcc-hier: name: Run GCC hier | ${{ matrix.cases }} - needs: build-gcc + needs: + - build-gcc-new + - build-gcc-old + # Run even if 'build-*-old' was skipped (no old SHA), but not if any + # dependency failed or the workflow was cancelled. + if: ${{ !failure() && !cancelled() }} uses: ./.github/workflows/reusable-rtlmeter-run.yml with: tag: gcc-hier runs-on: ubuntu-24.04 cc: gcc + verilator-archive-new: ${{ needs.build-gcc-new.outputs.archive }} + verilator-archive-old: ${{ needs.build-gcc-old.outputs.archive }} cases: ${{ matrix.cases }} run-name: "gcc --hierarchical" compileArgs: "--hierarchical" @@ -181,9 +234,14 @@ jobs: combine-results: name: Combine results needs: [run-gcc, run-clang, run-gcc-hier] - # Run if any of the dependencies have run, even if failed. - # That is: do not run if all skipped, or the workflow was cancelled. - if: ${{ (contains(needs.*.result, 'success') || contains(needs.*.result, 'failure')) && !cancelled() }} + # Skip if cancelled. + # On PRs, run if something succeded and nothign failed. + # On non-PRs, run if anything succeded or failed (so partial results are still publided). + if: >- + ${{ !cancelled() && ( + (github.event_name == 'pull_request' && (contains(needs.*.result, 'success') && !contains(needs.*.result, 'failure'))) + || (github.event_name != 'pull_request' && (contains(needs.*.result, 'success') || contains(needs.*.result, 'failure'))) + )}} runs-on: ubuntu-24.04 strategy: fail-fast: false @@ -195,9 +253,11 @@ jobs: with: repository: "verilator/rtlmeter" path: rtlmeter + - name: Setup RTLMeter venv working-directory: rtlmeter run: make venv + - name: Download all results uses: actions/download-artifact@v8 with: @@ -216,6 +276,29 @@ jobs: overwrite: true retention-days: 30 + # The reference (old) results only exist for pull requests, where the + # 'build-*-old' jobs run and the run jobs produce '*-reference-*' artifacts. + - name: Download reference results + if: ${{ github.event_name == 'pull_request' }} + uses: actions/download-artifact@v8 + with: + pattern: rtlmeter-${{ matrix.tag }}-reference-* + path: all-reference-${{ matrix.tag }} + merge-multiple: true + - name: Combine reference results + if: ${{ github.event_name == 'pull_request' }} + working-directory: rtlmeter + run: | + ./rtlmeter collate ../all-reference-${{ matrix.tag }}/*.json > ../all-reference-${{ matrix.tag }}.json + - name: Upload reference results + if: ${{ github.event_name == 'pull_request' }} + uses: actions/upload-artifact@v7 + with: + path: all-reference-${{ matrix.tag }}.json + name: all-reference-${{ matrix.tag }} + overwrite: true + retention-days: 30 + publish-scheduled-results: name: Publish results to verilator/verilator-rtlmeter-results needs: combine-results @@ -304,9 +387,7 @@ jobs: ln -s ../rtlmeter rtlmeter gh repo set-default ${{ github.repository }} # Compare to last successful scheduled run - REF_ID=$(gh run list --workflow RTLMeter --event schedule --status success --limit 1 --json databaseId --jq ".[0].databaseId") - NEW_ID=${{ github.run_id }} - ci/ci-rtlmeter-report.bash $REF_ID $NEW_ID gcc clang gcc-hier + ci/ci-rtlmeter-report.bash ${{ github.run_id }} ${{ github.sha }} gcc clang gcc-hier # Create the report artifact mkdir ../report-artifact mv rtlmeter-report/report ../report-artifact/ diff --git a/ci/ci-rtlmeter-report.bash b/ci/ci-rtlmeter-report.bash index 8fadd8416..29380d472 100755 --- a/ci/ci-rtlmeter-report.bash +++ b/ci/ci-rtlmeter-report.bash @@ -15,11 +15,11 @@ [ "$GITHUB_ACTIONS" != "true" ] || set -x # Arguments: -# 1. reference run ID -# 2. new run ID +# 1. run ID +# 2. SHA of the event that triggered the run (for a PR, the test merge commit) # rest: run tags -REF_ID=$1; shift -NEW_ID=$1; shift +RUN_ID=$1; shift +RUN_SHA=$1; shift RUNS="$@" # $VERILATOR_CHECKOUT/ci directory @@ -32,31 +32,30 @@ pushd rtlmeter-report &> /dev/null TMP_DIR=$(readlink -f .) # Artifacts to download -DOWNLOAD_ARTIFACTS="" +DOWNLOAD_NEW_ARTIFACTS="" +DOWNLOAD_REF_ARTIFACTS="" for r in $RUNS; do - DOWNLOAD_ARTIFACTS="$DOWNLOAD_ARTIFACTS --name all-results-$r" + DOWNLOAD_NEW_ARTIFACTS="$DOWNLOAD_NEW_ARTIFACTS --name all-results-$r" + DOWNLOAD_REF_ARTIFACTS="$DOWNLOAD_REF_ARTIFACTS --name all-reference-$r" done -# Download reference run results +# Download reference artifacts mkdir ref REF_DIR=$(readlink -f ref) -gh run download ${REF_ID} $DOWNLOAD_ARTIFACTS --dir $REF_DIR +gh run download ${RUN_ID} $DOWNLOAD_REF_ARTIFACTS --dir $REF_DIR mv $REF_DIR/*/*.json $REF_DIR/ find $REF_DIR -mindepth 1 -type d -delete -# Download current run results +# Download new version artifacts mkdir new NEW_DIR=$(readlink -f new) -gh run download ${NEW_ID} $DOWNLOAD_ARTIFACTS --dir $NEW_DIR +gh run download ${RUN_ID} $DOWNLOAD_NEW_ARTIFACTS --dir $NEW_DIR mv $NEW_DIR/*/*.json $NEW_DIR/ find $NEW_DIR -mindepth 1 -type d -delete # Get Some metadata about the runs -REF_URL=$(gh run view $REF_ID --json url --jq ".url") -REF_NUM=$(gh run view $REF_ID --json number --jq ".number") -REF_DATE=$(gh run view $REF_ID --json createdAt --jq ".createdAt") -NEW_URL=$(gh run view $NEW_ID --json url --jq ".url") -NEW_NUM=$(gh run view $NEW_ID --json number --jq ".number") +RUN_URL=$(gh run view $RUN_ID --json url --jq ".url") +RUN_NUM=$(gh run view $RUN_ID --json number --jq ".number") # Repository owner and name of the default repository, used to build the # GitHub Pages URL of the detailed report. The owner is lowercased, as required @@ -65,6 +64,12 @@ NEW_NUM=$(gh run view $NEW_ID --json number --jq ".number") PAGES_OWNER=$(gh repo view --json owner --jq '.owner.login' | tr '[:upper:]' '[:lower:]') PAGES_NAME=$(gh repo view --json name --jq '.name') +# The 'old' reference was built from the target branch base commit, which is +# the first parent of the triggering merge commit (same as the 'start' job in +# rtlmeter.yml). Resolved here, while still in the default repository's git +# context (before the 'cd rtlmeter' below). +REF_SHA=$(gh api "repos/{owner}/{repo}/commits/$RUN_SHA" --jq '.parents[0].sha') + # Go back to root directory popd &> /dev/null # Go to RTLMeter directory @@ -75,14 +80,14 @@ SUMMARY_ARGS=() for r in $RUNS; do CMP_JSON=$TMP_DIR/cmp-$r.json # Gather args for summary script - SUMMARY_ARGS+=($REF_DIR/all-results-$r.json $CMP_JSON) + SUMMARY_ARGS+=($NEW_DIR/all-results-$r.json $CMP_JSON) # Create JSON ./rtlmeter compare --format json --steps "*" --metrics "*" \ - $REF_DIR/all-results-$r.json $NEW_DIR/all-results-$r.json > $CMP_JSON + $REF_DIR/all-reference-$r.json $NEW_DIR/all-results-$r.json > $CMP_JSON # Also create detailed tables - ./rtlmeter compare --format ascii --steps 'verilate' --metrics '* !system !user' $REF_DIR/all-results-$r.json $NEW_DIR/all-results-$r.json > $TMP_DIR/verilate-$r.txt - ./rtlmeter compare --format ascii --steps 'cppbuild' --metrics '* !system !user' $REF_DIR/all-results-$r.json $NEW_DIR/all-results-$r.json > $TMP_DIR/cppbuild-$r.txt - ./rtlmeter compare --format ascii --steps 'execute' --metrics '* !system !user' $REF_DIR/all-results-$r.json $NEW_DIR/all-results-$r.json > $TMP_DIR/execute-$r.txt + ./rtlmeter compare --format ascii --steps 'verilate' --metrics '*' $REF_DIR/all-reference-$r.json $NEW_DIR/all-results-$r.json > $TMP_DIR/verilate-$r.txt + ./rtlmeter compare --format ascii --steps 'cppbuild' --metrics '*' $REF_DIR/all-reference-$r.json $NEW_DIR/all-results-$r.json > $TMP_DIR/cppbuild-$r.txt + ./rtlmeter compare --format ascii --steps 'execute' --metrics '*' $REF_DIR/all-reference-$r.json $NEW_DIR/all-results-$r.json > $TMP_DIR/execute-$r.txt # Chop them at new lines, into one table per file awk -v RS= -v prefix=$TMP_DIR/$r-frag '{print > sprintf("%s-verilate-%02d.txt",prefix,NR)}' $TMP_DIR/verilate-$r.txt awk -v RS= -v prefix=$TMP_DIR/$r-frag '{print > sprintf("%s-cppbuild-%02d.txt",prefix,NR)}' $TMP_DIR/cppbuild-$r.txt @@ -97,8 +102,10 @@ cat $TMP_DIR/summary.txt # Create notification comment content NOTIFICATION=$TMP_DIR/notification.txt cat > $NOTIFICATION < Summary of all runs 
@@ -106,7 +113,11 @@ $(cat $TMP_DIR/summary.txt)
-Detailed report: [${NEW_ID}](https://${PAGES_OWNER}.github.io/${PAGES_NAME}/rtlmeter-reports/${NEW_ID}/index.html) +The reported numbers are the geometric means of the ratios of the metrics over all cases, +less than 1 is a regression, greater than 1 is an improvement. +The jobs run on variable and noisy runners, so some variance is expected between runs. + +Detailed report: [${RUN_ID}](https://${PAGES_OWNER}.github.io/${PAGES_NAME}/rtlmeter-reports/${RUN_ID}/index.html) NOTIFICATION_TEMPLATE # Create detailed report @@ -119,7 +130,7 @@ $(cat $TMP_DIR/summary.txt) SUMMARY_TEMPLATE echo "

Detailed results

" >> $REPORT for r in $RUNS; do - RUN_NAME=$(jq -rj ".[0].runName" $REF_DIR/all-results-$r.json) + RUN_NAME=$(jq -rj ".[0].runName" $NEW_DIR/all-results-$r.json) echo "

$RUN_NAME

" >> $REPORT for f in $(ls -1 $TMP_DIR/$r-frag-verilate-*.txt | sort) \ $(ls -1 $TMP_DIR/$r-frag-cppbuild-*.txt | sort) \ @@ -141,7 +152,7 @@ cat > ${TMP_DIR}/report/index.html < - Verilator RTLMeter report #${NEW_NUM} + Verilator RTLMeter report #${RUN_NUM}