From 962acda810d92b3a76d495a070a3795591a9ff80 Mon Sep 17 00:00:00 2001 From: tangxifan Date: Wed, 22 Sep 2021 11:09:46 -0700 Subject: [PATCH] [Engine] Bug fix in fabric key generation when computing configurable children --- openfpga/src/base/openfpga_build_fabric.cpp | 1 + openfpga/src/fabric/fabric_key_writer.cpp | 13 +++++++++-- openfpga/src/fabric/fabric_key_writer.h | 1 + .../fpga_bitstream/build_device_bitstream.cpp | 20 +++-------------- openfpga/src/utils/memory_utils.cpp | 22 +++++++++++++++++++ openfpga/src/utils/memory_utils.h | 9 ++++++++ 6 files changed, 47 insertions(+), 19 deletions(-) diff --git a/openfpga/src/base/openfpga_build_fabric.cpp b/openfpga/src/base/openfpga_build_fabric.cpp index 08e9ef4f8..2c3f491f9 100644 --- a/openfpga/src/base/openfpga_build_fabric.cpp +++ b/openfpga/src/base/openfpga_build_fabric.cpp @@ -132,6 +132,7 @@ int build_fabric(OpenfpgaContext& openfpga_ctx, VTR_ASSERT(false == fkey_fname.empty()); curr_status = write_fabric_key_to_xml_file(openfpga_ctx.module_graph(), fkey_fname, + openfpga_ctx.arch().config_protocol.type(), cmd_context.option_enable(cmd, opt_verbose)); /* If there is any error, final status cannot be overwritten by a success flag */ if (CMD_EXEC_SUCCESS != curr_status) { diff --git a/openfpga/src/fabric/fabric_key_writer.cpp b/openfpga/src/fabric/fabric_key_writer.cpp index c099d5468..d6bea0527 100644 --- a/openfpga/src/fabric/fabric_key_writer.cpp +++ b/openfpga/src/fabric/fabric_key_writer.cpp @@ -14,6 +14,8 @@ #include "openfpga_naming.h" +#include "memory_utils.h" + #include "fabric_key_writer.h" /* begin namespace openfpga */ @@ -29,6 +31,7 @@ namespace openfpga { ***************************************************************************************/ int write_fabric_key_to_xml_file(const ModuleManager& module_manager, const std::string& fname, + const e_config_protocol_type& config_protocol_type, const bool& verbose) { std::string timer_message = std::string("Write fabric key to XML file '") + fname + std::string("'"); @@ -65,9 +68,15 @@ int write_fabric_key_to_xml_file(const ModuleManager& module_manager, /* Create regions for the keys and load keys by region */ for (const ConfigRegionId& config_region : module_manager.regions(top_module)) { FabricRegionId fabric_region = fabric_key.create_region(); - fabric_key.reserve_region_keys(fabric_region, module_manager.region_configurable_children(top_module, config_region).size()); + + /* Each configuration protocol has some child which should not be in the list. They are typically decoders */ + size_t curr_region_num_config_child = module_manager.region_configurable_children(top_module, config_region).size(); + size_t num_child_to_skip = estimate_num_configurable_children_to_skip_by_config_protocol(config_protocol_type, curr_region_num_config_child); + curr_region_num_config_child -= num_child_to_skip; - for (size_t ichild = 0; ichild < module_manager.region_configurable_children(top_module, config_region).size(); ++ichild) { + fabric_key.reserve_region_keys(fabric_region, curr_region_num_config_child); + + for (size_t ichild = 0; ichild < curr_region_num_config_child; ++ichild) { ModuleId child_module = module_manager.region_configurable_children(top_module, config_region)[ichild]; size_t child_instance = module_manager.region_configurable_child_instances(top_module, config_region)[ichild]; vtr::Point child_coord = module_manager.region_configurable_child_coordinates(top_module, config_region)[ichild]; diff --git a/openfpga/src/fabric/fabric_key_writer.h b/openfpga/src/fabric/fabric_key_writer.h index 68e6468b2..3062db15a 100644 --- a/openfpga/src/fabric/fabric_key_writer.h +++ b/openfpga/src/fabric/fabric_key_writer.h @@ -16,6 +16,7 @@ namespace openfpga { int write_fabric_key_to_xml_file(const ModuleManager& module_manager, const std::string& fname, + const e_config_protocol_type& config_protocol_type, const bool& verbose); } /* end namespace openfpga */ diff --git a/openfpga/src/fpga_bitstream/build_device_bitstream.cpp b/openfpga/src/fpga_bitstream/build_device_bitstream.cpp index 0af563914..1c81e2cb3 100644 --- a/openfpga/src/fpga_bitstream/build_device_bitstream.cpp +++ b/openfpga/src/fpga_bitstream/build_device_bitstream.cpp @@ -13,6 +13,7 @@ #include "openfpga_naming.h" +#include "memory_utils.h" #include "module_manager_utils.h" #include "build_grid_bitstream.h" @@ -84,23 +85,8 @@ size_t rec_estimate_device_bitstream_num_bits(const ModuleManager& module_manage if (parent_module == top_module) { for (const ConfigRegionId& config_region : module_manager.regions(parent_module)) { size_t curr_region_num_config_child = module_manager.region_configurable_children(parent_module, config_region).size(); - - /* Frame-based configuration protocol will have 1 decoder - * if there are more than 1 configurable children - */ - if ( (CONFIG_MEM_FRAME_BASED == config_protocol_type) - && (2 <= curr_region_num_config_child)) { - curr_region_num_config_child--; - } - - /* Memory configuration protocol will have 2 decoders - * at the top-level - */ - if (CONFIG_MEM_MEMORY_BANK == config_protocol_type - || CONFIG_MEM_QL_MEMORY_BANK == config_protocol_type) { - VTR_ASSERT(2 <= curr_region_num_config_child); - curr_region_num_config_child -= 2; - } + size_t num_child_to_skip = estimate_num_configurable_children_to_skip_by_config_protocol(config_protocol_type, curr_region_num_config_child); + curr_region_num_config_child -= num_child_to_skip; /* Visit all the children in a recursively way */ for (size_t ichild = 0; ichild < curr_region_num_config_child; ++ichild) { diff --git a/openfpga/src/utils/memory_utils.cpp b/openfpga/src/utils/memory_utils.cpp index 8e2038b06..e5e44b99c 100644 --- a/openfpga/src/utils/memory_utils.cpp +++ b/openfpga/src/utils/memory_utils.cpp @@ -429,5 +429,27 @@ size_t generate_pb_sram_port_size(const e_config_protocol_type sram_orgz_type, return sram_port_size; } +size_t estimate_num_configurable_children_to_skip_by_config_protocol(e_config_protocol_type config_protocol_type, + size_t curr_region_num_config_child) { + size_t num_child_to_skip = 0; + /* Frame-based configuration protocol will have 1 decoder + * if there are more than 1 configurable children + */ + if ( (CONFIG_MEM_FRAME_BASED == config_protocol_type) + && (2 <= curr_region_num_config_child)) { + num_child_to_skip = 1; + } + + /* Memory configuration protocol will have 2 decoders + * at the top-level + */ + if (CONFIG_MEM_MEMORY_BANK == config_protocol_type + || CONFIG_MEM_QL_MEMORY_BANK == config_protocol_type) { + VTR_ASSERT(2 <= curr_region_num_config_child); + num_child_to_skip = 2; + } + + return num_child_to_skip; +} } /* end namespace openfpga */ diff --git a/openfpga/src/utils/memory_utils.h b/openfpga/src/utils/memory_utils.h index 88eecf1f3..d34e5f0ce 100644 --- a/openfpga/src/utils/memory_utils.h +++ b/openfpga/src/utils/memory_utils.h @@ -40,6 +40,15 @@ size_t generate_sram_port_size(const e_config_protocol_type sram_orgz_type, size_t generate_pb_sram_port_size(const e_config_protocol_type sram_orgz_type, const size_t& num_config_bits); +/** + * @brief Compute the number of configurable children to be skipped for a given configuration protocol + * For some configuration protocol, the decoders are not counted as configurable children + * (they are included in the list for bitstream generator usage) + * The number of decoders depends on the type of configuration protocol. + */ +size_t estimate_num_configurable_children_to_skip_by_config_protocol(e_config_protocol_type config_protocol_type, + size_t curr_region_num_config_child); + } /* end namespace openfpga */ #endif