Merge pull request #144 from lnis-uofu/dev

Force the number of simulation clock cycles to be >= 2 to avoid false-positive self-testing in testbenches

openfpga/src/annotation/annotate_simulation_setting.cpp

@@ -5,6 +5,9 @@
 #include <cmath>
 #include <iterator>
 
+/* Headers from openfpgashell library */
+#include "command_exit_codes.h"
+
 /* Headers from vtrutil library */
 #include "vtr_assert.h"
 #include "vtr_log.h"
@@ -19,6 +22,8 @@
 /* begin namespace openfpga */
 namespace openfpga {
 
+constexpr int MIN_NUM_SIM_OPERATING_CLOCK_CYCLES = 2;
+
 /********************************************************************
  * Find the average signal density for all the nets of user's benchmark
  *******************************************************************/
@@ -164,6 +169,13 @@ size_t recommend_num_sim_clock_cycle(const AtomContext& atom_ctx,
   
   VTR_ASSERT(0 < recmd_num_clock_cycles);
 
+  /* Minimum number of clock cycles should be at least 2
+   * so that self-testing testbench can work!!!
+   */
+  if (MIN_NUM_SIM_OPERATING_CLOCK_CYCLES > recmd_num_clock_cycles) {
+    recmd_num_clock_cycles = MIN_NUM_SIM_OPERATING_CLOCK_CYCLES;
+  }
+
   return recmd_num_clock_cycles; 
 }
 
@@ -174,9 +186,9 @@ size_t recommend_num_sim_clock_cycle(const AtomContext& atom_ctx,
  *  - If the number of clock cycles in simulation is set to be automatically determined,
  *    we will infer the number based on the average signal density
  *******************************************************************/
-void annotate_simulation_setting(const AtomContext& atom_ctx, 
+int annotate_simulation_setting(const AtomContext& atom_ctx, 
-                                 const std::unordered_map<AtomNetId, t_net_power>& net_activity, 
+                                const std::unordered_map<AtomNetId, t_net_power>& net_activity, 
-                                 SimulationSetting& sim_setting) {
+                                SimulationSetting& sim_setting) {
 
   /* Find if the operating frequency is binded to vpr results */
   if (0. == sim_setting.operating_clock_frequency()) {
@@ -218,10 +230,22 @@ void annotate_simulation_setting(const AtomContext& atom_ctx,
                                                             net_activity, 
                                                             0.5);
     sim_setting.set_num_clock_cycles(num_clock_cycles);
-
-    VTR_LOG("Will apply %lu operating clock cycles to simulations\n",
-            sim_setting.num_clock_cycles());
   }
+
+  /* Minimum number of clock cycles should be at least 2
+   * so that self-testing testbench can work!!!
+   */
+  if (MIN_NUM_SIM_OPERATING_CLOCK_CYCLES > sim_setting.num_clock_cycles()) {
+    VTR_LOG_ERROR("Minimum number of operating clock cycles applicable to simulations is %d, while the specified number is %lu!\n",
+                  MIN_NUM_SIM_OPERATING_CLOCK_CYCLES,
+                  sim_setting.num_clock_cycles());
+    return CMD_EXEC_FATAL_ERROR;
+  }
+
+  VTR_LOG("Will apply %lu operating clock cycles to simulations\n",
+          sim_setting.num_clock_cycles());
+
+  return CMD_EXEC_SUCCESS;
 }
 
 } /* end namespace openfpga */

openfpga/src/annotation/annotate_simulation_setting.h

@@ -14,9 +14,9 @@
 /* begin namespace openfpga */
 namespace openfpga {
 
-void annotate_simulation_setting(const AtomContext& atom_ctx, 
+int annotate_simulation_setting(const AtomContext& atom_ctx, 
-                                 const std::unordered_map<AtomNetId, t_net_power>& net_activity, 
+                                const std::unordered_map<AtomNetId, t_net_power>& net_activity, 
-                                 SimulationSetting& sim_setting);
+                                SimulationSetting& sim_setting);
 
 } /* end namespace openfpga */
 

openfpga/src/base/openfpga_link_arch.cpp

@@ -159,9 +159,11 @@ int link_arch(OpenfpgaContext& openfpga_ctx,
    * TODO: This will be removed when openfpga flow is updated  
    */
   //openfpga_ctx.mutable_simulation_setting() = openfpga_ctx.mutable_arch().sim_setting;
-  annotate_simulation_setting(g_vpr_ctx.atom(),
+  if (CMD_EXEC_FATAL_ERROR == annotate_simulation_setting(g_vpr_ctx.atom(),
-                              openfpga_ctx.net_activity(),
+                                                          openfpga_ctx.net_activity(),
-                              openfpga_ctx.mutable_simulation_setting());
+                                                          openfpga_ctx.mutable_simulation_setting())) {
+    return CMD_EXEC_FATAL_ERROR;
+  }
 
   /* TODO: should identify the error code from internal function execution */
   return CMD_EXEC_SUCCESS;

openfpga/src/fpga_verilog/verilog_formal_random_top_testbench.cpp

@@ -250,10 +250,10 @@ void print_verilog_random_top_testbench(const std::string& circuit_name,
                                 clock_port_names,
                                 std::string(DEFAULT_CLOCK_NAME));
 
-  int simulation_time = find_operating_phase_simulation_time(MAGIC_NUMBER_FOR_SIMULATION_TIME,
+  float simulation_time = find_operating_phase_simulation_time(MAGIC_NUMBER_FOR_SIMULATION_TIME,
-                                                             simulation_parameters.num_clock_cycles(),
+                                                               simulation_parameters.num_clock_cycles(),
-                                                             1./simulation_parameters.operating_clock_frequency(),
+                                                               1./simulation_parameters.operating_clock_frequency(),
-                                                             VERILOG_SIM_TIMESCALE);
+                                                               VERILOG_SIM_TIMESCALE);
 
   /* Add Icarus requirement */
   print_verilog_timeout_and_vcd(fp, 

openfpga/src/fpga_verilog/verilog_testbench_utils.cpp

@@ -299,7 +299,7 @@ void print_verilog_timeout_and_vcd(std::fstream& fp,
                                    const std::string& vcd_fname,
                                    const std::string& simulation_start_counter_name,
                                    const std::string& error_counter_name,
-                                   const int& simulation_time) {
+                                   const float& simulation_time) {
   /* Validate the file stream */
   valid_file_stream(fp);
 
@@ -328,7 +328,7 @@ void print_verilog_timeout_and_vcd(std::fstream& fp,
   fp << "\t$timeformat(-9, 2, \"ns\", 20);" << std::endl;
   fp << "\t$display(\"Simulation start\");" << std::endl;
   print_verilog_comment(fp, std::string("----- Can be changed by the user for his/her need -------"));
-  fp << "\t#" << simulation_time << std::endl;
+  fp << "\t#" << std::setprecision(10) << simulation_time << std::endl;
   fp << "\tif(" << error_counter_name << " == 0) begin" << std::endl;
   fp << "\t\t$display(\"Simulation Succeed\");" << std::endl;
   fp << "\tend else begin" << std::endl;

openfpga/src/fpga_verilog/verilog_testbench_utils.h

@@ -58,7 +58,7 @@ void print_verilog_timeout_and_vcd(std::fstream& fp,
                                    const std::string& vcd_fname,
                                    const std::string& simulation_start_counter_name,
                                    const std::string& error_counter_name,
-                                   const int& simulation_time);
+                                   const float& simulation_time);
 
 BasicPort generate_verilog_testbench_clock_port(const std::vector<std::string>& clock_port_names,
                                                 const std::string& default_clock_name);

openfpga/src/fpga_verilog/verilog_top_testbench.cpp

@@ -1958,14 +1958,16 @@ void print_verilog_top_testbench(const ModuleManager& module_manager,
                                                       1./simulation_parameters.operating_clock_frequency());
 
 
-  /* Add Icarus requirement */
+  /* Add Icarus requirement: 
+   * Always ceil the simulation time so that we test a sufficient length of period!!!
+   */
   print_verilog_timeout_and_vcd(fp,
                                 std::string(ICARUS_SIMULATOR_FLAG),
                                 std::string(circuit_name + std::string(AUTOCHECK_TOP_TESTBENCH_VERILOG_MODULE_POSTFIX)),
                                 std::string(circuit_name + std::string("_formal.vcd")),
                                 std::string(TOP_TESTBENCH_SIM_START_PORT_NAME),
                                 std::string(TOP_TESTBENCH_ERROR_COUNTER),
-                                (int)simulation_time);
+                                std::ceil(simulation_time));
 
 
   /* Testbench ends*/

openfpga/src/utils/simulation_utils.cpp

@@ -14,14 +14,14 @@ namespace openfpga {
 /********************************************************************
  * Compute the time period for the simulation
  *******************************************************************/
-int find_operating_phase_simulation_time(const int& factor,
+float find_operating_phase_simulation_time(const int& factor,
-                                         const int& num_op_clock_cycles,
+                                           const int& num_op_clock_cycles,
-                                         const float& op_clock_period,
+                                           const float& op_clock_period,
-                                         const float& timescale) {
+                                           const float& timescale) {
   /* Take into account the prog_reset and reset cycles 
    * 1e9 is to change the unit to ns rather than second 
    */
-  return (factor * num_op_clock_cycles * op_clock_period) / timescale; 
+  return ((float)factor * (float)num_op_clock_cycles * op_clock_period) / timescale; 
 }
 
 /********************************************************************
@@ -37,8 +37,14 @@ float find_simulation_time_period(const float &time_unit,
                                   const float &op_clock_period) {
   float total_time_period = 0.;
 
-  /* Take into account the prog_reset and reset cycles */
+  /* Take into account 
-  total_time_period = (num_prog_clock_cycles + 2) * prog_clock_period + num_op_clock_cycles * op_clock_period;
+   * - the prog_reset
+   * - the gap clock cycle between programming and operating phase
+   * - 2 reset cycles before operating phase starts
+   * This is why the magic number 2 and 2 are added
+   */
+  total_time_period = ((float)num_prog_clock_cycles + 2) * prog_clock_period 
+                    + ((float)num_op_clock_cycles + 2) * op_clock_period;
   total_time_period = total_time_period / time_unit;
 
   return total_time_period;

openfpga/src/utils/simulation_utils.h

@@ -12,10 +12,10 @@
 /* begin namespace openfpga */
 namespace openfpga {
 
-int find_operating_phase_simulation_time(const int& factor,
+float find_operating_phase_simulation_time(const int& factor,
-                                         const int& num_op_clock_cycles,
+                                           const int& num_op_clock_cycles,
-                                         const float& op_clock_period,
+                                           const float& op_clock_period,
-                                         const float& timescale);
+                                           const float& timescale);
 
 float find_simulation_time_period(const float& time_unit,
                                   const int& num_prog_clock_cycles,