From 58e89cd36879eb93f36a99ab2ee80cca101d0ec8 Mon Sep 17 00:00:00 2001 From: whitequark Date: Wed, 15 Apr 2020 19:01:17 +0000 Subject: [PATCH 1/3] cxxrtl: remove inaccurate comment. NFC. --- backends/cxxrtl/cxxrtl.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/backends/cxxrtl/cxxrtl.cc b/backends/cxxrtl/cxxrtl.cc index d1a855bf0..d6b901aa0 100644 --- a/backends/cxxrtl/cxxrtl.cc +++ b/backends/cxxrtl/cxxrtl.cc @@ -1425,8 +1425,6 @@ struct CxxrtlWorker { if (cell->getPort(ID(CLK)).is_wire()) register_edge_signal(sigmap, cell->getPort(ID(CLK)), cell->parameters[ID(CLK_POLARITY)].as_bool() ? RTLIL::STp : RTLIL::STn); - // The $adff and $dffsr cells are level-sensitive, not edge-sensitive (in spite of the fact that they - // are inferred from an edge-sensitive Verilog process) and do not correspond to an edge-type sync rule. } // Similar for memory port cells. if (cell->type.in(ID($memrd), ID($memwr))) { From 9043632dcc9b4ab198e03ffbdb8ba232b047ef28 Mon Sep 17 00:00:00 2001 From: whitequark Date: Thu, 16 Apr 2020 16:30:43 +0000 Subject: [PATCH 2/3] cxxrtl: fix misleading example, caution about race conditions. Fixes #1944. --- backends/cxxrtl/cxxrtl.cc | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/backends/cxxrtl/cxxrtl.cc b/backends/cxxrtl/cxxrtl.cc index d6b901aa0..8f7f9d7a3 100644 --- a/backends/cxxrtl/cxxrtl.cc +++ b/backends/cxxrtl/cxxrtl.cc @@ -1640,21 +1640,30 @@ struct CxxrtlBackend : public Backend { log("\n"); log(" write_cxxrtl [options] [filename]\n"); log("\n"); - log("Write C++ code for simulating the design. The generated code requires a driver;\n"); - log("the following simple driver is provided as an example:\n"); + log("Write C++ code for simulating the design. The generated code requires a driver\n"); + log("that instantiates the design, toggles its clock, and interacts with its ports.\n"); + log("\n"); + log("The following driver may be used as an example for a design with a single clock\n"); + log("driving rising edge triggered flip-flops:\n"); log("\n"); log(" #include \"top.cc\"\n"); log("\n"); log(" int main() {\n"); log(" cxxrtl_design::p_top top;\n"); + log(" top.step();\n"); log(" while (1) {\n"); - log(" top.p_clk.next = value<1> {1u};\n"); - log(" top.step();\n"); + log(" /* user logic */\n"); log(" top.p_clk.next = value<1> {0u};\n"); log(" top.step();\n"); + log(" top.p_clk.next = value<1> {1u};\n"); + log(" top.step();\n"); log(" }\n"); log(" }\n"); log("\n"); + log("Note that CXXRTL simulations, just like the hardware they are simulating, are\n"); + log("subject to race conditions. If, in then example above, the user logic would run\n"); + log("simultaneously with the rising edge of the clock, the design would malfunction.\n"); + log("\n"); log("The following options are supported by this backend:\n"); log("\n"); log(" -header\n"); From 06c0338f2c19ca50675f3928de7fa19b05d304c4 Mon Sep 17 00:00:00 2001 From: whitequark Date: Thu, 16 Apr 2020 16:45:02 +0000 Subject: [PATCH 3/3] cxxrtl: make ROMs writable, document memory::operator[]. There is no practical benefit from using `const memory` for ROMs; it uses an std::vector internally, which prevents contemporary compilers from constant-propagating ROM contents. (It is not clear whether they are permitted to do so.) However, there is a major benefit from using non-const `memory` for ROMs, which is the ability to dynamically fill the ROM for each individual simulation. --- backends/cxxrtl/cxxrtl.cc | 3 +-- backends/cxxrtl/cxxrtl.h | 7 +++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/backends/cxxrtl/cxxrtl.cc b/backends/cxxrtl/cxxrtl.cc index 8f7f9d7a3..e4fa430f3 100644 --- a/backends/cxxrtl/cxxrtl.cc +++ b/backends/cxxrtl/cxxrtl.cc @@ -1166,8 +1166,7 @@ struct CxxrtlWorker { }); dump_attrs(memory); - f << indent << (writable_memories[memory] ? "" : "const ") - << "memory<" << memory->width << "> " << mangle(memory) + f << indent << "memory<" << memory->width << "> " << mangle(memory) << " { " << memory->size << "u"; if (init_cells.empty()) { f << " };\n"; diff --git a/backends/cxxrtl/cxxrtl.h b/backends/cxxrtl/cxxrtl.h index 593c31c28..fd390db79 100644 --- a/backends/cxxrtl/cxxrtl.h +++ b/backends/cxxrtl/cxxrtl.h @@ -604,12 +604,15 @@ struct memory { auto _ = {std::move(std::begin(init.data), std::end(init.data), data.begin() + init.offset)...}; } - value &operator [](size_t index) { + // An operator for direct memory reads. May be used at any time during the simulation. + const value &operator [](size_t index) const { assert(index < data.size()); return data[index]; } - const value &operator [](size_t index) const { + // An operator for direct memory writes. May only be used before the simulation is started. If used + // after the simulation is started, the design may malfunction. + value &operator [](size_t index) { assert(index < data.size()); return data[index]; }