From b74d33d1b8329ecda69ff6889479a097d8835664 Mon Sep 17 00:00:00 2001 From: Catherine Date: Fri, 19 Jan 2024 13:33:14 +0000 Subject: [PATCH] fmt: rename TIME to VLOG_TIME. The behavior of these format specifiers is highly specific to Verilog (`$time` and `$realtime` are only defined relative to `$timescale`) and may not fit other languages well, if at all. If they choose to use it, it is now clear what they are opting into. This commit also simplifies the CXXRTL code generation for these format specifiers. --- backends/cxxrtl/cxxrtl_backend.cc | 8 ++++---- backends/cxxrtl/runtime/cxxrtl/cxxrtl.h | 26 ++++++++++++++----------- kernel/fmt.cc | 26 ++++++++++++------------- kernel/fmt.h | 8 ++++---- tests/fmt/always_full_tb.cc | 2 +- 5 files changed, 37 insertions(+), 33 deletions(-) diff --git a/backends/cxxrtl/cxxrtl_backend.cc b/backends/cxxrtl/cxxrtl_backend.cc index ef718067f..620b530b0 100644 --- a/backends/cxxrtl/cxxrtl_backend.cc +++ b/backends/cxxrtl/cxxrtl_backend.cc @@ -1072,9 +1072,9 @@ struct CxxrtlWorker { dump_sigspec_rhs(cell->getPort(ID::EN)); f << " == value<1>{1u}) {\n"; inc_indent(); - f << indent << "auto formatter = [&](int64_t itime, double ftime) {\n"; + f << indent << "auto formatter = [&](struct performer *performer) {\n"; inc_indent(); - fmt.emit_cxxrtl(f, indent, [this](const RTLIL::SigSpec &sig) { dump_sigspec_rhs(sig); }); + fmt.emit_cxxrtl(f, indent, [this](const RTLIL::SigSpec &sig) { dump_sigspec_rhs(sig); }, "performer"); dec_indent(); f << indent << "};\n"; f << indent << "if (performer) {\n"; @@ -1082,11 +1082,11 @@ struct CxxrtlWorker { f << indent << "static const metadata_map attributes = "; dump_metadata_map(cell->attributes); f << ";\n"; - f << indent << "performer->on_print(formatter(performer->time(), performer->realtime()), attributes);\n"; + f << indent << "performer->on_print(formatter(performer), attributes);\n"; dec_indent(); f << indent << "} else {\n"; inc_indent(); - f << indent << print_output << " << formatter(0, 0.0);\n"; + f << indent << print_output << " << formatter(performer);\n"; dec_indent(); f << indent << "}\n"; dec_indent(); diff --git a/backends/cxxrtl/runtime/cxxrtl/cxxrtl.h b/backends/cxxrtl/runtime/cxxrtl/cxxrtl.h index cfdb66f5c..a8a011d15 100644 --- a/backends/cxxrtl/runtime/cxxrtl/cxxrtl.h +++ b/backends/cxxrtl/runtime/cxxrtl/cxxrtl.h @@ -948,10 +948,10 @@ typedef std::map metadata_map; // An object that can be passed to a `eval()` method in order to act on side effects. struct performer { // Called to evaluate a Verilog `$time` expression. - virtual int64_t time() const { return 0; } + virtual int64_t vlog_time() const { return 0; } // Called to evaluate a Verilog `$realtime` expression. - virtual double realtime() const { return time(); } + virtual double vlog_realtime() const { return vlog_time(); } // Called when a `$print` cell is triggered. virtual void on_print(const std::string &output, const metadata_map &attributes) { std::cout << output; } @@ -976,10 +976,10 @@ struct observer { // Default member initializers would make this a non-aggregate-type in C++11, so they are commented out. struct fmt_part { enum { - STRING = 0, - INTEGER = 1, + STRING = 0, + INTEGER = 1, CHARACTER = 2, - TIME = 3, + VLOG_TIME = 3, } type; // STRING type @@ -988,7 +988,7 @@ struct fmt_part { // INTEGER/CHARACTER types // + value val; - // INTEGER/CHARACTER/TIME types + // INTEGER/CHARACTER/VLOG_TIME types enum { RIGHT = 0, LEFT = 1, @@ -1001,16 +1001,16 @@ struct fmt_part { bool signed_; // = false; bool plus; // = false; - // TIME type + // VLOG_TIME type bool realtime; // = false; // + int64_t itime; // + double ftime; // Format the part as a string. // - // The values of `itime` and `ftime` are used for `$time` and `$realtime`, correspondingly. + // The values of `vlog_time` and `vlog_realtime` are used for Verilog `$time` and `$realtime`, correspondingly. template - std::string render(value val, int64_t itime, double ftime) + std::string render(value val, performer *performer = nullptr) { // We might want to replace some of these bit() calls with direct // chunk access if it turns out to be slow enough to matter. @@ -1077,8 +1077,12 @@ struct fmt_part { break; } - case TIME: { - buf = realtime ? std::to_string(ftime) : std::to_string(itime); + case VLOG_TIME: { + if (performer) { + buf = realtime ? std::to_string(performer->vlog_realtime()) : std::to_string(performer->vlog_time()); + } else { + buf = realtime ? std::to_string(0.0) : std::to_string(0); + } break; } } diff --git a/kernel/fmt.cc b/kernel/fmt.cc index 7d16b20c1..18eb7cb71 100644 --- a/kernel/fmt.cc +++ b/kernel/fmt.cc @@ -110,9 +110,9 @@ void Fmt::parse_rtlil(const RTLIL::Cell *cell) { } else if (fmt[i] == 'c') { part.type = FmtPart::CHARACTER; } else if (fmt[i] == 't') { - part.type = FmtPart::TIME; + part.type = FmtPart::VLOG_TIME; } else if (fmt[i] == 'r') { - part.type = FmtPart::TIME; + part.type = FmtPart::VLOG_TIME; part.realtime = true; } else { log_assert(false && "Unexpected character in format substitution"); @@ -172,7 +172,7 @@ void Fmt::emit_rtlil(RTLIL::Cell *cell) const { } break; - case FmtPart::TIME: + case FmtPart::VLOG_TIME: log_assert(part.sig.size() == 0); YS_FALLTHROUGH case FmtPart::CHARACTER: @@ -205,7 +205,7 @@ void Fmt::emit_rtlil(RTLIL::Cell *cell) const { fmt += part.signed_ ? 's' : 'u'; } else if (part.type == FmtPart::CHARACTER) { fmt += 'c'; - } else if (part.type == FmtPart::TIME) { + } else if (part.type == FmtPart::VLOG_TIME) { if (part.realtime) fmt += 'r'; else @@ -328,7 +328,7 @@ void Fmt::parse_verilog(const std::vector &args, bool sformat_lik case VerilogFmtArg::TIME: { FmtPart part = {}; - part.type = FmtPart::TIME; + part.type = FmtPart::VLOG_TIME; part.realtime = arg->realtime; part.padding = ' '; part.width = 20; @@ -419,7 +419,7 @@ void Fmt::parse_verilog(const std::vector &args, bool sformat_lik part.padding = ' '; } else if (fmt[i] == 't' || fmt[i] == 'T') { if (arg->type == VerilogFmtArg::TIME) { - part.type = FmtPart::TIME; + part.type = FmtPart::VLOG_TIME; part.realtime = arg->realtime; if (!has_width && !has_leading_zero) part.width = 20; @@ -541,7 +541,7 @@ std::vector Fmt::emit_verilog() const break; } - case FmtPart::TIME: { + case FmtPart::VLOG_TIME: { VerilogFmtArg arg; arg.type = VerilogFmtArg::TIME; if (part.realtime) @@ -592,7 +592,7 @@ std::string escape_cxx_string(const std::string &input) return output; } -void Fmt::emit_cxxrtl(std::ostream &os, std::string indent, std::function emit_sig) const +void Fmt::emit_cxxrtl(std::ostream &os, std::string indent, std::function emit_sig, const std::string &context) const { os << indent << "std::string buf;\n"; for (auto &part : parts) { @@ -602,7 +602,7 @@ void Fmt::emit_cxxrtl(std::ostream &os, std::string indent, std::function &args, bool sformat_like, int default_base, RTLIL::IdString task_name, RTLIL::IdString module_name); std::vector emit_verilog() const; - void emit_cxxrtl(std::ostream &os, std::string indent, std::function emit_sig) const; + void emit_cxxrtl(std::ostream &os, std::string indent, std::function emit_sig, const std::string &context) const; std::string render() const; diff --git a/tests/fmt/always_full_tb.cc b/tests/fmt/always_full_tb.cc index a29a8ae05..1dd80d0a2 100644 --- a/tests/fmt/always_full_tb.cc +++ b/tests/fmt/always_full_tb.cc @@ -3,7 +3,7 @@ int main() { struct : public performer { - int64_t time() const override { return 1; } + int64_t vlog_time() const override { return 1; } void on_print(const std::string &output, const cxxrtl::metadata_map &) override { std::cerr << output; } } performer;