From ea7818d31bb2533d4ecceb2ed1bcf4a22b850453 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 22 Dec 2023 00:15:54 +0000 Subject: [PATCH 1/9] Bump version --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e367846e2..00569d95c 100644 --- a/Makefile +++ b/Makefile @@ -141,7 +141,7 @@ LDLIBS += -lrt endif endif -YOSYS_VER := 0.36+42 +YOSYS_VER := 0.36+58 # Note: We arrange for .gitcommit to contain the (short) commit hash in # tarballs generated with git-archive(1) using .gitattributes. The git repo From fb72dc1a4085547bce2a4d1b9a69b99a9f1ab784 Mon Sep 17 00:00:00 2001 From: Claire Xenia Wolf Date: Fri, 29 Dec 2023 19:20:44 +0100 Subject: [PATCH 2/9] Add constexpr hashlib default constructors Signed-off-by: Claire Xenia Wolf --- kernel/hashlib.h | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/kernel/hashlib.h b/kernel/hashlib.h index 47aba71a8..25aa94b80 100644 --- a/kernel/hashlib.h +++ b/kernel/hashlib.h @@ -420,7 +420,7 @@ public: operator const_iterator() const { return const_iterator(ptr, index); } }; - dict() + constexpr dict() { } @@ -855,7 +855,7 @@ public: operator const_iterator() const { return const_iterator(ptr, index); } }; - pool() + constexpr pool() { } @@ -1062,6 +1062,10 @@ public: const K *operator->() const { return &container[index]; } }; + constexpr idict() + { + } + int operator()(const K &key) { int hash = database.do_hash(key); @@ -1132,6 +1136,10 @@ class mfp public: typedef typename idict::const_iterator const_iterator; + constexpr mfp() + { + } + int operator()(const K &key) const { int i = database(key); From df65634e07d283202bebfae2e2110724a4d8003f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 30 Dec 2023 00:15:15 +0000 Subject: [PATCH 3/9] Bump version --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 00569d95c..60d67d05a 100644 --- a/Makefile +++ b/Makefile @@ -141,7 +141,7 @@ LDLIBS += -lrt endif endif -YOSYS_VER := 0.36+58 +YOSYS_VER := 0.36+61 # Note: We arrange for .gitcommit to contain the (short) commit hash in # tarballs generated with git-archive(1) using .gitattributes. The git repo From 627fbc3477a4f11dd9824b4385ec6b776824ae5d Mon Sep 17 00:00:00 2001 From: Miodrag Milanovic Date: Tue, 2 Jan 2024 11:26:48 +0100 Subject: [PATCH 4/9] Fix Windows build by forcing initialization order, fixes #4068 --- techlibs/quicklogic/ql_bram_merge.cc | 6 +-- techlibs/quicklogic/ql_dsp_io_regs.cc | 9 ++-- techlibs/quicklogic/ql_dsp_simd.cc | 70 +++++++++++++-------------- 3 files changed, 43 insertions(+), 42 deletions(-) diff --git a/techlibs/quicklogic/ql_bram_merge.cc b/techlibs/quicklogic/ql_bram_merge.cc index 1098bc8f6..bed1ba572 100644 --- a/techlibs/quicklogic/ql_bram_merge.cc +++ b/techlibs/quicklogic/ql_bram_merge.cc @@ -31,9 +31,6 @@ PRIVATE_NAMESPACE_BEGIN struct QlBramMergeWorker { - const RTLIL::IdString split_cell_type = ID($__QLF_TDP36K); - const RTLIL::IdString merged_cell_type = ID($__QLF_TDP36K_MERGED); - // can be used to record parameter values that have to match on both sides typedef dict MergeableGroupKeyType; @@ -42,6 +39,8 @@ struct QlBramMergeWorker { QlBramMergeWorker(RTLIL::Module* module) : module(module) { + const RTLIL::IdString split_cell_type = ID($__QLF_TDP36K); + for (RTLIL::Cell* cell : module->selected_cells()) { if(cell->type != split_cell_type) continue; @@ -125,6 +124,7 @@ struct QlBramMergeWorker { void merge_brams(RTLIL::Cell* bram1, RTLIL::Cell* bram2) { + const RTLIL::IdString merged_cell_type = ID($__QLF_TDP36K_MERGED); // Create the new cell RTLIL::Cell* merged = module->addCell(NEW_ID, merged_cell_type); diff --git a/techlibs/quicklogic/ql_dsp_io_regs.cc b/techlibs/quicklogic/ql_dsp_io_regs.cc index 523c86e73..ecf163dbf 100644 --- a/techlibs/quicklogic/ql_dsp_io_regs.cc +++ b/techlibs/quicklogic/ql_dsp_io_regs.cc @@ -30,10 +30,6 @@ PRIVATE_NAMESPACE_BEGIN // ============================================================================ struct QlDspIORegs : public Pass { - const std::vector ports2del_mult = {ID(load_acc), ID(subtract), ID(acc_fir), ID(dly_b), - ID(saturate_enable), ID(shift_right), ID(round)}; - const std::vector ports2del_mult_acc = {ID(acc_fir), ID(dly_b)}; - SigMap sigmap; // .......................................... @@ -67,6 +63,11 @@ struct QlDspIORegs : public Pass { void ql_dsp_io_regs_pass(RTLIL::Module *module) { + static const std::vector ports2del_mult = {ID(load_acc), ID(subtract), ID(acc_fir), ID(dly_b), + ID(saturate_enable), ID(shift_right), ID(round)}; + static const std::vector ports2del_mult_acc = {ID(acc_fir), ID(dly_b)}; + + sigmap.set(module); for (auto cell : module->cells()) { diff --git a/techlibs/quicklogic/ql_dsp_simd.cc b/techlibs/quicklogic/ql_dsp_simd.cc index 153f3995f..9df979c33 100644 --- a/techlibs/quicklogic/ql_dsp_simd.cc +++ b/techlibs/quicklogic/ql_dsp_simd.cc @@ -60,43 +60,11 @@ struct QlDspSimdPass : public Pass { // .......................................... - // DSP control and config ports to consider and how to map them to ports - // of the target DSP cell - const std::vector> m_DspCfgPorts = { - std::make_pair(ID(clock_i), ID(clk)), - std::make_pair(ID(reset_i), ID(reset)), - std::make_pair(ID(feedback_i), ID(feedback)), - std::make_pair(ID(load_acc_i), ID(load_acc)), - std::make_pair(ID(unsigned_a_i), ID(unsigned_a)), - std::make_pair(ID(unsigned_b_i), ID(unsigned_b)), - std::make_pair(ID(subtract_i), ID(subtract)), - std::make_pair(ID(output_select_i), ID(output_select)), - std::make_pair(ID(saturate_enable_i), ID(saturate_enable)), - std::make_pair(ID(shift_right_i), ID(shift_right)), - std::make_pair(ID(round_i), ID(round)), - std::make_pair(ID(register_inputs_i), ID(register_inputs)) - }; - const int m_ModeBitsSize = 80; - // DSP data ports and how to map them to ports of the target DSP cell - const std::vector> m_DspDataPorts = { - std::make_pair(ID(a_i), ID(a)), - std::make_pair(ID(b_i), ID(b)), - std::make_pair(ID(acc_fir_i), ID(acc_fir)), - std::make_pair(ID(z_o), ID(z)), - std::make_pair(ID(dly_b_o), ID(dly_b)) - }; - // DSP parameters const std::vector m_DspParams = {"COEFF_3", "COEFF_2", "COEFF_1", "COEFF_0"}; - // Source DSP cell type (SISD) - const IdString m_SisdDspType = ID(dsp_t1_10x9x32); - - // Target DSP cell types for the SIMD mode - const IdString m_SimdDspType = ID(QL_DSP2); - /// Temporary SigBit to SigBit helper map. SigMap sigmap; @@ -106,6 +74,38 @@ struct QlDspSimdPass : public Pass { { log_header(a_Design, "Executing QL_DSP_SIMD pass.\n"); + // DSP control and config ports to consider and how to map them to ports + // of the target DSP cell + static const std::vector> m_DspCfgPorts = { + std::make_pair(ID(clock_i), ID(clk)), + std::make_pair(ID(reset_i), ID(reset)), + std::make_pair(ID(feedback_i), ID(feedback)), + std::make_pair(ID(load_acc_i), ID(load_acc)), + std::make_pair(ID(unsigned_a_i), ID(unsigned_a)), + std::make_pair(ID(unsigned_b_i), ID(unsigned_b)), + std::make_pair(ID(subtract_i), ID(subtract)), + std::make_pair(ID(output_select_i), ID(output_select)), + std::make_pair(ID(saturate_enable_i), ID(saturate_enable)), + std::make_pair(ID(shift_right_i), ID(shift_right)), + std::make_pair(ID(round_i), ID(round)), + std::make_pair(ID(register_inputs_i), ID(register_inputs)) + }; + + // DSP data ports and how to map them to ports of the target DSP cell + static const std::vector> m_DspDataPorts = { + std::make_pair(ID(a_i), ID(a)), + std::make_pair(ID(b_i), ID(b)), + std::make_pair(ID(acc_fir_i), ID(acc_fir)), + std::make_pair(ID(z_o), ID(z)), + std::make_pair(ID(dly_b_o), ID(dly_b)) + }; + + // Source DSP cell type (SISD) + static const IdString m_SisdDspType = ID(dsp_t1_10x9x32); + + // Target DSP cell types for the SIMD mode + static const IdString m_SimdDspType = ID(QL_DSP2); + // Parse args extra_args(a_Args, 1, a_Design); @@ -126,7 +126,7 @@ struct QlDspSimdPass : public Pass { continue; // Add to a group - const auto key = getDspConfig(cell); + const auto key = getDspConfig(cell, m_DspCfgPorts); groups[key].push_back(cell); } @@ -255,11 +255,11 @@ struct QlDspSimdPass : public Pass { } /// Given a DSP cell populates and returns a DspConfig struct for it. - DspConfig getDspConfig(RTLIL::Cell *a_Cell) + DspConfig getDspConfig(RTLIL::Cell *a_Cell, const std::vector> &dspCfgPorts) { DspConfig config; - for (const auto &it : m_DspCfgPorts) { + for (const auto &it : dspCfgPorts) { auto port = it.first; // Port unconnected From 1bbea13f80fa7e5d337dcf3affd28d1ab8664cae Mon Sep 17 00:00:00 2001 From: Dag Lem Date: Thu, 4 Jan 2024 17:22:07 +0100 Subject: [PATCH 5/9] Correct hierarchical path names for structs and unions --- frontends/ast/simplify.cc | 36 ++++++++++++++++++--------------- tests/svtypes/typedef_scopes.sv | 6 +++--- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc index 2a500b56b..dfa1ed6af 100644 --- a/frontends/ast/simplify.cc +++ b/frontends/ast/simplify.cc @@ -226,17 +226,6 @@ void AstNode::annotateTypedEnums(AstNode *template_node) } } -static bool name_has_dot(const std::string &name, std::string &struct_name) -{ - // check if plausible struct member name \sss.mmm - std::string::size_type pos; - if (name.substr(0, 1) == "\\" && (pos = name.find('.', 0)) != std::string::npos) { - struct_name = name.substr(0, pos); - return true; - } - return false; -} - static AstNode *make_range(int left, int right, bool is_signed = false) { // generate a pre-validated range node for a fixed signal range. @@ -2185,11 +2174,24 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin if (type == AST_IDENTIFIER && !basic_prep) { // check if a plausible struct member sss.mmmm - std::string sname; - if (name_has_dot(str, sname)) { - if (current_scope.count(str) > 0) { - auto item_node = current_scope[str]; - if (item_node->type == AST_STRUCT_ITEM || item_node->type == AST_STRUCT || item_node->type == AST_UNION) { + if (!str.empty() && str[0] == '\\' && current_scope.count(str)) { + auto item_node = current_scope[str]; + if (item_node->type == AST_STRUCT_ITEM || item_node->type == AST_STRUCT || item_node->type == AST_UNION) { + // Traverse any hierarchical path until the full name for the referenced struct/union is found. + std::string sname; + bool found_sname = false; + for (std::string::size_type pos = 0; (pos = str.find('.', pos)) != std::string::npos; pos++) { + sname = str.substr(0, pos); + if (current_scope.count(sname)) { + auto stype = current_scope[sname]->type; + if (stype == AST_WIRE || stype == AST_PARAMETER || stype == AST_LOCALPARAM) { + found_sname = true; + break; + } + } + } + + if (found_sname) { // structure member, rewrite this node to reference the packed struct wire auto range = make_struct_member_range(this, item_node); newNode = new AstNode(AST_IDENTIFIER, range); @@ -4681,6 +4683,8 @@ void AstNode::expand_genblock(const std::string &prefix) switch (child->type) { case AST_WIRE: case AST_MEMORY: + case AST_STRUCT: + case AST_UNION: case AST_PARAMETER: case AST_LOCALPARAM: case AST_FUNCTION: diff --git a/tests/svtypes/typedef_scopes.sv b/tests/svtypes/typedef_scopes.sv index 5ac9a4664..cd7b7953e 100644 --- a/tests/svtypes/typedef_scopes.sv +++ b/tests/svtypes/typedef_scopes.sv @@ -45,12 +45,12 @@ module top; localparam W = 10; typedef T U; typedef logic [W-1:0] V; - struct packed { + typedef struct packed { logic [W-1:0] x; // width 10 U y; // width 5 V z; // width 10 - } shadow; - // This currently only works as long as long as shadow is not typedef'ed + } shadow_t; + shadow_t shadow; always @(*) assert($bits(shadow.x) == 10); always @(*) assert($bits(shadow.y) == 5); always @(*) assert($bits(shadow.z) == 10); From a94fafa8fe70650a1e8ded35e352e9fb48b27639 Mon Sep 17 00:00:00 2001 From: Catherine Date: Fri, 8 Dec 2023 18:21:19 +0000 Subject: [PATCH 6/9] cxxrtl: add a representation of simulation timestamps. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While the VCD format separates the timescale and the timestep (likely to allow representing the timestep with a small integer type), time in CXXRTL is represented using a uniform 96-bit number, which allows for a ±100 year range at femtosecond resolution. The implementation uses `value<96>`, which provides fast arithmetic and comparison operations, as well as conversion to/from a more common representation of integer seconds plus femtoseconds. --- backends/cxxrtl/runtime/cxxrtl/cxxrtl_time.h | 229 +++++++++++++++++++ 1 file changed, 229 insertions(+) create mode 100644 backends/cxxrtl/runtime/cxxrtl/cxxrtl_time.h diff --git a/backends/cxxrtl/runtime/cxxrtl/cxxrtl_time.h b/backends/cxxrtl/runtime/cxxrtl/cxxrtl_time.h new file mode 100644 index 000000000..51f59321e --- /dev/null +++ b/backends/cxxrtl/runtime/cxxrtl/cxxrtl_time.h @@ -0,0 +1,229 @@ +/* + * yosys -- Yosys Open SYnthesis Suite + * + * Copyright (C) 2023 Catherine + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + * + */ + +#ifndef CXXRTL_TIME_H +#define CXXRTL_TIME_H + +#include +#include + +#include + +namespace cxxrtl { + +// A timestamp or a difference in time, stored as a 96-bit number of femtoseconds (10e-15 s). The dynamic range and +// resolution of this format can represent any VCD timestamp within 136 years, without the need for a timescale. +class time { +public: + static constexpr size_t bits = 96; // 3 chunks + +private: + static constexpr value resolution = value { + chunk_t(1000000000000000ull & 0xffffffffull), chunk_t(1000000000000000ull >> 32), 0u + }; + static constexpr size_t resolution_digits = 15; + + // Signed number of femtoseconds from the beginning of time. + value raw; + +public: + constexpr time() {} + + explicit constexpr time(const value &raw) : raw(raw) {} + explicit operator const value &() const { return raw; } + + static constexpr time maximum() { + return time(value { 0xffffffffu, 0xffffffffu, 0x7fffffffu }); + } + + time(int32_t secs, int64_t femtos) { + value<32> secs_val; + secs_val.set((uint32_t)secs); + value<64> femtos_val; + femtos_val.set((uint64_t)femtos); + raw = secs_val.sext().mul(resolution).add(femtos_val.sext()); + } + + bool is_zero() const { + return raw.is_zero(); + } + + // Extracts the sign of the value. + bool is_negative() const { + return raw.is_neg(); + } + + // Extracts the absolute number of whole seconds. Negative if the value is negative. + int32_t secs() const { + return raw.sdivmod(resolution).first.trunc<32>().get(); + } + + // Extracts the absolute number of femtoseconds in the fractional second. Negative if the value is negative. + int64_t femtos() const { + return raw.sdivmod(resolution).second.trunc<64>().get(); + } + + bool operator==(const time &other) const { + return raw == other.raw; + } + + bool operator!=(const time &other) const { + return raw != other.raw; + } + + bool operator>(const time &other) const { + return other.raw.scmp(raw); + } + + bool operator>=(const time &other) const { + return !raw.scmp(other.raw); + } + + bool operator<(const time &other) const { + return raw.scmp(other.raw); + } + + bool operator<=(const time &other) const { + return !other.raw.scmp(raw); + } + + time operator+(const time &other) const { + return time(raw.add(other.raw)); + } + + time &operator+=(const time &other) { + *this = *this + other; + return *this; + } + + time operator-() const { + return time(raw.neg()); + } + + time operator-(const time &other) const { + return *this + (-other); + } + + time &operator-=(const time &other) { + *this = *this - other; + return *this; + } + + operator std::string() const { + char buf[38]; // len(f"-{2**64}.{10**15-1}") + 1 == 38 + int32_t secs = this->secs(); + int64_t femtos = this->femtos(); + snprintf(buf, sizeof(buf), "%s%" PRIi32 ".%015" PRIi64, + is_negative() ? "-" : "", secs >= 0 ? secs : -secs, femtos >= 0 ? femtos : -femtos); + return buf; + } + +#if __cplusplus >= 201603L + [[nodiscard("ignoring parse errors")]] +#endif + bool parse(const std::string &str) { + enum { + parse_sign_opt, + parse_integral, + parse_fractional, + } state = parse_sign_opt; + bool negative = false; + int32_t integral = 0; + int64_t fractional = 0; + size_t frac_digits = 0; + for (auto chr : str) { + switch (state) { + case parse_sign_opt: + state = parse_integral; + if (chr == '+' || chr == '-') { + negative = (chr == '-'); + break; + } + /* fallthrough */ + case parse_integral: + if (chr >= '0' && chr <= '9') { + integral *= 10; + integral += chr - '0'; + } else if (chr == '.') { + state = parse_fractional; + } else { + return false; + } + break; + case parse_fractional: + if (chr >= '0' && chr <= '9' && frac_digits < resolution_digits) { + fractional *= 10; + fractional += chr - '0'; + frac_digits++; + } else { + return false; + } + break; + } + } + if (frac_digits == 0) + return false; + while (frac_digits++ < resolution_digits) + fractional *= 10; + *this = negative ? -time { integral, fractional} : time { integral, fractional }; + return true; + } +}; + +// Out-of-line definition required until C++17. +constexpr value time::resolution; + +std::ostream &operator<<(std::ostream &os, const time &val) { + os << (std::string)val; + return os; +} + +// These literals are (confusingly) compatible with the ones from `std::chrono`: the `std::chrono` literals do not +// have an underscore (e.g. 1ms) and the `cxxrtl::time` literals do (e.g. 1_ms). This syntactic difference is +// a requirement of the C++ standard. Despite being compatible the literals should not be mixed in the same namespace. +namespace time_literals { + +time operator""_s(unsigned long long seconds) { + return time { (int32_t)seconds, 0 }; +} + +time operator""_ms(unsigned long long milliseconds) { + return time { 0, (int64_t)milliseconds * 1000000000000 }; +} + +time operator""_us(unsigned long long microseconds) { + return time { 0, (int64_t)microseconds * 1000000000 }; +} + +time operator""_ns(unsigned long long nanoseconds) { + return time { 0, (int64_t)nanoseconds * 1000000 }; +} + +time operator""_ps(unsigned long long picoseconds) { + return time { 0, (int64_t)picoseconds * 1000 }; +} + +time operator""_fs(unsigned long long femtoseconds) { + return time { 0, (int64_t)femtoseconds }; +} + +}; + +}; + +#endif From 3e358d9bfa9289fefbc68e83ab40c80f4b123641 Mon Sep 17 00:00:00 2001 From: Catherine Date: Wed, 25 Oct 2023 10:51:39 +0000 Subject: [PATCH 7/9] cxxrtl: add a way to observe state changes during the commit step. The commit observer is a structure containing a callback that is invoked whenever the `commit()` method changes a wire or a memory. This allows code external to the compiled netlist to react to changes in the design state in a very efficient way. One example of how this feature can be used is an efficient implementation of record/replay. Note that the VCD writer does not benefit from this feature because it must be able to react to changes in any debug items and not just those that contain design state. --- backends/cxxrtl/cxxrtl_backend.cc | 42 ++++++++++++++++--------- backends/cxxrtl/runtime/cxxrtl/cxxrtl.h | 40 +++++++++++++++++++++-- 2 files changed, 65 insertions(+), 17 deletions(-) diff --git a/backends/cxxrtl/cxxrtl_backend.cc b/backends/cxxrtl/cxxrtl_backend.cc index 1ab865a27..2c35a1943 100644 --- a/backends/cxxrtl/cxxrtl_backend.cc +++ b/backends/cxxrtl/cxxrtl_backend.cc @@ -2115,19 +2115,19 @@ struct CxxrtlWorker { if (wire_type.type == WireType::MEMBER && edge_wires[wire]) f << indent << "prev_" << mangle(wire) << " = " << mangle(wire) << ";\n"; if (wire_type.is_buffered()) - f << indent << "if (" << mangle(wire) << ".commit()) changed = true;\n"; + f << indent << "if (" << mangle(wire) << ".commit(observer)) changed = true;\n"; } if (!module->get_bool_attribute(ID(cxxrtl_blackbox))) { for (auto &mem : mod_memories[module]) { if (!writable_memories.count({module, mem.memid})) continue; - f << indent << "if (" << mangle(&mem) << ".commit()) changed = true;\n"; + f << indent << "if (" << mangle(&mem) << ".commit(observer)) changed = true;\n"; } for (auto cell : module->cells()) { if (is_internal_cell(cell->type)) continue; const char *access = is_cxxrtl_blackbox_cell(cell) ? "->" : "."; - f << indent << "if (" << mangle(cell) << access << "commit()) changed = true;\n"; + f << indent << "if (" << mangle(cell) << access << "commit(observer)) changed = true;\n"; } } f << indent << "return changed;\n"; @@ -2146,7 +2146,7 @@ struct CxxrtlWorker { if (!metadata_item.first.isPublic()) continue; if (metadata_item.second.size() > 64 && (metadata_item.second.flags & RTLIL::CONST_FLAG_STRING) == 0) { - f << indent << "/* attribute " << metadata_item.first.str().substr(1) << " is over 64 bits wide */"; + f << indent << "/* attribute " << metadata_item.first.str().substr(1) << " is over 64 bits wide */\n"; continue; } f << indent << "{ " << escape_cxx_string(metadata_item.first.str().substr(1)) << ", "; @@ -2371,16 +2371,22 @@ struct CxxrtlWorker { dump_eval_method(module); f << indent << "}\n"; f << "\n"; - f << indent << "bool commit() override {\n"; + f << indent << "template\n"; + f << indent << "bool commit(ObserverT &observer) {\n"; dump_commit_method(module); f << indent << "}\n"; f << "\n"; + f << indent << "bool commit() override {\n"; + f << indent << indent << "null_observer observer;\n"; + f << indent << indent << "return commit<>(observer);\n"; + f << indent << "}\n"; if (debug_info) { + f << "\n"; f << indent << "void debug_info(debug_items &items, std::string path = \"\") override {\n"; dump_debug_info_method(module); f << indent << "}\n"; - f << "\n"; } + f << "\n"; f << indent << "static std::unique_ptr<" << mangle(module); f << template_params(module, /*is_decl=*/false) << "> "; f << "create(std::string name, metadata_map parameters, metadata_map attributes);\n"; @@ -2457,8 +2463,18 @@ struct CxxrtlWorker { f << indent << "};\n"; f << "\n"; f << indent << "void reset() override;\n"; + f << "\n"; f << indent << "bool eval() override;\n"; - f << indent << "bool commit() override;\n"; + f << "\n"; + f << indent << "template\n"; + f << indent << "bool commit(ObserverT &observer) {\n"; + dump_commit_method(module); + f << indent << "}\n"; + f << "\n"; + f << indent << "bool commit() override {\n"; + f << indent << indent << "null_observer observer;\n"; + f << indent << indent << "return commit<>(observer);\n"; + f << indent << "}\n"; if (debug_info) { if (debug_eval) { f << "\n"; @@ -2490,24 +2506,20 @@ struct CxxrtlWorker { f << indent << "bool " << mangle(module) << "::eval() {\n"; dump_eval_method(module); f << indent << "}\n"; - f << "\n"; - f << indent << "bool " << mangle(module) << "::commit() {\n"; - dump_commit_method(module); - f << indent << "}\n"; - f << "\n"; if (debug_info) { if (debug_eval) { + f << "\n"; f << indent << "void " << mangle(module) << "::debug_eval() {\n"; dump_debug_eval_method(module); f << indent << "}\n"; - f << "\n"; } + f << "\n"; f << indent << "CXXRTL_EXTREMELY_COLD\n"; f << indent << "void " << mangle(module) << "::debug_info(debug_items &items, std::string path) {\n"; dump_debug_info_method(module); f << indent << "}\n"; - f << "\n"; } + f << "\n"; } void dump_design(RTLIL::Design *design) @@ -3267,6 +3279,8 @@ struct CxxrtlBackend : public Backend { log(" wire<8> p_o_data;\n"); log("\n"); log(" bool eval() override;\n"); + log(" template\n"); + log(" bool commit(ObserverT &observer);\n"); log(" bool commit() override;\n"); log("\n"); log(" static std::unique_ptr\n"); diff --git a/backends/cxxrtl/runtime/cxxrtl/cxxrtl.h b/backends/cxxrtl/runtime/cxxrtl/cxxrtl.h index 78dbf3707..53fb3dbc0 100644 --- a/backends/cxxrtl/runtime/cxxrtl/cxxrtl.h +++ b/backends/cxxrtl/runtime/cxxrtl/cxxrtl.h @@ -841,6 +841,29 @@ std::ostream &operator<<(std::ostream &os, const value_formatted &vf) return os; } +// An object that can be passed to a `commit()` method in order to produce a replay log of every +// state change in the simulation. +struct observer { + // Called when a `commit()` method for a wire is about to update the `chunks` chunks at `base` + // with `chunks` chunks at `value` that have a different bit pattern. It is guaranteed that + // `chunks` is equal to the wire chunk count and `base` points to the first chunk. + virtual void on_commit(size_t chunks, const chunk_t *base, const chunk_t *value) = 0; + + // Called when a `commit()` method for a memory is about to update the `chunks` chunks at + // `&base[chunks * index]` with `chunks` chunks at `value` that have a different bit pattern. + // It is guaranteed that `chunks` covers is equal to the memory element chunk count and `base` + // points to the first chunk of the first element of the memory. + virtual void on_commit(size_t chunks, const chunk_t *base, const chunk_t *value, size_t index) = 0; +}; + +// The `null_observer` class has the same interface as `observer`, but has no invocation overhead, +// since its methods are final and have no implementation. This allows the observer feature to be +// zero-cost when not in use. +struct null_observer final: observer { + void on_commit(size_t chunks, const chunk_t *base, const chunk_t *value) override {} + void on_commit(size_t chunks, const chunk_t *base, const chunk_t *value, size_t index) override {} +}; + template struct wire { static constexpr size_t bits = Bits; @@ -875,8 +898,14 @@ struct wire { next.template set(other); } - bool commit() { + // This method intentionally takes a mandatory argument (to make it more difficult to misuse in + // black box implementations, leading to missed observer events). It is generic over its argument + // to make sure the `on_commit` call is devirtualized. This is somewhat awkward but lets us keep + // a single implementation for both this method and the one in `memory`. + template + bool commit(ObserverT &observer) { if (curr != next) { + observer.on_commit(curr.chunks, curr.data, next.data); curr = next; return true; } @@ -950,12 +979,17 @@ struct memory { write { index, val, mask, priority }); } - bool commit() { + // See the note for `wire::commit()`. + template + bool commit(ObserverT &observer) { bool changed = false; for (const write &entry : write_queue) { value elem = data[entry.index]; elem = elem.update(entry.val, entry.mask); - changed |= (data[entry.index] != elem); + if (data[entry.index] != elem) { + observer.on_commit(value::chunks, data[0].data, elem.data, entry.index); + changed |= true; + } data[entry.index] = elem; } write_queue.clear(); From f9dc1a2184720748c36042792888f6e82867f285 Mon Sep 17 00:00:00 2001 From: Catherine Date: Fri, 5 Jan 2024 20:18:36 +0000 Subject: [PATCH 8/9] cxxrtl: fix comment wording. NFC --- backends/cxxrtl/runtime/cxxrtl/cxxrtl.h | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/backends/cxxrtl/runtime/cxxrtl/cxxrtl.h b/backends/cxxrtl/runtime/cxxrtl/cxxrtl.h index 53fb3dbc0..183fbb2c7 100644 --- a/backends/cxxrtl/runtime/cxxrtl/cxxrtl.h +++ b/backends/cxxrtl/runtime/cxxrtl/cxxrtl.h @@ -841,24 +841,22 @@ std::ostream &operator<<(std::ostream &os, const value_formatted &vf) return os; } -// An object that can be passed to a `commit()` method in order to produce a replay log of every -// state change in the simulation. +// An object that can be passed to a `commit()` method in order to produce a replay log of every state change in +// the simulation. struct observer { - // Called when a `commit()` method for a wire is about to update the `chunks` chunks at `base` - // with `chunks` chunks at `value` that have a different bit pattern. It is guaranteed that - // `chunks` is equal to the wire chunk count and `base` points to the first chunk. + // Called when the `commit()` method for a wire is about to update the `chunks` chunks at `base` with `chunks` chunks + // at `value` that have a different bit pattern. It is guaranteed that `chunks` is equal to the wire chunk count and + // `base` points to the first chunk. virtual void on_commit(size_t chunks, const chunk_t *base, const chunk_t *value) = 0; - // Called when a `commit()` method for a memory is about to update the `chunks` chunks at - // `&base[chunks * index]` with `chunks` chunks at `value` that have a different bit pattern. - // It is guaranteed that `chunks` covers is equal to the memory element chunk count and `base` - // points to the first chunk of the first element of the memory. + // Called when the `commit()` method for a memory is about to update the `chunks` chunks at `&base[chunks * index]` + // with `chunks` chunks at `value` that have a different bit pattern. It is guaranteed that `chunks` is equal to + // the memory element chunk count and `base` points to the first chunk of the first element of the memory. virtual void on_commit(size_t chunks, const chunk_t *base, const chunk_t *value, size_t index) = 0; }; -// The `null_observer` class has the same interface as `observer`, but has no invocation overhead, -// since its methods are final and have no implementation. This allows the observer feature to be -// zero-cost when not in use. +// The `null_observer` class has the same interface as `observer`, but has no invocation overhead, since its methods +// are final and have no implementation. This allows the observer feature to be zero-cost when not in use. struct null_observer final: observer { void on_commit(size_t chunks, const chunk_t *base, const chunk_t *value) override {} void on_commit(size_t chunks, const chunk_t *base, const chunk_t *value, size_t index) override {} From 30b795601c514cad22952a199d4cae2c735be595 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 6 Jan 2024 00:16:22 +0000 Subject: [PATCH 9/9] Bump version --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 60d67d05a..3da89690f 100644 --- a/Makefile +++ b/Makefile @@ -141,7 +141,7 @@ LDLIBS += -lrt endif endif -YOSYS_VER := 0.36+61 +YOSYS_VER := 0.36+67 # Note: We arrange for .gitcommit to contain the (short) commit hash in # tarballs generated with git-archive(1) using .gitattributes. The git repo