From 0e92cc1afdb77ae6db05a3147ae4192ec683457a Mon Sep 17 00:00:00 2001 From: Jannis Harder Date: Fri, 22 Sep 2023 17:52:25 +0200 Subject: [PATCH 1/3] sim: Include $display output in JSON summary This allows tools like SBY to capture the $display output independent from anything else sim might log. Additionally it provides source and hierarchy locations for everything printed. --- passes/sat/sim.cc | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/passes/sat/sim.cc b/passes/sat/sim.cc index 1e6645303b5..c1a4a7a7f2b 100644 --- a/passes/sat/sim.cc +++ b/passes/sat/sim.cc @@ -88,6 +88,17 @@ struct TriggeredAssertion { { } }; +struct DisplayOutput { + int step; + SimInstance *instance; + Cell *cell; + std::string output; + + DisplayOutput(int step, SimInstance *instance, Cell *cell, std::string output) : + step(step), instance(instance), cell(cell), output(output) + { } +}; + struct SimShared { bool debug = false; @@ -110,6 +121,7 @@ struct SimShared int next_output_id = 0; int step = 0; std::vector triggered_assertions; + std::vector display_output; bool serious_asserts = false; }; @@ -841,6 +853,7 @@ struct SimInstance std::string rendered = print.fmt.render(); log("%s", rendered.c_str()); + shared->display_output.emplace_back(shared->step, this, cell, rendered); } update_print: @@ -2020,6 +2033,20 @@ struct SimWorker : SimShared json.end_object(); } json.end_array(); + json.name("display_output"); + json.begin_array(); + for (auto &output : display_output) { + json.begin_object(); + json.entry("step", output.step); + json.entry("path", output.instance->witness_full_path(output.cell)); + auto src = output.cell->get_string_attribute(ID::src); + if (!src.empty()) { + json.entry("src", src); + } + json.entry("output", output.output); + json.end_object(); + } + json.end_array(); json.end_object(); } From 8c89b46cb1e6f3b9588a055cf17c21acb34e0bdc Mon Sep 17 00:00:00 2001 From: Jannis Harder Date: Fri, 22 Sep 2023 17:56:34 +0200 Subject: [PATCH 2/3] fmt: Allow non-constant $display calls in initial blocks These are useful for formal verification with SBY where they can be used to display solver chosen `rand const reg` signals and signals derived from those. The previous error message for non-constant initial $display statements is downgraded to a log message. Constant initial $display statements will be shown both during elaboration and become part of the RTLIL so that the `sim` output is complete. --- frontends/ast/ast.h | 2 +- frontends/ast/simplify.cc | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/frontends/ast/ast.h b/frontends/ast/ast.h index f789e930b3e..0a5c51f0b64 100644 --- a/frontends/ast/ast.h +++ b/frontends/ast/ast.h @@ -287,7 +287,7 @@ namespace AST bool is_simple_const_expr(); // helper for parsing format strings - Fmt processFormat(int stage, bool sformat_like, int default_base = 10, size_t first_arg_at = 0); + Fmt processFormat(int stage, bool sformat_like, int default_base = 10, size_t first_arg_at = 0, bool required = true); bool is_recursive_function() const; std::pair get_tern_choice(); diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc index c4a30302712..b90bebd9b7d 100644 --- a/frontends/ast/simplify.cc +++ b/frontends/ast/simplify.cc @@ -132,7 +132,7 @@ void AstNode::fixup_hierarchy_flags(bool force_descend) // Process a format string and arguments for $display, $write, $sprintf, etc -Fmt AstNode::processFormat(int stage, bool sformat_like, int default_base, size_t first_arg_at) { +Fmt AstNode::processFormat(int stage, bool sformat_like, int default_base, size_t first_arg_at, bool required) { std::vector args; for (size_t index = first_arg_at; index < children.size(); index++) { AstNode *node_arg = children[index]; @@ -156,6 +156,9 @@ Fmt AstNode::processFormat(int stage, bool sformat_like, int default_base, size_ arg.type = VerilogFmtArg::INTEGER; arg.sig = node_arg->bitsAsConst(); arg.signed_ = node_arg->is_signed; + } else if (!required) { + log_file_info(filename, location.first_line, "Skipping system task `%s' with non-constant argument at position %zu.\n", str.c_str(), index + 1); + return Fmt(); } else { log_file_error(filename, location.first_line, "Failed to evaluate system task `%s' with non-constant argument at position %zu.\n", str.c_str(), index + 1); } @@ -1063,10 +1066,13 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin default_base = 16; // when $display()/$write() functions are used in an initial block, print them during synthesis - Fmt fmt = processFormat(stage, /*sformat_like=*/false, default_base); + Fmt fmt = processFormat(stage, /*sformat_like=*/false, default_base, /*first_arg_at=*/0, /*required=*/false); if (str.substr(0, 8) == "$display") fmt.append_string("\n"); log("%s", fmt.render().c_str()); + for (auto node : children) + while (node->simplify(true, stage, -1, false)) {} + return false; } else { // when $display()/$write() functions are used in an always block, simplify the expressions and // convert them to a special cell later in genrtlil From 1dbd3cc0173fbe4d2d75909e3947d6652d6da13c Mon Sep 17 00:00:00 2001 From: Jannis Harder Date: Fri, 22 Sep 2023 18:02:10 +0200 Subject: [PATCH 3/3] fmt, sim: Hierarchical $display %m output and dynamic %t output This keeps %m as a format part of the RTLIL format syntax, as %m is supposed to be the hierarchical name of the current instance, which is not statically known for an RTLIL module. To allow sim to output the correct hierarchical name and the correct simulation time, the `Fmt::render` function is extended to take both as argument. When `render` is called in a constant context, this keeps essentially the previous behavior and uses the module name as fallback, as the hierarchy is not known at that point. --- frontends/ast/simplify.cc | 4 +-- kernel/fmt.cc | 69 +++++++++++++++++++++++++++------------ kernel/fmt.h | 10 +++--- passes/sat/sim.cc | 28 ++++++++-------- 4 files changed, 72 insertions(+), 39 deletions(-) diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc index b90bebd9b7d..1eaa3bb76ca 100644 --- a/frontends/ast/simplify.cc +++ b/frontends/ast/simplify.cc @@ -1069,7 +1069,7 @@ bool AstNode::simplify(bool const_fold, int stage, int width_hint, bool sign_hin Fmt fmt = processFormat(stage, /*sformat_like=*/false, default_base, /*first_arg_at=*/0, /*required=*/false); if (str.substr(0, 8) == "$display") fmt.append_string("\n"); - log("%s", fmt.render().c_str()); + log("%s", fmt.render(unescape_id(current_module->name).c_str()).c_str()); for (auto node : children) while (node->simplify(true, stage, -1, false)) {} return false; @@ -3744,7 +3744,7 @@ skip_dynamic_range_lvalue_expansion:; if (str == "\\$sformatf") { Fmt fmt = processFormat(stage, /*sformat_like=*/true); - newNode = AstNode::mkconst_str(fmt.render()); + newNode = AstNode::mkconst_str(fmt.render(unescape_id(current_module->name))); goto apply_newNode; } diff --git a/kernel/fmt.cc b/kernel/fmt.cc index 965e58ebce4..55dd2a76c6b 100644 --- a/kernel/fmt.cc +++ b/kernel/fmt.cc @@ -114,6 +114,8 @@ void Fmt::parse_rtlil(const RTLIL::Cell *cell) { } else if (fmt[i] == 'r') { part.type = FmtPart::TIME; part.realtime = true; + } else if (fmt[i] == 'm') { + part.type = FmtPart::HIERNAME; } else { log_assert(false && "Unexpected character in format substitution"); } @@ -173,6 +175,7 @@ void Fmt::emit_rtlil(RTLIL::Cell *cell) const { break; case FmtPart::TIME: + case FmtPart::HIERNAME: log_assert(part.sig.size() == 0); YS_FALLTHROUGH case FmtPart::CHARACTER: @@ -210,6 +213,8 @@ void Fmt::emit_rtlil(RTLIL::Cell *cell) const { fmt += 'r'; else fmt += 't'; + } else if (part.type == FmtPart::HIERNAME) { + fmt += 'm'; } else log_abort(); fmt += '}'; break; @@ -340,9 +345,6 @@ void Fmt::parse_verilog(const std::vector &args, bool sformat_lik } else if (fmt.substr(i, 2) == "%l" || fmt.substr(i, 2) == "%L") { i++; part.str += module_name.str(); - } else if (fmt.substr(i, 2) == "%m" || fmt.substr(i, 2) == "%M") { - i++; - part.str += module_name.str(); } else { if (!part.str.empty()) { part.type = FmtPart::STRING; @@ -353,12 +355,6 @@ void Fmt::parse_verilog(const std::vector &args, bool sformat_lik log_file_error(fmtarg->filename, fmtarg->first_line, "System task `%s' called with incomplete format specifier in argument %zu.\n", task_name.c_str(), fmtarg - args.begin() + 1); } - if (++arg == args.end()) { - log_file_error(fmtarg->filename, fmtarg->first_line, "System task `%s' called with fewer arguments than the format specifiers in argument %zu require.\n", task_name.c_str(), fmtarg - args.begin() + 1); - } - part.sig = arg->sig; - part.signed_ = arg->signed_; - for (; i < fmt.size(); i++) { if (fmt[i] == '-') { // left justify; not in IEEE 1800-2017 or verilator but iverilog has it @@ -408,14 +404,11 @@ void Fmt::parse_verilog(const std::vector &args, bool sformat_lik // %10s and %010s not fully defined in IEEE 1800-2017 and do the same thing in iverilog part.padding = ' '; } else if (fmt[i] == 't' || fmt[i] == 'T') { - if (arg->type == VerilogFmtArg::TIME) { - part.type = FmtPart::TIME; - part.realtime = arg->realtime; - if (!has_width && !has_leading_zero) - part.width = 20; - } else { - log_file_error(fmtarg->filename, fmtarg->first_line, "System task `%s' called with format character `%c' in argument %zu, but the argument is not $time or $realtime.\n", task_name.c_str(), fmt[i], fmtarg - args.begin() + 1); - } + part.type = FmtPart::TIME; + if (!has_width && !has_leading_zero) + part.width = 20; + } else if (fmt[i] == 'm' || fmt[i] == 'M') { + part.type = FmtPart::HIERNAME; } else { log_file_error(fmtarg->filename, fmtarg->first_line, "System task `%s' called with unrecognized format character `%c' in argument %zu.\n", task_name.c_str(), fmt[i], fmtarg - args.begin() + 1); } @@ -425,6 +418,22 @@ void Fmt::parse_verilog(const std::vector &args, bool sformat_lik log_file_error(fmtarg->filename, fmtarg->first_line, "System task `%s' called with incomplete format specifier in argument %zu.\n", task_name.c_str(), fmtarg - args.begin() + 1); } + if (part.type != FmtPart::HIERNAME) { + if (++arg == args.end()) { + log_file_error(fmtarg->filename, fmtarg->first_line, "System task `%s' called with fewer arguments than the format specifiers in argument %zu require.\n", task_name.c_str(), fmtarg - args.begin() + 1); + } + part.sig = arg->sig; + part.signed_ = arg->signed_; + + if (part.type == VerilogFmtArg::TIME) { + if (arg->type == VerilogFmtArg::TIME) { + part.realtime = arg->realtime; + } else { + log_file_error(fmtarg->filename, fmtarg->first_line, "System task `%s' called with format character `%c' in argument %zu, but the argument is not $time or $realtime.\n", task_name.c_str(), fmt[i], fmtarg - args.begin() + 1); + } + } + } + if (part.padding == '\0') part.padding = (has_leading_zero && part.justify == FmtPart::RIGHT) ? '0' : ' '; @@ -551,6 +560,23 @@ std::vector Fmt::emit_verilog() const break; } + case FmtPart::HIERNAME: { + VerilogFmtArg arg; + arg.type = VerilogFmtArg::HIERNAME; + args.push_back(arg); + fmt.str += '%'; + if (part.plus) + fmt.str += '+'; + if (part.justify == FmtPart::LEFT) + fmt.str += '-'; + log_assert(part.padding == ' ' || part.padding == '0'); + if (part.padding == '0' && part.width > 0) + fmt.str += '0'; + fmt.str += std::to_string(part.width); + fmt.str += 'm'; + break; + } + default: log_abort(); } } @@ -637,7 +663,7 @@ void Fmt::emit_cxxrtl(std::ostream &f, std::function emit_sig) const; - std::string render() const; + std::string render(const std::string &hiername, int time = 0) const; private: void apply_verilog_automatic_sizing_and_add(FmtPart &part); diff --git a/passes/sat/sim.cc b/passes/sat/sim.cc index c1a4a7a7f2b..e8f743339aa 100644 --- a/passes/sat/sim.cc +++ b/passes/sat/sim.cc @@ -120,6 +120,7 @@ struct SimShared bool multiclock = false; int next_output_id = 0; int step = 0; + int trace_time = 0; std::vector triggered_assertions; std::vector display_output; bool serious_asserts = false; @@ -851,7 +852,7 @@ struct SimInstance pos += part.sig.size(); } - std::string rendered = print.fmt.render(); + std::string rendered = print.fmt.render(hiername(), shared->trace_time); log("%s", rendered.c_str()); shared->display_output.emplace_back(shared->step, this, cell, rendered); } @@ -1292,8 +1293,9 @@ struct SimWorker : SimShared } } - void update(bool gclk) + void update(bool gclk, int trace_time) { + this->trace_time = trace_time; if (gclk) step += 1; @@ -1369,7 +1371,7 @@ struct SimWorker : SimShared set_inports(clock, State::Sx); set_inports(clockn, State::Sx); - update(false); + update(false, 0); register_output_step(0); @@ -1382,7 +1384,7 @@ struct SimWorker : SimShared set_inports(clock, State::S0); set_inports(clockn, State::S1); - update(true); + update(true, 10*cycle + 5); register_output_step(10*cycle + 5); if (debug) @@ -1398,7 +1400,7 @@ struct SimWorker : SimShared set_inports(resetn, State::S1); } - update(true); + update(true, 10*cycle + 10); register_output_step(10*cycle + 10); } @@ -1510,7 +1512,7 @@ struct SimWorker : SimShared initial = false; } if (did_something) - update(true); + update(true, time); register_output_step(time); bool status = top->checkSignals(); @@ -1654,12 +1656,12 @@ struct SimWorker : SimShared set_inports(clock, State::S0); set_inports(clockn, State::S1); } - update(true); + update(true, 10*cycle); register_output_step(10*cycle); if (!multiclock && cycle) { set_inports(clock, State::S0); set_inports(clockn, State::S1); - update(true); + update(true, 10*cycle + 5); register_output_step(10*cycle + 5); } cycle++; @@ -1731,12 +1733,12 @@ struct SimWorker : SimShared log("Simulating cycle %d.\n", cycle); set_inports(clock, State::S1); set_inports(clockn, State::S0); - update(true); + update(true, 10*cycle+0); register_output_step(10*cycle+0); if (!multiclock) { set_inports(clock, State::S0); set_inports(clockn, State::S1); - update(true); + update(true, 10*cycle+5); register_output_step(10*cycle+5); } cycle++; @@ -1976,7 +1978,7 @@ struct SimWorker : SimShared if (debug) log("Simulating non-active clock edge.\n"); set_yw_clocks(yw, hierarchy, false); - update(false); + update(false, 5); register_output_step(5); } top->set_initstate_outputs(State::S0); @@ -1989,14 +1991,14 @@ struct SimWorker : SimShared if (cycle < GetSize(yw.steps)) set_yw_state(yw, hierarchy, cycle); set_yw_clocks(yw, hierarchy, true); - update(true); + update(true, 10 * cycle); register_output_step(10 * cycle); if (!yw.clocks.empty()) { if (debug) log("Simulating non-active clock edge.\n"); set_yw_clocks(yw, hierarchy, false); - update(false); + update(false, 5 + 10 * cycle); register_output_step(5 + 10 * cycle); } }