From b684995292b8943c944850b7b9eef03ecbe18912 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Thu, 19 Nov 2020 21:32:33 -0500 Subject: [PATCH] Support $random and $urandom seeds. --- Changes | 2 ++ bin/verilator | 8 ++--- include/verilated.cpp | 57 ++++++++++++++++++------------ include/verilated.h | 5 +++ src/V3AstNodes.h | 44 +++++++++++------------ src/V3EmitC.cpp | 3 ++ src/V3Unknown.cpp | 9 ++--- src/V3Width.cpp | 12 +++---- src/verilog.y | 11 +++--- test_regress/t/t_debug_emitv.out | 4 ++- test_regress/t/t_debug_emitv.v | 2 ++ test_regress/t/t_sys_rand_seed.out | 13 ------- test_regress/t/t_sys_rand_seed.pl | 5 ++- test_regress/t/t_urandom.v | 2 -- 14 files changed, 92 insertions(+), 85 deletions(-) delete mode 100644 test_regress/t/t_sys_rand_seed.out diff --git a/Changes b/Changes index a89ef1124..2655ecdfa 100644 --- a/Changes +++ b/Changes @@ -5,6 +5,8 @@ The contributors that suggested a given feature are shown in []. Thanks! * Verilator 4.105 devel +*** Support $random and $urandom seeds. + **** Fix trace signal names getting hashed (#2643). [Barbara Gigerl] **** Fix unpacked array parameters near functions (#2639). [Anderson Ignacio da Silva] diff --git a/bin/verilator b/bin/verilator index 213d90e34..3e67e11c9 100755 --- a/bin/verilator +++ b/bin/verilator @@ -4113,10 +4113,10 @@ similar. =item $random, $urandom, $urandom_range -$random and $urandom do not support the optional argument to set the seed. -Use +verilator+seed argument to set the seed. There is one random seed per -C thread, not per module for $random, nor per object for random stability -of $urandom/$urandom_range. +Use +verilator+seed argument to set the seed if there is no $random or +$urandom optional argument to set the seed. There is one random seed per C +thread, not per module for $random, nor per object for random stability of +$urandom/$urandom_range. =item $readmemb, $readmemh diff --git a/include/verilated.cpp b/include/verilated.cpp index 9f3f23b64..cbe5655fd 100644 --- a/include/verilated.cpp +++ b/include/verilated.cpp @@ -256,6 +256,7 @@ Verilated::Serialized::Serialized() { s_errorLimit = 1; s_randReset = 0; s_randSeed = 0; + s_randSeedEpoch = 1; s_timeunit = VL_TIME_UNIT; // Initial value until overriden by _Vconfigure s_timeprecision = VL_TIME_PRECISION; // Initial value until overriden by _Vconfigure } @@ -279,6 +280,8 @@ void* Verilated::serialized2Ptr() VL_MT_UNSAFE { return &VerilatedImp::s_s.v.m_s static vluint32_t vl_sys_rand32() VL_MT_UNSAFE { // Return random 32-bits using system library. // Used only to construct seed for Verilator's PNRG. + static VerilatedMutex s_mutex; + const VerilatedLockGuard lock(s_mutex); // Otherwise rand is unsafe #if defined(_WIN32) && !defined(__CYGWIN__) // Windows doesn't have lrand48(), although Cygwin does. return (rand() << 16) ^ rand(); @@ -288,29 +291,18 @@ static vluint32_t vl_sys_rand32() VL_MT_UNSAFE { } vluint64_t vl_rand64() VL_MT_SAFE { - static VerilatedMutex s_mutex; - static VL_THREAD_LOCAL bool t_seeded = false; + static VL_THREAD_LOCAL vluint32_t t_seedEpoch = 0; static VL_THREAD_LOCAL vluint64_t t_state[2]; - if (VL_UNLIKELY(!t_seeded)) { - t_seeded = true; - { - const VerilatedLockGuard lock(s_mutex); // Otherwise vl_sys_rand32 is unsafe - if (Verilated::randSeed() != 0) { - t_state[0] = ((static_cast(Verilated::randSeed()) << 32) - ^ (static_cast(Verilated::randSeed()))); - t_state[1] = ((static_cast(Verilated::randSeed()) << 32) - ^ (static_cast(Verilated::randSeed()))); - } else { - t_state[0] = ((static_cast(vl_sys_rand32()) << 32) - ^ (static_cast(vl_sys_rand32()))); - t_state[1] = ((static_cast(vl_sys_rand32()) << 32) - ^ (static_cast(vl_sys_rand32()))); - } - // Fix state as algorithm is slow to randomize if many zeros - // This causes a loss of ~ 1 bit of seed entropy, no big deal - if (VL_COUNTONES_I(t_state[0]) < 10) t_state[0] = ~t_state[0]; - if (VL_COUNTONES_I(t_state[1]) < 10) t_state[1] = ~t_state[1]; - } + // For speed, we use a thread-local epoch number to know when to reseed + if (VL_UNLIKELY(t_seedEpoch != Verilated::randSeedEpoch())) { + // Set epoch before state, in case races with new seeding + t_seedEpoch = Verilated::randSeedEpoch(); + t_state[0] = Verilated::randSeedDefault64(); + t_state[1] = t_state[0]; + // Fix state as algorithm is slow to randomize if many zeros + // This causes a loss of ~ 1 bit of seed entropy, no big deal + if (VL_COUNTONES_I(t_state[0]) < 10) t_state[0] = ~t_state[0]; + if (VL_COUNTONES_I(t_state[1]) < 10) t_state[1] = ~t_state[1]; } // Xoroshiro128+ algorithm vluint64_t result = t_state[0] + t_state[1]; @@ -334,6 +326,11 @@ WDataOutP VL_RANDOM_W(int obits, WDataOutP outwp) VL_MT_SAFE { } // LCOV_EXCL_STOP +IData VL_RANDOM_SEEDED_II(int obits, IData seed) VL_MT_SAFE { + Verilated::randSeed(static_cast(seed)); + return VL_RANDOM_I(obits); +} + IData VL_RAND_RESET_I(int obits) VL_MT_SAFE { if (Verilated::randReset() == 0) return 0; IData data = ~0; @@ -2223,6 +2220,22 @@ void Verilated::randReset(int val) VL_MT_SAFE { void Verilated::randSeed(int val) VL_MT_SAFE { const VerilatedLockGuard lock(m_mutex); s_s.s_randSeed = val; + vluint64_t newEpoch = s_s.s_randSeedEpoch + 1; + if (VL_UNLIKELY(newEpoch == 0)) newEpoch = 1; + // Obververs must see new epoch AFTER seed updated +#ifdef VL_THREADED + std::atomic_signal_fence(std::memory_order_release); +#endif + s_s.s_randSeedEpoch = newEpoch; +} +vluint64_t Verilated::randSeedDefault64() VL_MT_SAFE { + if (Verilated::randSeed() != 0) { + return ((static_cast(Verilated::randSeed()) << 32) + ^ (static_cast(Verilated::randSeed()))); + } else { + return ((static_cast(vl_sys_rand32()) << 32) + ^ (static_cast(vl_sys_rand32()))); + } } void Verilated::calcUnusedSigs(bool flag) VL_MT_SAFE { const VerilatedLockGuard lock(m_mutex); diff --git a/include/verilated.h b/include/verilated.h index fb2ca3671..06bcb7741 100644 --- a/include/verilated.h +++ b/include/verilated.h @@ -387,6 +387,7 @@ class Verilated final { int s_errorLimit; ///< Stop on error number int s_randReset; ///< Random reset: 0=all 0s, 1=all 1s, 2=random int s_randSeed; ///< Random seed: 0=random + int s_randSeedEpoch; ///< Number incrementing on each reseed, 0=illegal Serialized(); ~Serialized() = default; } s_s; @@ -443,6 +444,9 @@ public: static int randReset() VL_MT_SAFE { return s_s.s_randReset; } ///< Return randReset value static void randSeed(int val) VL_MT_SAFE; static int randSeed() VL_MT_SAFE { return s_s.s_randSeed; } ///< Return randSeed value + static vluint32_t randSeedEpoch() VL_MT_SAFE { return s_s.s_randSeedEpoch; } + /// Random seed extended to 64 bits, and defaulted if user seed==0 + static vluint64_t randSeedDefault64() VL_MT_SAFE; /// Enable debug of internal verilated code static void debug(int level) VL_MT_SAFE; @@ -649,6 +653,7 @@ extern vluint64_t vl_rand64() VL_MT_SAFE; inline IData VL_RANDOM_I(int obits) VL_MT_SAFE { return vl_rand64() & VL_MASK_I(obits); } inline QData VL_RANDOM_Q(int obits) VL_MT_SAFE { return vl_rand64() & VL_MASK_Q(obits); } extern WDataOutP VL_RANDOM_W(int obits, WDataOutP outwp); ///< Randomize a signal +extern IData VL_RANDOM_SEEDED_II(int obits, IData seed) VL_MT_SAFE; inline IData VL_URANDOM_RANGE_I(IData hi, IData lo) { vluint64_t rnd = vl_rand64(); if (VL_LIKELY(hi > lo)) { diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index d38a39675..f8632dc49 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -5348,22 +5348,33 @@ public: //====================================================================== // non-ary ops -class AstRand final : public AstNodeTermop { +class AstRand final : public AstNodeMath { + // $random/$random(seed) or $urandom/$urandom(seed) // Return a random number, based upon width() private: + bool m_urandom = false; // $urandom vs $random bool m_reset = false; // Random reset, versus always random public: - AstRand(FileLine* fl, AstNodeDType* dtp, bool reset) + class Reset {}; + AstRand(FileLine* fl, Reset, AstNodeDType* dtp, bool reset) : ASTGEN_SUPER(fl) , m_reset{reset} { dtypep(dtp); } - explicit AstRand(FileLine* fl) - : ASTGEN_SUPER(fl) {} + AstRand(FileLine* fl, AstNode* seedp, bool urandom) + : ASTGEN_SUPER(fl) + , m_urandom(urandom) { + setNOp1p(seedp); + } ASTNODE_NODE_FUNCS(Rand) - virtual string emitVerilog() override { return "%f$random"; } + virtual string emitVerilog() override { + return seedp() ? (m_urandom ? "%f$urandom(%l)" : "%f$random(%l)") + : (m_urandom ? "%f$urandom()" : "%f$random()"); + } virtual string emitC() override { - return (m_reset ? "VL_RAND_RESET_%nq(%nw, %P)" : "VL_RANDOM_%nq(%nw, %P)"); + return m_reset + ? "VL_RAND_RESET_%nq(%nw, %P)" + : seedp() ? "VL_RANDOM_SEEDED_%nq%lq(%nw, %P, %li)" : "VL_RANDOM_%nq(%nw, %P)"; } virtual bool cleanOut() const override { return true; } virtual bool isGateOptimizable() const override { return false; } @@ -5371,24 +5382,9 @@ public: virtual int instrCount() const override { return instrCountPli(); } virtual V3Hash sameHash() const override { return V3Hash(); } virtual bool same(const AstNode* samep) const override { return true; } -}; - -class AstURandom final : public AstNodeTermop { - // $urandom -public: - explicit AstURandom(FileLine* fl) - : ASTGEN_SUPER(fl) { - dtypeSetUInt32(); // Says IEEE - } - ASTNODE_NODE_FUNCS(URandom) - virtual string emitVerilog() override { return "%f$urandom"; } - virtual string emitC() override { return "VL_RANDOM_%nq(%nw)"; } - virtual bool cleanOut() const override { return true; } - virtual bool isGateOptimizable() const override { return false; } - virtual bool isPredictOptimizable() const override { return false; } - virtual int instrCount() const override { return instrCountPli(); } - virtual V3Hash sameHash() const override { return V3Hash(); } - virtual bool same(const AstNode* samep) const override { return true; } + AstNode* seedp() const { return op1p(); } + bool reset() const { return m_reset; } + bool urandom() const { return m_urandom; } }; class AstURandomRange final : public AstNodeBiop { diff --git a/src/V3EmitC.cpp b/src/V3EmitC.cpp index 0345ceb44..465036379 100644 --- a/src/V3EmitC.cpp +++ b/src/V3EmitC.cpp @@ -836,6 +836,9 @@ public: putsQuoted(nodep->timeunit().ascii()); puts(");\n"); } + virtual void visit(AstRand* nodep) override { + emitOpName(nodep, nodep->emitC(), nodep->seedp(), nullptr, nullptr); + } virtual void visit(AstTime* nodep) override { puts("VL_TIME_UNITED_Q("); if (nodep->timeunit().isNone()) nodep->v3fatalSrc("$time has no units"); diff --git a/src/V3Unknown.cpp b/src/V3Unknown.cpp index dbfa9a101..8c952c9a0 100644 --- a/src/V3Unknown.cpp +++ b/src/V3Unknown.cpp @@ -288,10 +288,11 @@ private: new AstAssign( nodep->fileline(), new AstVarRef(nodep->fileline(), newvarp, VAccess::WRITE), - new AstOr( - nodep->fileline(), new AstConst(nodep->fileline(), numb1), - new AstAnd(nodep->fileline(), new AstConst(nodep->fileline(), numbx), - new AstRand(nodep->fileline(), nodep->dtypep(), true))))); + new AstOr(nodep->fileline(), new AstConst(nodep->fileline(), numb1), + new AstAnd(nodep->fileline(), + new AstConst(nodep->fileline(), numbx), + new AstRand(nodep->fileline(), AstRand::Reset{}, + nodep->dtypep(), true))))); // Add inits in front of other statement. // In the future, we should stuff the initp into the module's constructor. AstNode* afterp = m_modp->stmtsp()->unlinkFrBackWithNext(); diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 556a0cc9c..32590cf9d 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -1143,12 +1143,12 @@ private: virtual void visit(AstRand* nodep) override { if (m_vup->prelim()) { - nodep->dtypeSetSigned32(); // Says the spec - } - } - virtual void visit(AstURandom* nodep) override { - if (m_vup->prelim()) { - nodep->dtypeSetUInt32(); // Says the spec + if (nodep->urandom()) { + nodep->dtypeSetUInt32(); // Says the spec + } else { + nodep->dtypeSetSigned32(); // Says the spec + } + if (nodep->seedp()) iterateCheckSigned32(nodep, "seed", nodep->seedp(), BOTH); } } virtual void visit(AstURandomRange* nodep) override { diff --git a/src/verilog.y b/src/verilog.y index 4e3fcf687..8ca424f3c 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -3711,9 +3711,8 @@ system_f_call_or_t: // IEEE: part of system_tf_call (can be task or func) | yD_PAST '(' expr ',' expr ',' expr ')' { $$ = $3; BBUNSUP($1, "Unsupported: $past expr2 and clock arguments"); } | yD_PAST '(' expr ',' expr ',' expr ',' expr')' { $$ = $3; BBUNSUP($1, "Unsupported: $past expr2 and clock arguments"); } | yD_POW '(' expr ',' expr ')' { $$ = new AstPowD($1,$3,$5); } - // // Seeding is unsupported as would be slow to invalidate all per-thread RNGs - | yD_RANDOM '(' expr ')' { $$ = new AstRand($1); BBUNSUP($1, "Unsupported: Seed on $random. Suggest use +verilator+seed+ runtime flag"); } - | yD_RANDOM parenE { $$ = new AstRand($1); } + | yD_RANDOM '(' expr ')' { $$ = new AstRand($1, $3, false); } + | yD_RANDOM parenE { $$ = new AstRand($1, nullptr, false); } | yD_REALTIME parenE { $$ = new AstTimeD($1, VTimescale(VTimescale::NONE)); } | yD_REALTOBITS '(' expr ')' { $$ = new AstRealToBits($1,$3); } | yD_REWIND '(' idClassSel ')' { $$ = new AstFSeek($1, $3, new AstConst($1, 0), new AstConst($1, 0)); } @@ -3742,9 +3741,9 @@ system_f_call_or_t: // IEEE: part of system_tf_call (can be task or func) | yD_TYPENAME '(' exprOrDataType ')' { $$ = new AstAttrOf($1, AstAttrType::TYPENAME, $3); } | yD_UNGETC '(' expr ',' expr ')' { $$ = new AstFUngetC($1, $5, $3); } // Arg swap to file first | yD_UNPACKED_DIMENSIONS '(' exprOrDataType ')' { $$ = new AstAttrOf($1,AstAttrType::DIM_UNPK_DIMENSIONS,$3); } - | yD_UNSIGNED '(' expr ')' { $$ = new AstUnsigned($1,$3); } - | yD_URANDOM '(' expr ')' { $$ = new AstURandom($1); BBUNSUP($1, "Unsupported: Seed on $urandom. Suggest use +verilator+seed+ runtime flag"); } - | yD_URANDOM parenE { $$ = new AstURandom($1); } + | yD_UNSIGNED '(' expr ')' { $$ = new AstUnsigned($1, $3); } + | yD_URANDOM '(' expr ')' { $$ = new AstRand($1, $3, true); } + | yD_URANDOM parenE { $$ = new AstRand($1, nullptr, true); } | yD_URANDOM_RANGE '(' expr ',' expr ')' { $$ = new AstURandomRange($1, $3, $5); } | yD_VALUEPLUSARGS '(' expr ',' expr ')' { $$ = new AstValuePlusArgs($1, $3, $5); } ; diff --git a/test_regress/t/t_debug_emitv.out b/test_regress/t/t_debug_emitv.out index 3507dee82..dc1d81c5a 100644 --- a/test_regress/t/t_debug_emitv.out +++ b/test_regress/t/t_debug_emitv.out @@ -175,7 +175,9 @@ module Vt_debug_emitv; if ((32'sh5 != t.i)) begin $stop; end - t.sum = $urandom; + t.sum = + ???? // RAND + 32'sha; end /*verilator public_flat_rw @(posedge clk)@(negedge clk) t.pubflat*/ diff --git a/test_regress/t/t_debug_emitv.v b/test_regress/t/t_debug_emitv.v index b5c7c5875..5764cedcf 100644 --- a/test_regress/t/t_debug_emitv.v +++ b/test_regress/t/t_debug_emitv.v @@ -146,7 +146,9 @@ module t (/*AUTOARG*/ if (i != 5) $stop; sum = $random; + sum = $random(10); sum = $urandom; + sum = $urandom(10); end endmodule diff --git a/test_regress/t/t_sys_rand_seed.out b/test_regress/t/t_sys_rand_seed.out deleted file mode 100644 index a9ae64425..000000000 --- a/test_regress/t/t_sys_rand_seed.out +++ /dev/null @@ -1,13 +0,0 @@ -%Error-UNSUPPORTED: t/t_sys_rand_seed.v:13:16: Unsupported: Seed on $random. Suggest use +verilator+seed+ runtime flag - 13 | valuea = $random(10); - | ^~~~~~~ -%Error-UNSUPPORTED: t/t_sys_rand_seed.v:14:16: Unsupported: Seed on $random. Suggest use +verilator+seed+ runtime flag - 14 | valueb = $random(10); - | ^~~~~~~ -%Error-UNSUPPORTED: t/t_sys_rand_seed.v:16:16: Unsupported: Seed on $urandom. Suggest use +verilator+seed+ runtime flag - 16 | valuea = $urandom(10); - | ^~~~~~~~ -%Error-UNSUPPORTED: t/t_sys_rand_seed.v:17:16: Unsupported: Seed on $urandom. Suggest use +verilator+seed+ runtime flag - 17 | valueb = $urandom(10); - | ^~~~~~~~ -%Error: Exiting due to diff --git a/test_regress/t/t_sys_rand_seed.pl b/test_regress/t/t_sys_rand_seed.pl index 391ec23fd..b46d46042 100755 --- a/test_regress/t/t_sys_rand_seed.pl +++ b/test_regress/t/t_sys_rand_seed.pl @@ -11,12 +11,11 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di scenarios(simulator => 1); compile( - fails => $Self->{vlt_all}, - expect_filename => $Self->{golden_filename}, ); execute( - ) if !$Self->{vlt_all}; + check_finished => 1, + ); ok(1); 1; diff --git a/test_regress/t/t_urandom.v b/test_regress/t/t_urandom.v index 3a894c6ae..f20529134 100644 --- a/test_regress/t/t_urandom.v +++ b/test_regress/t/t_urandom.v @@ -42,7 +42,6 @@ module t(/*AUTOARG*/); if (v1 != 0 && v1 != 1) $stop; end -`ifndef VERILATOR // Seed stability // Note UVM doesn't use $urandom seeding v1 = $urandom(1); @@ -50,7 +49,6 @@ module t(/*AUTOARG*/); if (v1 != v2) $stop; v2 = $urandom(1); if (v1 != v2) $stop; -`endif `ifdef PROC // Seed stability via process.srandom