Update code based on feedback

libs/libarchopenfpga/src/bitstream_setting.cpp

@@ -24,6 +24,12 @@ BitstreamSetting::interconnect_settings() const {
                          interconnect_setting_ids_.end());
 }
 
+BitstreamSetting::overwrite_bitstream_range
+BitstreamSetting::overwrite_bitstreams() const {
+  return vtr::make_range(overwrite_bitstream_ids_.begin(),
+                         overwrite_bitstream_ids_.end());
+}
+
 /************************************************************************
  * Constructors
  ***********************************************************************/
@@ -106,8 +112,16 @@ std::vector<NonFabricBitstreamSetting> BitstreamSetting::non_fabric() const {
   return non_fabric_;
 }
 
-std::vector<PathBitSetting> BitstreamSetting::path_bit_settings() const {
+std::string BitstreamSetting::overwrite_bitstream_path(
-  return path_bit_settings_;
+  const OverwriteBitstreamId& id) const {
+  VTR_ASSERT(true == valid_overwrite_bitstream_id(id));
+  return overwrite_bitstream_paths_[id];
+}
+
+bool BitstreamSetting::overwrite_bitstream_value(
+  const OverwriteBitstreamId& id) const {
+  VTR_ASSERT(true == valid_overwrite_bitstream_id(id));
+  return overwrite_bitstream_values_[id];
 }
 
 /************************************************************************
@@ -182,10 +196,19 @@ void BitstreamSetting::add_non_fabric_pb(const std::string& pb,
   }
 }
 
-void BitstreamSetting::add_path_bit_setting(const std::string& path,
+OverwriteBitstreamId BitstreamSetting::add_overwrite_bitstream(
-                                            const bool value) {
+  const std::string& path, const bool& value) {
   VTR_ASSERT(path.size());
-  path_bit_settings_.push_back(PathBitSetting(path, value));
+  VTR_ASSERT(overwrite_bitstream_ids_.size() ==
+             overwrite_bitstream_paths_.size());
+  VTR_ASSERT(overwrite_bitstream_paths_.size() ==
+             overwrite_bitstream_values_.size());
+  OverwriteBitstreamId id =
+    OverwriteBitstreamId(overwrite_bitstream_ids_.size());
+  overwrite_bitstream_ids_.push_back(id);
+  overwrite_bitstream_paths_.push_back(path);
+  overwrite_bitstream_values_.push_back(value);
+  return id;
 }
 
 /************************************************************************
@@ -204,4 +227,14 @@ bool BitstreamSetting::valid_bitstream_interconnect_setting_id(
           interconnect_setting_ids_[interconnect_setting_id]);
 }
 
+bool BitstreamSetting::valid_overwrite_bitstream_id(
+  const OverwriteBitstreamId& id) const {
+  VTR_ASSERT(overwrite_bitstream_ids_.size() ==
+             overwrite_bitstream_paths_.size());
+  VTR_ASSERT(overwrite_bitstream_paths_.size() ==
+             overwrite_bitstream_values_.size());
+  return (size_t(id) < overwrite_bitstream_ids_.size()) &&
+         (id == overwrite_bitstream_ids_[id]);
+}
+
 }  // namespace openfpga

libs/libarchopenfpga/src/bitstream_setting.h

@@ -37,12 +37,6 @@ struct NonFabricBitstreamSetting {
   std::vector<NonFabricBitstreamPBSetting> pbs;
 };
 
-struct PathBitSetting {
-  PathBitSetting(const std::string& p, bool v) : path(p), value(v) {}
-  const std::string path = "";
-  const bool value = false;
-};
-
 /********************************************************************
  * A data structure to describe bitstream settings
  *
@@ -67,11 +61,15 @@ class BitstreamSetting {
   typedef vtr::vector<BitstreamInterconnectSettingId,
                       BitstreamInterconnectSettingId>::const_iterator
     bitstream_interconnect_setting_iterator;
+  typedef vtr::vector<OverwriteBitstreamId,
+                      OverwriteBitstreamId>::const_iterator
+    overwrite_bitstream_iterator;
   /* Create range */
   typedef vtr::Range<bitstream_pb_type_setting_iterator>
     bitstream_pb_type_setting_range;
   typedef vtr::Range<bitstream_interconnect_setting_iterator>
     bitstream_interconnect_setting_range;
+  typedef vtr::Range<overwrite_bitstream_iterator> overwrite_bitstream_range;
 
  public: /* Constructors */
   BitstreamSetting();
@@ -79,6 +77,7 @@ class BitstreamSetting {
  public: /* Accessors: aggregates */
   bitstream_pb_type_setting_range pb_type_settings() const;
   bitstream_interconnect_setting_range interconnect_settings() const;
+  overwrite_bitstream_range overwrite_bitstreams() const;
 
  public: /* Public Accessors */
   std::string pb_type_name(
@@ -104,7 +103,8 @@ class BitstreamSetting {
   std::string default_path(
     const BitstreamInterconnectSettingId& interconnect_setting_id) const;
   std::vector<NonFabricBitstreamSetting> non_fabric() const;
-  std::vector<PathBitSetting> path_bit_settings() const;
+  std::string overwrite_bitstream_path(const OverwriteBitstreamId& id) const;
+  bool overwrite_bitstream_value(const OverwriteBitstreamId& id) const;
 
  public: /* Public Mutators */
   BitstreamPbTypeSettingId add_bitstream_pb_type_setting(
@@ -127,13 +127,15 @@ class BitstreamSetting {
   void add_non_fabric(const std::string& name, const std::string& file);
   void add_non_fabric_pb(const std::string& pb, const std::string& content);
 
-  void add_path_bit_setting(const std::string& path, const bool value);
+  OverwriteBitstreamId add_overwrite_bitstream(const std::string& path,
+                                               const bool& value);
 
  public: /* Public Validators */
   bool valid_bitstream_pb_type_setting_id(
     const BitstreamPbTypeSettingId& pb_type_setting_id) const;
   bool valid_bitstream_interconnect_setting_id(
     const BitstreamInterconnectSettingId& interconnect_setting_id) const;
+  bool valid_overwrite_bitstream_id(const OverwriteBitstreamId& id) const;
 
  private: /* Internal data */
   /* Pb type -related settings
@@ -171,7 +173,10 @@ class BitstreamSetting {
   vtr::vector<BitstreamInterconnectSettingId, std::string>
     interconnect_default_paths_;
   std::vector<NonFabricBitstreamSetting> non_fabric_;
-  std::vector<PathBitSetting> path_bit_settings_;
+  vtr::vector<OverwriteBitstreamId, OverwriteBitstreamId>
+    overwrite_bitstream_ids_;
+  vtr::vector<OverwriteBitstreamId, std::string> overwrite_bitstream_paths_;
+  vtr::vector<OverwriteBitstreamId, bool> overwrite_bitstream_values_;
 };
 
 }  // namespace openfpga

libs/libarchopenfpga/src/bitstream_setting_fwd.h

@@ -15,11 +15,13 @@
 
 struct bitstream_pb_type_setting_id_tag;
 struct bitstream_interconnect_setting_id_tag;
+struct overwrite_bitstream_id_tag;
 
 typedef vtr::StrongId<bitstream_pb_type_setting_id_tag>
   BitstreamPbTypeSettingId;
 typedef vtr::StrongId<bitstream_interconnect_setting_id_tag>
   BitstreamInterconnectSettingId;
+typedef vtr::StrongId<overwrite_bitstream_id_tag> OverwriteBitstreamId;
 
 /* Short declaration of class */
 class BitstreamSetting;

libs/libarchopenfpga/src/read_xml_bitstream_setting.cpp

@@ -105,17 +105,21 @@ static void read_xml_non_fabric_bitstream_setting(
 /********************************************************************
  * Parse XML description for a bit setting under a <bit> XML node
  *******************************************************************/
-static void read_xml_overwrite_bit_setting(
+static void read_xml_overwrite_bitstream_setting(
   pugi::xml_node& xml_overwrite_bitstream, const pugiutil::loc_data& loc_data,
   openfpga::BitstreamSetting& bitstream_setting) {
+  // Loopthrough bit
   for (pugi::xml_node xml_bit : xml_overwrite_bitstream.children()) {
+    if (xml_bit.name() != std::string("bit")) {
+      bad_tag(xml_bit, loc_data, xml_overwrite_bitstream, {"bit"});
+    }
     const std::string& path_attr =
       get_attribute(xml_bit, "path", loc_data).as_string();
     const std::string& value_attr =
       get_attribute(xml_bit, "value", loc_data).as_string();
     VTR_ASSERT(value_attr == "0" || value_attr == "1");
     /* Add to bit */
-    bitstream_setting.add_path_bit_setting(path_attr, value_attr == "1");
+    bitstream_setting.add_overwrite_bitstream(path_attr, value_attr == "1");
   }
 }
 
@@ -150,7 +154,8 @@ openfpga::BitstreamSetting read_xml_bitstream_setting(
                                             bitstream_setting);
     } else {
       VTR_ASSERT_SAFE(xml_child.name() == std::string("overwrite_bitstream"));
-      read_xml_overwrite_bit_setting(xml_child, loc_data, bitstream_setting);
+      read_xml_overwrite_bitstream_setting(xml_child, loc_data,
+                                           bitstream_setting);
     }
   }
 

libs/libfpgabitstream/src/bitstream_manager.cpp

@@ -6,6 +6,7 @@
 #include <algorithm>
 
 #include "bitstream_manager_utils.h"
+#include "openfpga_tokenizer.h"
 #include "vtr_assert.h"
 #include "vtr_log.h"
 
@@ -297,25 +298,27 @@ void BitstreamManager::add_output_net_id_to_block(
   block_output_net_ids_[block] = output_net_id;
 }
 
-void BitstreamManager::set_path_bit(const std::string& path, const bool value) {
+void BitstreamManager::overwrite_bitstream(const std::string& path,
+                                           const bool& value) {
   ConfigBlockId block;
-  std::vector<std::string> blocks =
+  StringToken tokenizer(path);
-    reverse_split_bit_path_to_blocks((std::string)(path));
+  std::vector<std::string> blocks = tokenizer.split(".");
-  std::string block_name = "";
+  std::reverse(blocks.begin(), blocks.end());
+  std::string current_block_name = "";
   bool match = false;
   for (size_t i = 0; i < bit_values_.size() && !match; i++) {
     block = bit_parent_blocks_[ConfigBitId(i)];
-    if (size_t(block) < num_blocks_) {
+    if (valid_block_id(block)) {
       size_t index = find_bitstream_manager_config_bit_index_in_parent_block(
         *this, ConfigBitId(i));
-      block_name = block_names_[block] + ("[" + std::to_string(index) + "]");
+      current_block_name =
-      if (block_name == blocks[0]) {
+        block_name(block) + ("[" + std::to_string(index) + "]");
+      if (current_block_name == blocks[0]) {
         match = true;
         for (size_t b = 1; b < blocks.size() && match; b++) {
-          valid_block_id(block);
+          block = block_parent(block);
-          block = parent_block_ids_[block];
+          if (valid_block_id(block)) {
-          if (size_t(block) < num_blocks_) {
+            if (block_name(block) != blocks[b]) {
-            if (block_names_[block] != blocks[b]) {
               match = false;
             }
           } else {
@@ -323,8 +326,7 @@ void BitstreamManager::set_path_bit(const std::string& path, const bool value) {
           }
         }
         if (match) {
-          valid_block_id(block);
+          if (!valid_block_id(block_parent(block))) {
-          if (parent_block_ids_[block] == ConfigBlockId::INVALID()) {
             bit_values_[ConfigBitId(i)] = value ? '1' : '0';
           } else {
             match = false;

libs/libfpgabitstream/src/bitstream_manager.h

@@ -214,7 +214,7 @@ class BitstreamManager {
                                   const std::string& output_net_id);
 
   /* Set bit to the bitstream at the given path */
-  void set_path_bit(const std::string& path, const bool value);
+  void overwrite_bitstream(const std::string& path, const bool& value);
 
  public: /* Public Validators */
   bool valid_bit_id(const ConfigBitId& bit_id) const;

libs/libfpgabitstream/src/bitstream_manager_utils.cpp

@@ -130,21 +130,4 @@ size_t rec_find_bitstream_manager_block_sum_of_bits(
   return sum_of_bits;
 }
 
-/********************************************************************
- * Split the bit path with delimiter ".". The blocks is reversed
- *******************************************************************/
-std::vector<std::string> reverse_split_bit_path_to_blocks(std::string path) {
-  std::vector<std::string> blocks;
-  size_t index = path.find_first_of(".");
-  while (index != std::string::npos) {
-    blocks.insert(blocks.begin(), path.substr(0, index));
-    path = path.substr(index + 1);
-    index = path.find_first_of(".");
-  }
-  if (path.size()) {
-    blocks.insert(blocks.begin(), path);
-  }
-  return blocks;
-}
-
 } /* end namespace openfpga */

libs/libfpgabitstream/src/bitstream_manager_utils.h

@@ -31,8 +31,6 @@ size_t find_bitstream_manager_config_bit_index_in_grandparent_block(
 size_t rec_find_bitstream_manager_block_sum_of_bits(
   const BitstreamManager& bitstream_manager, const ConfigBlockId& block);
 
-std::vector<std::string> reverse_split_bit_path_to_blocks(std::string path);
-
 } /* end namespace openfpga */
 
 #endif

openfpga/src/base/openfpga_bitstream_template.h

@@ -16,6 +16,7 @@
 #include "openfpga_digest.h"
 #include "openfpga_naming.h"
 #include "openfpga_reserved_words.h"
+#include "overwrite_bitstream.h"
 #include "read_xml_arch_bitstream.h"
 #include "report_bitstream_distribution.h"
 #include "vtr_log.h"
@@ -47,6 +48,10 @@ int fpga_bitstream_template(T& openfpga_ctx, const Command& cmd,
       g_vpr_ctx, openfpga_ctx, cmd_context.option_enable(cmd, opt_verbose));
   }
 
+  overwrite_bitstream(openfpga_ctx.mutable_bitstream_manager(),
+                      openfpga_ctx.bitstream_setting(),
+                      cmd_context.option_enable(cmd, opt_verbose));
+
   if (true == cmd_context.option_enable(cmd, opt_write_file)) {
     std::string src_dir_path =
       find_path_dir_name(cmd_context.option_value(cmd, opt_write_file));

openfpga/src/fpga_bitstream/build_device_bitstream.cpp

@@ -231,11 +231,6 @@ BitstreamManager build_device_bitstream(const VprContext& vpr_ctx,
     vpr_ctx.device().rr_graph, openfpga_ctx.device_rr_gsb(),
     openfpga_ctx.flow_manager().compress_routing(), verbose);
 
-  /* Apply path bit value */
-  for (auto bit : openfpga_ctx.bitstream_setting().path_bit_settings()) {
-    bitstream_manager.set_path_bit(bit.path, bit.value);
-  }
-
   VTR_LOGV(verbose, "Done\n");
 
   VTR_LOGV(verbose, "Decoded %lu configuration bits into %lu blocks\n",

openfpga/src/fpga_bitstream/overwrite_bitstream.cpp

@@ -0,0 +1,38 @@
+/********************************************************************
+ * This file includes functions to build bitstream from a mapped
+ * FPGA fabric.
+ * We decode the bitstream from configuration of routing multiplexers
+ * and Look-Up Tables (LUTs) which locate in CLBs and global routing
+ *architecture
+ *******************************************************************/
+
+/* Headers from vtrutil library */
+#include "overwrite_bitstream.h"
+
+#include "vtr_assert.h"
+#include "vtr_log.h"
+#include "vtr_time.h"
+
+/* begin namespace openfpga */
+namespace openfpga {
+
+/********************************************************************
+ * Overwrite bitstream retrieve from bitstream annotation XML which stored in
+ *BitstreamSetting
+ *******************************************************************/
+void overwrite_bitstream(openfpga::BitstreamManager& bitstream_manager,
+                         const openfpga::BitstreamSetting& bitstream_setting,
+                         const bool& verbose) {
+  vtr::ScopedStartFinishTimer timer("\nOverwrite Bitstream\n");
+
+  /* Apply overwrite_bitstream bit's path and value */
+  for (auto& id : bitstream_setting.overwrite_bitstreams()) {
+    std::string path = bitstream_setting.overwrite_bitstream_path(id);
+    bool value = bitstream_setting.overwrite_bitstream_value(id);
+    VTR_LOGV(verbose, "Overwrite bitstream path='%s' to value='%d'\n",
+             path.c_str(), value);
+    bitstream_manager.overwrite_bitstream(path, value);
+  }
+}
+
+} /* end namespace openfpga */

openfpga/src/fpga_bitstream/overwrite_bitstream.h

@@ -0,0 +1,23 @@
+#ifndef OVERWRITE_BITSTREAM_H
+#define OVERWRITE_BITSTREAM_H
+
+/********************************************************************
+ * Include header files that are required by function declaration
+ *******************************************************************/
+
+#include "openfpga_context.h"
+
+/********************************************************************
+ * Function declaration
+ *******************************************************************/
+
+/* begin namespace openfpga */
+namespace openfpga {
+
+void overwrite_bitstream(openfpga::BitstreamManager& bitstream_manager,
+                         const openfpga::BitstreamSetting& bitstream_setting,
+                         const bool& verbose);
+
+} /* end namespace openfpga */
+
+#endif