From: Zachary Snow Date: Wed, 20 Oct 2021 00:46:26 +0000 (-0600) Subject: verilog: use derived module info to elaborate cell connections X-Git-Tag: yosys-0.11~25 X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=e833c6a418103feb30f0cc3e5c482da00ee9f820;p=yosys.git verilog: use derived module info to elaborate cell connections - Attempt to lookup a derived module if it potentially contains a port connection with elaboration ambiguities - Mark the cell if module has not yet been derived - This can be extended to implement automatic hierarchical port connections in a future change --- diff --git a/CHANGELOG b/CHANGELOG index a6285ddb2..b980c5a1a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -8,6 +8,14 @@ Yosys 0.10 .. Yosys 0.10-dev * Various - Added $aldff and $aldffe (flip-flops with async load) cells + * SystemVerilog + - Fixed an issue which prevented writing directly to a memory word via a + connection to an output port + - Fixed an issue which prevented unbased unsized literals (e.g., `'1`) from + filling the width of a cell input + - Fixed an issue where connecting a slice covering the entirety of a signed + signal to a cell input would cause a failed assertion + Yosys 0.9 .. Yosys 0.10 -------------------------- diff --git a/README.md b/README.md index ab656352a..cc5c806fb 100644 --- a/README.md +++ b/README.md @@ -489,6 +489,11 @@ Verilog Attributes and non-standard features for use in blackboxes and whiteboxes. Use ``read_verilog -specify`` to enable this functionality. (By default these blocks are ignored.) +- The ``reprocess_after`` internal attribute is used by the Verilog frontend to + mark cells with bindings which might depend on the specified instantiated + module. Modules with such cells will be reprocessed during the ``hierarchy`` + pass once the referenced module definition(s) become available. + Non-standard or SystemVerilog features for formal verification ============================================================== diff --git a/frontends/ast/ast.cc b/frontends/ast/ast.cc index fe1f9e861..7be8ab565 100644 --- a/frontends/ast/ast.cc +++ b/frontends/ast/ast.cc @@ -854,7 +854,7 @@ RTLIL::Const AstNode::bitsAsConst(int width) return bitsAsConst(width, is_signed); } -RTLIL::Const AstNode::asAttrConst() +RTLIL::Const AstNode::asAttrConst() const { log_assert(type == AST_CONSTANT); @@ -869,8 +869,17 @@ RTLIL::Const AstNode::asAttrConst() return val; } -RTLIL::Const AstNode::asParaConst() +RTLIL::Const AstNode::asParaConst() const { + if (type == AST_REALVALUE) + { + AstNode *strnode = AstNode::mkconst_str(stringf("%f", realvalue)); + RTLIL::Const val = strnode->asAttrConst(); + val.flags |= RTLIL::CONST_FLAG_REAL; + delete strnode; + return val; + } + RTLIL::Const val = asAttrConst(); if (is_signed) val.flags |= RTLIL::CONST_FLAG_SIGNED; @@ -1043,8 +1052,11 @@ static RTLIL::Module *process_module(RTLIL::Design *design, AstNode *ast, bool d } } - // TODO(zachjs): make design available to simplify() in the future + // simplify this module or interface using the current design as context + // for lookup up ports and wires within cells + set_simplify_design_context(design); while (ast->simplify(!flag_noopt, false, false, 0, -1, false, false)) { } + set_simplify_design_context(nullptr); if (flag_dump_ast2) { log("Dumping AST after simplification:\n"); @@ -1171,6 +1183,9 @@ static RTLIL::Module *process_module(RTLIL::Design *design, AstNode *ast, bool d continue; module->attributes[attr.first] = attr.second->asAttrConst(); } + for (const AstNode *node : ast->children) + if (node->type == AST_PARAMETER) + current_module->avail_parameters(node->str); } if (ast->type == AST_INTERFACE) @@ -1445,6 +1460,26 @@ void AST::explode_interface_port(AstNode *module_ast, RTLIL::Module * intfmodule } } +// AstModules may contain cells marked with ID::reprocess_after, which indicates +// that it should be reprocessed once the specified module has been elaborated. +bool AstModule::reprocess_if_necessary(RTLIL::Design *design) +{ + for (const RTLIL::Cell *cell : cells()) + { + std::string modname = cell->get_string_attribute(ID::reprocess_after); + if (modname.empty()) + continue; + if (design->module(modname) || design->module("$abstract" + modname)) { + log("Reprocessing module %s because instantiated module %s has become available.\n", + log_id(name), log_id(modname)); + loadconfig(); + process_and_replace_module(design, this, ast, NULL); + return true; + } + } + return false; +} + // When an interface instance is found in a module, the whole RTLIL for the module will be rederived again // from AST. The interface members are copied into the AST module with the prefix of the interface. void AstModule::expand_interfaces(RTLIL::Design *design, const dict &local_interfaces) @@ -1649,6 +1684,17 @@ static std::string serialize_param_value(const RTLIL::Const &val) { return res; } +std::string AST::derived_module_name(std::string stripped_name, const std::vector> ¶meters) { + std::string para_info; + for (const auto &elem : parameters) + para_info += stringf("%s=%s", elem.first.c_str(), serialize_param_value(elem.second).c_str()); + + if (para_info.size() > 60) + return "$paramod$" + sha1(para_info) + stripped_name; + else + return "$paramod" + stripped_name + para_info; +} + // create a new parametric module (when needed) and return the name of the generated module std::string AstModule::derive_common(RTLIL::Design *design, const dict ¶meters, AstNode **new_ast_out, bool quiet) { @@ -1657,9 +1703,8 @@ std::string AstModule::derive_common(RTLIL::Design *design, const dict> named_parameters; for (const auto child : ast->children) { if (child->type != AST_PARAMETER) continue; @@ -1668,25 +1713,21 @@ std::string AstModule::derive_common(RTLIL::Design *design, const dictstr.c_str(), log_signal(it->second)); - para_info += stringf("%s=%s", child->str.c_str(), serialize_param_value(it->second).c_str()); + named_parameters.emplace_back(child->str, it->second); continue; } it = parameters.find(stringf("$%d", para_counter)); if (it != parameters.end()) { if (!quiet) log("Parameter %d (%s) = %s\n", para_counter, child->str.c_str(), log_signal(it->second)); - para_info += stringf("%s=%s", child->str.c_str(), serialize_param_value(it->second).c_str()); + named_parameters.emplace_back(child->str, it->second); continue; } } - std::string modname; - if (parameters.size() == 0) - modname = stripped_name; - else if (para_info.size() > 60) - modname = "$paramod$" + sha1(para_info) + stripped_name; - else - modname = "$paramod" + stripped_name + para_info; + std::string modname = stripped_name; + if (parameters.size()) // not named_parameters to cover hierarchical defparams + modname = derived_module_name(stripped_name, named_parameters); if (design->has(modname)) return modname; diff --git a/frontends/ast/ast.h b/frontends/ast/ast.h index 66bbdd7b4..48ec9a063 100644 --- a/frontends/ast/ast.h +++ b/frontends/ast/ast.h @@ -262,6 +262,7 @@ namespace AST void mem2reg_remove(pool &mem2reg_set, vector &delnodes); void meminfo(int &mem_width, int &mem_size, int &addr_bits); bool detect_latch(const std::string &var); + const RTLIL::Module* lookup_cell_module(); // additional functionality for evaluating constant functions struct varinfo_t { @@ -313,8 +314,8 @@ namespace AST RTLIL::Const bitsAsConst(int width, bool is_signed); RTLIL::Const bitsAsConst(int width = -1); RTLIL::Const bitsAsUnsizedConst(int width); - RTLIL::Const asAttrConst(); - RTLIL::Const asParaConst(); + RTLIL::Const asAttrConst() const; + RTLIL::Const asParaConst() const; uint64_t asInt(bool is_signed); bool bits_only_01() const; bool asBool() const; @@ -349,6 +350,7 @@ namespace AST RTLIL::IdString derive(RTLIL::Design *design, const dict ¶meters, const dict &interfaces, const dict &modports, bool mayfail) override; std::string derive_common(RTLIL::Design *design, const dict ¶meters, AstNode **new_ast_out, bool quiet = false); void expand_interfaces(RTLIL::Design *design, const dict &local_interfaces) override; + bool reprocess_if_necessary(RTLIL::Design *design) override; RTLIL::Module *clone() const override; void loadconfig() const; }; @@ -377,6 +379,14 @@ namespace AST // struct helper exposed from simplify for genrtlil AstNode *make_struct_member_range(AstNode *node, AstNode *member_node); + + // generate standard $paramod... derived module name; parameters should be + // in the order they are declared in the instantiated module + std::string derived_module_name(std::string stripped_name, const std::vector> ¶meters); + + // used to provide simplify() access to the current design for looking up + // modules, ports, wires, etc. + void set_simplify_design_context(const RTLIL::Design *design); } namespace AST_INTERNAL diff --git a/frontends/ast/genrtlil.cc b/frontends/ast/genrtlil.cc index c82664b98..a68bcd9ee 100644 --- a/frontends/ast/genrtlil.cc +++ b/frontends/ast/genrtlil.cc @@ -1917,21 +1917,15 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint) continue; } if (child->type == AST_PARASET) { - int extra_const_flags = 0; IdString paraname = child->str.empty() ? stringf("$%d", ++para_counter) : child->str; - if (child->children[0]->type == AST_REALVALUE) { + const AstNode *value = child->children[0]; + if (value->type == AST_REALVALUE) log_file_warning(filename, location.first_line, "Replacing floating point parameter %s.%s = %f with string.\n", - log_id(cell), log_id(paraname), child->children[0]->realvalue); - extra_const_flags = RTLIL::CONST_FLAG_REAL; - auto strnode = AstNode::mkconst_str(stringf("%f", child->children[0]->realvalue)); - strnode->cloneInto(child->children[0]); - delete strnode; - } - if (child->children[0]->type != AST_CONSTANT) + log_id(cell), log_id(paraname), value->realvalue); + else if (value->type != AST_CONSTANT) log_file_error(filename, location.first_line, "Parameter %s.%s with non-constant value!\n", log_id(cell), log_id(paraname)); - cell->parameters[paraname] = child->children[0]->asParaConst(); - cell->parameters[paraname].flags |= extra_const_flags; + cell->parameters[paraname] = value->asParaConst(); continue; } if (child->type == AST_ARGUMENT) { @@ -1948,7 +1942,12 @@ RTLIL::SigSpec AstNode::genRTLIL(int width_hint, bool sign_hint) if (sig.is_wire()) { // if the resulting SigSpec is a wire, its // signedness should match that of the AstNode - log_assert(arg->is_signed == sig.as_wire()->is_signed); + if (arg->type == AST_IDENTIFIER && arg->id2ast && arg->id2ast->is_signed && !arg->is_signed) + // fully-sliced signed wire will be resolved + // once the module becomes available + log_assert(attributes.count(ID::reprocess_after)); + else + log_assert(arg->is_signed == sig.as_wire()->is_signed); } else if (arg->is_signed) { // non-trivial signed nodes are indirected through // signed wires to enable sign extension diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc index 607ca9a8b..64d0fe475 100644 --- a/frontends/ast/simplify.cc +++ b/frontends/ast/simplify.cc @@ -564,6 +564,115 @@ static std::string prefix_id(const std::string &prefix, const std::string &str) return prefix + str; } +// direct access to this global should be limited to the following two functions +static const RTLIL::Design *simplify_design_context = nullptr; + +void AST::set_simplify_design_context(const RTLIL::Design *design) +{ + log_assert(!simplify_design_context || !design); + simplify_design_context = design; +} + +// lookup the module with the given name in the current design context +static const RTLIL::Module* lookup_module(const std::string &name) +{ + return simplify_design_context->module(name); +} + +const RTLIL::Module* AstNode::lookup_cell_module() +{ + log_assert(type == AST_CELL); + + auto reprocess_after = [this] (const std::string &modname) { + if (!attributes.count(ID::reprocess_after)) + attributes[ID::reprocess_after] = AstNode::mkconst_str(modname); + }; + + const AstNode *celltype = nullptr; + for (const AstNode *child : children) + if (child->type == AST_CELLTYPE) { + celltype = child; + break; + } + log_assert(celltype != nullptr); + + const RTLIL::Module *module = lookup_module(celltype->str); + if (!module) + module = lookup_module("$abstract" + celltype->str); + if (!module) { + if (celltype->str.at(0) != '$') + reprocess_after(celltype->str); + return nullptr; + } + + // build a mapping from true param name to param value + size_t para_counter = 0; + dict cell_params_map; + for (AstNode *child : children) { + if (child->type != AST_PARASET) + continue; + + if (child->str.empty() && para_counter >= module->avail_parameters.size()) + return nullptr; // let hierarchy handle this error + IdString paraname = child->str.empty() ? module->avail_parameters[para_counter++] : child->str; + + const AstNode *value = child->children[0]; + if (value->type != AST_REALVALUE && value->type != AST_CONSTANT) + return nullptr; // let genrtlil handle this error + cell_params_map[paraname] = value->asParaConst(); + } + + // put the parameters in order and generate the derived module name + std::vector> named_parameters; + for (RTLIL::IdString param : module->avail_parameters) { + auto it = cell_params_map.find(param); + if (it != cell_params_map.end()) + named_parameters.emplace_back(it->first, it->second); + } + std::string modname = celltype->str; + if (cell_params_map.size()) // not named_parameters to cover hierarchical defparams + modname = derived_module_name(celltype->str, named_parameters); + + // try to find the resolved module + module = lookup_module(modname); + if (!module) { + reprocess_after(modname); + return nullptr; + } + return module; +} + +// returns whether an expression contains an unbased unsized literal; does not +// check the literal exists in a self-determined context +static bool contains_unbased_unsized(const AstNode *node) +{ + if (node->type == AST_CONSTANT) + return node->is_unsized; + for (const AstNode *child : node->children) + if (contains_unbased_unsized(child)) + return true; + return false; +} + +// adds a wire to the current module with the given name that matches the +// dimensions of the given wire reference +void add_wire_for_ref(const RTLIL::Wire *ref, const std::string &str) +{ + AstNode *left = AstNode::mkconst_int(ref->width - 1 + ref->start_offset, true); + AstNode *right = AstNode::mkconst_int(ref->start_offset, true); + if (ref->upto) + std::swap(left, right); + AstNode *range = new AstNode(AST_RANGE, left, right); + + AstNode *wire = new AstNode(AST_WIRE, range); + wire->is_signed = ref->is_signed; + wire->is_logic = true; + wire->str = str; + + current_ast_mod->children.push_back(wire); + current_scope[str] = wire; +} + // convert the AST into a simpler AST that has all parameters substituted by their // values, unrolled for-loops, expanded generate blocks, etc. when this function // is done with an AST it can be converted into RTLIL using genRTLIL(). @@ -920,19 +1029,110 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage, } } - if (type == AST_ARGUMENT) - { - if (children.size() == 1 && children[0]->type == AST_CONSTANT) - { - // HACK: For port bindings using unbased unsized literals, mark them - // signed so they sign-extend. The hierarchy will still incorrectly - // generate a warning complaining about resizing the expression. - // This also doesn't handle the complex of something like a ternary - // expression bound to a port, where the actual size of the port is - // needed to resolve the expression correctly. - AstNode *arg = children[0]; - if (arg->is_unsized) - arg->is_signed = true; + if (type == AST_CELL) { + bool lookup_suggested = false; + + for (AstNode *child : children) { + // simplify any parameters to constants + if (child->type == AST_PARASET) + while (child->simplify(true, false, false, 1, -1, false, true)) { } + + // look for patterns which _may_ indicate ambiguity requiring + // resolution of the underlying module + if (child->type == AST_ARGUMENT) { + if (child->children.size() != 1) + continue; + const AstNode *value = child->children[0]; + if (value->type == AST_IDENTIFIER) { + const AstNode *elem = value->id2ast; + if (elem == nullptr) { + if (current_scope.count(value->str)) + elem = current_scope.at(value->str); + else + continue; + } + if (elem->type == AST_MEMORY) + // need to determine is the is a read or wire + lookup_suggested = true; + else if (elem->type == AST_WIRE && elem->is_signed && !value->children.empty()) + // this may be a fully sliced signed wire which needs + // to be indirected to produce an unsigned connection + lookup_suggested = true; + } + else if (contains_unbased_unsized(value)) + // unbased unsized literals extend to width of the context + lookup_suggested = true; + } + } + + const RTLIL::Module *module = nullptr; + if (lookup_suggested) + module = lookup_cell_module(); + if (module) { + size_t port_counter = 0; + for (AstNode *child : children) { + if (child->type != AST_ARGUMENT) + continue; + + // determine the full name of port this argument is connected to + RTLIL::IdString port_name; + if (child->str.size()) + port_name = child->str; + else { + if (port_counter >= module->ports.size()) + log_file_error(filename, location.first_line, + "Cell instance has more ports than the module!\n"); + port_name = module->ports[port_counter++]; + } + + // find the port's wire in the underlying module + const RTLIL::Wire *ref = module->wire(port_name); + if (ref == nullptr) + log_file_error(filename, location.first_line, + "Cell instance refers to port %s which does not exist in module %s!.\n", + log_id(port_name), log_id(module->name)); + + // select the argument, if present + log_assert(child->children.size() <= 1); + if (child->children.empty()) + continue; + AstNode *arg = child->children[0]; + + // plain identifiers never need indirection; this also prevents + // adding infinite levels of indirection + if (arg->type == AST_IDENTIFIER && arg->children.empty()) + continue; + + // only add indirection for standard inputs or outputs + if (ref->port_input == ref->port_output) + continue; + + did_something = true; + + // create the indirection wire + std::stringstream sstr; + sstr << "$indirect$" << ref->name.c_str() << "$" << filename << ":" << location.first_line << "$" << (autoidx++); + std::string tmp_str = sstr.str(); + add_wire_for_ref(ref, tmp_str); + + AstNode *asgn = new AstNode(AST_ASSIGN); + current_ast_mod->children.push_back(asgn); + + AstNode *ident = new AstNode(AST_IDENTIFIER); + ident->str = tmp_str; + child->children[0] = ident->clone(); + + if (ref->port_input && !ref->port_output) { + asgn->children.push_back(ident); + asgn->children.push_back(arg); + } else { + log_assert(!ref->port_input && ref->port_output); + asgn->children.push_back(arg); + asgn->children.push_back(ident); + } + } + + } } diff --git a/kernel/constids.inc b/kernel/constids.inc index 8d8e97eb7..8ccb60089 100644 --- a/kernel/constids.inc +++ b/kernel/constids.inc @@ -163,6 +163,7 @@ X(RD_TRANSPARENCY_MASK) X(RD_TRANSPARENT) X(RD_WIDE_CONTINUATION) X(reg) +X(reprocess_after) X(S) X(SET) X(SET_POLARITY) diff --git a/kernel/rtlil.cc b/kernel/rtlil.cc index 9fac57523..88153a380 100644 --- a/kernel/rtlil.cc +++ b/kernel/rtlil.cc @@ -941,6 +941,11 @@ void RTLIL::Module::expand_interfaces(RTLIL::Design *, const dict &, bool mayfail) { if (mayfail) diff --git a/kernel/rtlil.h b/kernel/rtlil.h index c428f3154..68481b81c 100644 --- a/kernel/rtlil.h +++ b/kernel/rtlil.h @@ -1161,6 +1161,7 @@ public: virtual RTLIL::IdString derive(RTLIL::Design *design, const dict ¶meters, const dict &interfaces, const dict &modports, bool mayfail = false); virtual size_t count_id(RTLIL::IdString id); virtual void expand_interfaces(RTLIL::Design *design, const dict &local_interfaces); + virtual bool reprocess_if_necessary(RTLIL::Design *design); virtual void sort(); virtual void check(); diff --git a/passes/hierarchy/hierarchy.cc b/passes/hierarchy/hierarchy.cc index 1e69ab903..440881f19 100644 --- a/passes/hierarchy/hierarchy.cc +++ b/passes/hierarchy/hierarchy.cc @@ -558,6 +558,10 @@ bool expand_module(RTLIL::Design *design, RTLIL::Module *module, bool flag_check return did_something; } + // Now that modules have been derived, we may want to reprocess this + // module given the additional available context. + if (module->reprocess_if_necessary(design)) + return true; for (auto &it : array_cells) { diff --git a/passes/techmap/techmap.cc b/passes/techmap/techmap.cc index a69a6d460..5cd78fe28 100644 --- a/passes/techmap/techmap.cc +++ b/passes/techmap/techmap.cc @@ -377,10 +377,12 @@ struct TechmapWorker if (c->attributes.count(ID::src)) c->add_strpool_attribute(ID::src, extra_src_attrs); - if (techmap_replace_cell) + if (techmap_replace_cell) { for (auto attr : cell->attributes) if (!c->attributes.count(attr.first)) c->attributes[attr.first] = attr.second; + c->attributes.erase(ID::reprocess_after); + } } for (auto &it : tpl->connections()) { diff --git a/tests/simple/memwr_port_connection.sv b/tests/simple/memwr_port_connection.sv new file mode 100644 index 000000000..5bf414e08 --- /dev/null +++ b/tests/simple/memwr_port_connection.sv @@ -0,0 +1,13 @@ +module producer( + output logic [3:0] out +); + assign out = 4'hA; +endmodule + +module top( + output logic [3:0] out +); + logic [3:0] v[0:0]; + producer p(v[0]); + assign out = v[0]; +endmodule diff --git a/tests/simple/signed_full_slice.v b/tests/simple/signed_full_slice.v new file mode 100644 index 000000000..f8a331578 --- /dev/null +++ b/tests/simple/signed_full_slice.v @@ -0,0 +1,29 @@ +module pass_through_a( + input wire [31:0] inp, + output wire [31:0] out +); + assign out[31:0] = inp[31:0]; +endmodule + +module top_a( + input wire signed [31:0] inp, + output wire signed [31:0] out +); + pass_through_a pt(inp[31:0], out[31:0]); +endmodule + +// tests both module declaration orderings + +module top_b( + input wire signed [31:0] inp, + output wire signed [31:0] out +); + pass_through_b pt(inp[31:0], out[31:0]); +endmodule + +module pass_through_b( + input wire [31:0] inp, + output wire [31:0] out +); + assign out[31:0] = inp[31:0]; +endmodule diff --git a/tests/verilog/unbased_unsized_tern.sv b/tests/verilog/unbased_unsized_tern.sv new file mode 100644 index 000000000..ad8493394 --- /dev/null +++ b/tests/verilog/unbased_unsized_tern.sv @@ -0,0 +1,31 @@ +module pass_through #( + parameter WIDTH = 1 +) ( + input logic [WIDTH-1:0] inp, + output logic [WIDTH-1:0] out +); + assign out = inp; +endmodule + +module gate ( + input logic inp, + output logic [63:0] + out1, out2, out3, out4 +); + pass_through #(40) pt1('1, out1); + pass_through #(40) pt2(inp ? '1 : '0, out2); + pass_through #(40) pt3(inp ? '1 : 2'b10, out3); + pass_through #(40) pt4(inp ? '1 : inp, out4); +endmodule + +module gold ( + input logic inp, + output logic [63:0] + out1, out2, out3, out4 +); + localparam ONES = 40'hFF_FFFF_FFFF; + pass_through #(40) pt1(ONES, out1); + pass_through #(40) pt2(inp ? ONES : 0, out2); + pass_through #(40) pt3(inp ? ONES : 2'sb10, out3); + pass_through #(40) pt4(inp ? ONES : inp, out4); +endmodule diff --git a/tests/verilog/unbased_unsized_tern.ys b/tests/verilog/unbased_unsized_tern.ys new file mode 100644 index 000000000..5ef63c559 --- /dev/null +++ b/tests/verilog/unbased_unsized_tern.ys @@ -0,0 +1,6 @@ +read_verilog -sv unbased_unsized_tern.sv +hierarchy +proc +equiv_make gold gate equiv +equiv_simple +equiv_status -assert