From d5da74d12226127d37df274869fc4ccaca80a0fc Mon Sep 17 00:00:00 2001 From: Kamil Rakoczy Date: Mon, 6 Mar 2023 11:21:28 +0100 Subject: [PATCH 1/3] systemverilog-plugin: make sure size of constant handled by mkconst_int is lower than 32bits Signed-off-by: Kamil Rakoczy --- systemverilog-plugin/UhdmAst.cc | 36 ++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/systemverilog-plugin/UhdmAst.cc b/systemverilog-plugin/UhdmAst.cc index 0b465f0b6..c07f6e62a 100644 --- a/systemverilog-plugin/UhdmAst.cc +++ b/systemverilog-plugin/UhdmAst.cc @@ -1236,6 +1236,8 @@ AST::AstNode *UhdmAst::process_value(vpiHandle obj_h) vpi_get_value(obj_h, &val); std::string strValType = "'"; bool is_signed = false; + bool is_unsized = false; + int size = -1; if (vpiHandle typespec_h = vpi_handle(vpiTypespec, obj_h)) { is_signed = vpi_get(vpiSigned, typespec_h); if (is_signed) { @@ -1281,11 +1283,10 @@ AST::AstNode *UhdmAst::process_value(vpiHandle obj_h) break; } - int size = -1; // Surelog sometimes report size as part of vpiTypespec (e.g. int_typespec) // if it is the case, we need to set size to the left_range of first packed range visit_one_to_one({vpiTypespec}, obj_h, [&](AST::AstNode *node) { - if (node && node->attributes.count(UhdmAst::packed_ranges()) && node->attributes[UhdmAst::packed_ranges()]->children.size() && + if (node->attributes.count(UhdmAst::packed_ranges()) && node->attributes[UhdmAst::packed_ranges()]->children.size() && node->attributes[UhdmAst::packed_ranges()]->children[0]->children.size()) { size = node->attributes[UhdmAst::packed_ranges()]->children[0]->children[0]->integer + 1; } @@ -1293,17 +1294,21 @@ AST::AstNode *UhdmAst::process_value(vpiHandle obj_h) if (size == -1) { size = vpi_get(vpiSize, obj_h); } - // Surelog by default returns 64 bit numbers and stardard says that they shall be at least 32bits - // yosys is assuming that int/uint is 32 bit, so we are setting here correct size - // NOTE: it *shouldn't* break on explicite 64 bit const values, as they *should* be handled - // above by vpi*StrVal if (size == 64) { + // we know that value can be stored in 32bit integer + // and yosys only supports 32 bits size = 32; - is_signed = true; } - auto c = AST::AstNode::mkconst_int(val.format == vpiUIntVal ? val.value.uint : val.value.integer, is_signed, size > 0 ? size : 32); - if (size == 0 || size == -1) - c->is_unsized = true; + is_signed = true; + if (size == -1) { + is_unsized = false; + size = 32; + } + if (size > 32) { + report_error("Constant with size: %s bits handled by mkconst_int that doesn't work with values larger than 32bits: %d\n!", size); + } + auto c = AST::AstNode::mkconst_int(val.format == vpiUIntVal ? val.value.uint : val.value.integer, is_signed, size); + c->is_unsized = is_unsized; return c; } case vpiRealVal: @@ -1345,7 +1350,7 @@ AST::AstNode *UhdmAst::process_value(vpiHandle obj_h) if (std::strchr(val.value.str, '\'')) { return ::systemverilog_plugin::const2ast(val.value.str, caseType, false); } else { - auto size = vpi_get(vpiSize, obj_h); + size = vpi_get(vpiSize, obj_h); if (size == 0) { auto c = AST::AstNode::mkconst_int(atoi(val.value.str), true, 32); c->is_unsized = true; @@ -4016,13 +4021,16 @@ void UhdmAst::process_string_typespec() void UhdmAst::process_bit_typespec() { + std::vector packed_ranges; // comes before wire name current_node = make_ast_node(AST::AST_WIRE); visit_range(obj_h, [&](AST::AstNode *node) { - if (node) { - current_node->children.push_back(node); - } + packed_ranges.push_back(node); }); + if (packed_ranges.empty()) { + packed_ranges.push_back(make_range(0,0)); + } current_node->is_signed = vpi_get(vpiSigned, obj_h); + add_multirange_wire(current_node, packed_ranges, {}); } void UhdmAst::process_repeat() From 871857eb56a34bf9e4daf9a1ce44de564a573212 Mon Sep 17 00:00:00 2001 From: Kamil Rakoczy Date: Mon, 6 Mar 2023 13:02:46 +0100 Subject: [PATCH 2/3] systemverilog-plugin: handle module names when parameter value is larger than 32bit Signed-off-by: Kamil Rakoczy --- systemverilog-plugin/UhdmAst.cc | 36 ++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/systemverilog-plugin/UhdmAst.cc b/systemverilog-plugin/UhdmAst.cc index c07f6e62a..f7c31f161 100644 --- a/systemverilog-plugin/UhdmAst.cc +++ b/systemverilog-plugin/UhdmAst.cc @@ -1276,13 +1276,6 @@ AST::AstNode *UhdmAst::process_value(vpiHandle obj_h) } [[fallthrough]]; case vpiIntVal: { - if (val.value.integer > std::numeric_limits::max()) { - strValType = "'sd"; - string str_value = std::to_string(val.value.integer); - val.value.str = strdup(str_value.c_str()); - break; - } - // Surelog sometimes report size as part of vpiTypespec (e.g. int_typespec) // if it is the case, we need to set size to the left_range of first packed range visit_one_to_one({vpiTypespec}, obj_h, [&](AST::AstNode *node) { @@ -1304,8 +1297,16 @@ AST::AstNode *UhdmAst::process_value(vpiHandle obj_h) is_unsized = false; size = 32; } + if (val.value.integer > std::numeric_limits::max() || size > 32) { + strValType = "'sd"; + string str_value = std::to_string(val.value.integer); + val.value.str = strdup(str_value.c_str()); + break; + } if (size > 32) { - report_error("Constant with size: %s bits handled by mkconst_int that doesn't work with values larger than 32bits: %d\n!", size); + const uhdm_handle *const handle = (const uhdm_handle *)obj_h; + const UHDM::BaseClass *const object = (const UHDM::BaseClass *)handle->object; + report_error("%.*s:%d: Constant with size: %d bits handled by mkconst_int that doesn't work with values larger than 32bits\n!", (int)object->VpiFile().length(), object->VpiFile().data(), object->VpiLineNo(), size); } auto c = AST::AstNode::mkconst_int(val.format == vpiUIntVal ? val.value.uint : val.value.integer, is_signed, size); c->is_unsized = is_unsized; @@ -1896,11 +1897,26 @@ void UhdmAst::process_module() } } if (shared.top_nodes.count(type)) { - if (!node->children[0]->str.empty()) + if (!node->children[0]->str.empty()) { module_parameters += node->str + "=" + node->children[0]->str; - else + } else if (node->children[0]->bits.size() > 32) { + std::string value = std::to_string(node->children[0]->bits.size()) + "'b"; + for (const auto& bit : node->children[0]->bits) { + switch (bit) { + case RTLIL::State::S0: value += "0"; break; + case RTLIL::State::S1: value += "1"; break; + case RTLIL::State::Sx: value += "x"; break; + case RTLIL::State::Sz: value += "z"; break; + case RTLIL::State::Sa: value += "a"; break; + case RTLIL::State::Sm: value += "m"; break; + default: report_error("Unhandled RTLIL State case!\n"); + } + } + module_parameters += node->str + "=" + value; + } else { module_parameters += node->str + "=" + std::to_string(node->children[0]->bits.size()) + "'d" + std::to_string(node->children[0]->integer); + } } delete node; } From 56b7b3b798ae39dbd3898f485e1750e0b3dda7b8 Mon Sep 17 00:00:00 2001 From: Kamil Rakoczy Date: Mon, 6 Mar 2023 13:10:44 +0100 Subject: [PATCH 3/3] systemverilog-plugin: apply format Signed-off-by: Kamil Rakoczy --- systemverilog-plugin/UhdmAst.cc | 40 +++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/systemverilog-plugin/UhdmAst.cc b/systemverilog-plugin/UhdmAst.cc index f7c31f161..054f9f3ac 100644 --- a/systemverilog-plugin/UhdmAst.cc +++ b/systemverilog-plugin/UhdmAst.cc @@ -1306,7 +1306,8 @@ AST::AstNode *UhdmAst::process_value(vpiHandle obj_h) if (size > 32) { const uhdm_handle *const handle = (const uhdm_handle *)obj_h; const UHDM::BaseClass *const object = (const UHDM::BaseClass *)handle->object; - report_error("%.*s:%d: Constant with size: %d bits handled by mkconst_int that doesn't work with values larger than 32bits\n!", (int)object->VpiFile().length(), object->VpiFile().data(), object->VpiLineNo(), size); + report_error("%.*s:%d: Constant with size: %d bits handled by mkconst_int that doesn't work with values larger than 32bits\n!", + (int)object->VpiFile().length(), object->VpiFile().data(), object->VpiLineNo(), size); } auto c = AST::AstNode::mkconst_int(val.format == vpiUIntVal ? val.value.uint : val.value.integer, is_signed, size); c->is_unsized = is_unsized; @@ -1901,15 +1902,28 @@ void UhdmAst::process_module() module_parameters += node->str + "=" + node->children[0]->str; } else if (node->children[0]->bits.size() > 32) { std::string value = std::to_string(node->children[0]->bits.size()) + "'b"; - for (const auto& bit : node->children[0]->bits) { + for (const auto &bit : node->children[0]->bits) { switch (bit) { - case RTLIL::State::S0: value += "0"; break; - case RTLIL::State::S1: value += "1"; break; - case RTLIL::State::Sx: value += "x"; break; - case RTLIL::State::Sz: value += "z"; break; - case RTLIL::State::Sa: value += "a"; break; - case RTLIL::State::Sm: value += "m"; break; - default: report_error("Unhandled RTLIL State case!\n"); + case RTLIL::State::S0: + value += "0"; + break; + case RTLIL::State::S1: + value += "1"; + break; + case RTLIL::State::Sx: + value += "x"; + break; + case RTLIL::State::Sz: + value += "z"; + break; + case RTLIL::State::Sa: + value += "a"; + break; + case RTLIL::State::Sm: + value += "m"; + break; + default: + report_error("Unhandled RTLIL State case!\n"); } } module_parameters += node->str + "=" + value; @@ -4037,13 +4051,11 @@ void UhdmAst::process_string_typespec() void UhdmAst::process_bit_typespec() { - std::vector packed_ranges; // comes before wire name + std::vector packed_ranges; // comes before wire name current_node = make_ast_node(AST::AST_WIRE); - visit_range(obj_h, [&](AST::AstNode *node) { - packed_ranges.push_back(node); - }); + visit_range(obj_h, [&](AST::AstNode *node) { packed_ranges.push_back(node); }); if (packed_ranges.empty()) { - packed_ranges.push_back(make_range(0,0)); + packed_ranges.push_back(make_range(0, 0)); } current_node->is_signed = vpi_get(vpiSigned, obj_h); add_multirange_wire(current_node, packed_ranges, {});