Merge branch 'master+wip' into equivalent-tiles

acomodi · acomodi · commit 90a87b931628 · 2019-06-24T12:54:38.000+02:00
Signed-off-by: Alessandro Comodi &lt;acomodi@antmicro.com&gt;
diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md
@@ -1,6 +1,9 @@
 ---
 name: Bug report
 about: Create a report to help us improve
+title: ''
+labels: ''
+assignees: ''
 
 ---
 
diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md
@@ -1,6 +1,9 @@
 ---
 name: Feature request
 about: Suggest an idea for this project
+title: ''
+labels: ''
+assignees: ''
 
 ---
 
diff --git a/.github/ISSUE_TEMPLATE/vtr-change.md b/.github/ISSUE_TEMPLATE/vtr-change.md
@@ -0,0 +1,25 @@
+---
+name: VTR change
+about: Describe purpose and lifecycle of a local change we made to VTR
+title: ''
+labels: ''
+assignees: ''
+
+---
+
+### Why did we need this? (what does this change enable us to do)
+<!--- i.e. what does this change enable us to do? -->
+
+### What did it change?
+<!--- i.e. technical description what the change does -->
+
+### Should it be merged upstream - if not, when can we delete it?
+
+### What is needed to get this merged / deleted? 
+
+* [ ] is the implementation work to make suitable for merging / deletion completed?
+* [ ] Is there an associated test? <!--- i.e. how will we prevent it from regressing? -->
+* [ ] is this currently part of the Conda package?
+* [ ] is this properly cleaned up in our local repositories? <!--- add subtasks here if needed) -->
+
+### Tracker / branch / PR & other useful links
diff --git a/README.md b/README.md
@@ -1,5 +1,14 @@
+
+SymbiFlow WIP changes for Verilog to Routing (VTR)
+==================================================
+
+This branch contains work in progress changes for using Verilog to Routing
+(VTR) as part of SymbiFlow.
+
+---
+
 # Verilog to Routing (VTR)
-[![Build Status](https://travis-ci.org/verilog-to-routing/vtr-verilog-to-routing.svg?branch=master)](https://travis-ci.org/verilog-to-routing/vtr-verilog-to-routing) [![Documentation Status](https://readthedocs.org/projects/vtr/badge/?version=latest)](http://docs.verilogtorouting.org/en/latest/?badge=latest)
+[![Build Status](https://travis-ci.com/SymbiFlow/vtr-verilog-to-routing.svg?branch=master)](https://travis-ci.com/SymbiFlow/vtr-verilog-to-routing) [![Documentation Status](https://readthedocs.org/projects/vtr/badge/?version=latest)](http://docs.verilogtorouting.org/en/latest/?badge=latest)
 
 ## Introduction
 The Verilog-to-Routing (VTR) project is a world-wide collaborative effort to provide a open-source framework for conducting FPGA architecture and CAD research and development.
diff --git a/libs/libarchfpga/src/physical_types.h b/libs/libarchfpga/src/physical_types.h
@@ -1254,6 +1254,7 @@ enum class BufferSize {
  * R:  Equivalent resistance of the buffer/switch.                           *
  * Cin:  Input capacitance.                                                  *
  * Cout:  Output capacitance.                                                *
+ * Cinternal: Internal capacitance in a buffer with fanout.                  *
  * Tdel_map: A map where the key is the number of inputs and the entry       *
  *           is the corresponding delay. If there is only one entry at key   *
  *           UNDEFINED, then delay is a constant (doesn't vary with fan-in). *
@@ -1271,6 +1272,7 @@ struct t_arch_switch_inf {
     float R = 0.;
     float Cin = 0.;
     float Cout = 0.;
+    float Cinternal = 0.; // defined the property Cinternal
     float mux_trans_size = 1.;
     BufferSize buf_size_type = BufferSize::AUTO;
     float buf_size = 0.;
@@ -1322,6 +1324,7 @@ struct t_arch_switch_inf {
  * R:  Equivalent resistance of the buffer/switch.                           *
  * Cin:  Input capacitance.                                                  *
  * Cout:  Output capacitance.                                                *
+ * Cinternal: Internal capacitance in a buffer.                              *
  * Tdel:  Intrinsic delay.  The delay through an unloaded switch is          *
  *        Tdel + R * Cout.                                                   *
  * mux_trans_size:  The area of each transistor in the segment's driving mux *
@@ -1332,6 +1335,7 @@ struct t_rr_switch_inf {
     float R = 0.;
     float Cin = 0.;
     float Cout = 0.;
+    float Cinternal = 0.; //defined the property Cinternal
     float Tdel = 0.;
     float mux_trans_size = 0.;
     float buf_size = 0.;
diff --git a/libs/libarchfpga/src/read_xml_arch_file.cpp b/libs/libarchfpga/src/read_xml_arch_file.cpp
@@ -3284,23 +3284,23 @@ static void ProcessSwitches(pugi::xml_node Parent,
         SwitchType type = SwitchType::MUX;
         if (0 == strcmp(type_name, "mux")) {
             type = SwitchType::MUX;
-            expect_only_attributes(Node, {"type", "name", "R", "Cin", "Cout", "Tdel", "buf_size", "power_buf_size", "mux_trans_size"}, " with type '"s + type_name + "'"s, loc_data);
+            expect_only_attributes(Node, {"type", "name", "R", "Cin", "Cout", "Cinternal", "Tdel", "buf_size", "power_buf_size", "mux_trans_size"}, " with type '"s + type_name + "'"s, loc_data); // buffered switch should have a Cinternal element
 
         } else if (0 == strcmp(type_name, "tristate")) {
             type = SwitchType::TRISTATE;
-            expect_only_attributes(Node, {"type", "name", "R", "Cin", "Cout", "Tdel", "buf_size", "power_buf_size"}, " with type '"s + type_name + "'"s, loc_data);
+            expect_only_attributes(Node, {"type", "name", "R", "Cin", "Cout", "Cinternal", "Tdel", "buf_size", "power_buf_size"}, " with type '"s + type_name + "'"s, loc_data); // buffered switch should have a Cinternal element
 
         } else if (0 == strcmp(type_name, "buffer")) {
             type = SwitchType::BUFFER;
-            expect_only_attributes(Node, {"type", "name", "R", "Cin", "Cout", "Tdel", "buf_size", "power_buf_size"}, " with type '"s + type_name + "'"s, loc_data);
+            expect_only_attributes(Node, {"type", "name", "R", "Cin", "Cout", "Tdel", "buf_size", "power_buf_size"}, " with type '"s + type_name + "'"s, loc_data); // buffer should not have a Cinternal element
 
         } else if (0 == strcmp(type_name, "pass_gate")) {
             type = SwitchType::PASS_GATE;
-            expect_only_attributes(Node, {"type", "name", "R", "Cin", "Cout", "Tdel"}, " with type '"s + type_name + "'"s, loc_data);
+            expect_only_attributes(Node, {"type", "name", "R", "Cin", "Cout", "Tdel"}, " with type '"s + type_name + "'"s, loc_data); // unbuffered switch does not have Cinternal element
 
         } else if (0 == strcmp(type_name, "short")) {
             type = SwitchType::SHORT;
-            expect_only_attributes(Node, {"type", "name", "R", "Cin", "Cout", "Tdel"}, " with type "s + type_name + "'"s, loc_data);
+            expect_only_attributes(Node, {"type", "name", "R", "Cin", "Cout", "Tdel"}, " with type "s + type_name + "'"s, loc_data); // unbuffered switch does not have Cinternal element
 
         } else {
             archfpga_throw(loc_data.filename_c_str(), loc_data.line(Node),
@@ -3312,13 +3312,16 @@ static void ProcessSwitches(pugi::xml_node Parent,
 
         ReqOpt COUT_REQD = TIMING_ENABLE_REQD;
         ReqOpt CIN_REQD = TIMING_ENABLE_REQD;
+        ReqOpt CINTERNAL_REQD = OPTIONAL; //defined the parameter
+
         if (arch_switch.type() == SwitchType::SHORT) {
             //Cin/Cout are optional on shorts, since they really only have one capacitance
             CIN_REQD = OPTIONAL;
             COUT_REQD = OPTIONAL;
         }
         arch_switch.Cin = get_attribute(Node, "Cin", loc_data, CIN_REQD).as_float(0);
         arch_switch.Cout = get_attribute(Node, "Cout", loc_data, COUT_REQD).as_float(0);
+        arch_switch.Cinternal = get_attribute(Node, "Cinternal", loc_data, CINTERNAL_REQD).as_float(0); // retrieve the optional parameter
 
         if (arch_switch.type() == SwitchType::MUX) {
             //Only muxes have mux transistors
diff --git a/vpr/src/base/SetupVPR.cpp b/vpr/src/base/SetupVPR.cpp
@@ -347,8 +347,8 @@ static void SetupRouterOpts(const t_options& Options, t_router_opts* RouterOpts)
     RouterOpts->max_convergence_count = Options.router_max_convergence_count;
     RouterOpts->reconvergence_cpd_threshold = Options.router_reconvergence_cpd_threshold;
     RouterOpts->first_iteration_timing_report_file = Options.router_first_iteration_timing_report_file;
-
     RouterOpts->strict_checks = Options.strict_checks;
+    RouterOpts->disable_check_route = Options.disable_check_route;
 }
 
 static void SetupAnnealSched(const t_options& Options,
diff --git a/vpr/src/base/read_options.cpp b/vpr/src/base/read_options.cpp
@@ -931,6 +931,16 @@ static argparse::ArgumentParser create_arg_parser(std::string prog_name, t_optio
         .default_value("on")
         .show_in(argparse::ShowIn::HELP_ONLY);
 
+    gen_grp.add_argument<bool, ParseOnOff>(args.allow_dangling_combinational_nodes, "--allow_dangling_combinational_nodes")
+        .help(
+            "Option to allow dangling combinational nodes in the timing graph.\n"
+            "This option should normally be off, as dangling combinational nodes are unusual\n"
+            "in the timing graph and may indicate a problem in the circuit or architecture.\n"
+            "Unless you understand why your architecture/circuit can have valid dangling combinational nodes, this option should be off.\n"
+            "In general this is a dev-only option and should not be turned on by the end-user.")
+        .default_value("off")
+        .show_in(argparse::ShowIn::HELP_ONLY);
+
     auto& file_grp = parser.add_argument_group("file options");
 
     file_grp.add_argument(args.BlifFile, "--circuit_file")
@@ -1533,6 +1543,11 @@ static argparse::ArgumentParser create_arg_parser(std::string prog_name, t_optio
         .default_value("")
         .show_in(argparse::ShowIn::HELP_ONLY);
 
+    route_timing_grp.add_argument<bool, ParseOnOff>(args.disable_check_route, "--disable_check_route")
+        .help("Disables check_route once routing step has finished or when routing file is loaded")
+        .default_value("off")
+        .show_in(argparse::ShowIn::HELP_ONLY);
+
     route_timing_grp.add_argument(args.router_debug_net, "--router_debug_net")
         .help(
             "Controls when router debugging is enabled.\n"
diff --git a/vpr/src/base/read_options.h b/vpr/src/base/read_options.h
@@ -50,6 +50,7 @@ struct t_options {
     argparse::ArgValue<e_clock_modeling> clock_modeling;
     argparse::ArgValue<bool> exit_before_pack;
     argparse::ArgValue<bool> strict_checks;
+    argparse::ArgValue<bool> allow_dangling_combinational_nodes;
 
     /* Atom netlist options */
     argparse::ArgValue<bool> absorb_buffer_luts;
@@ -118,6 +119,7 @@ struct t_options {
     argparse::ArgValue<bool> verify_binary_search;
     argparse::ArgValue<e_router_algorithm> RouterAlgorithm;
     argparse::ArgValue<int> min_incremental_reroute_fanout;
+    argparse::ArgValue<bool> disable_check_route;
 
     /* Timing-driven router options only */
     argparse::ArgValue<float> astar_fac;
diff --git a/vpr/src/base/vpr_api.cpp b/vpr/src/base/vpr_api.cpp
@@ -274,7 +274,7 @@ void vpr_init(const int argc, const char** argv, t_options* options, t_vpr_setup
         auto& timing_ctx = g_vpr_ctx.mutable_timing();
         {
             vtr::ScopedStartFinishTimer t("Build Timing Graph");
-            timing_ctx.graph = TimingGraphBuilder(atom_ctx.nlist, atom_ctx.lookup).timing_graph();
+            timing_ctx.graph = TimingGraphBuilder(atom_ctx.nlist, atom_ctx.lookup).timing_graph(options->allow_dangling_combinational_nodes);
             VTR_LOG("  Timing Graph Nodes: %zu\n", timing_ctx.graph->nodes().size());
             VTR_LOG("  Timing Graph Edges: %zu\n", timing_ctx.graph->edges().size());
             VTR_LOG("  Timing Graph Levels: %zu\n", timing_ctx.graph->levels().size());
@@ -636,7 +636,9 @@ RouteStatus vpr_route_flow(t_vpr_setup& vpr_setup, const t_arch& arch) {
         std::string graphics_msg;
         if (route_status.success()) {
             //Sanity check the routing
-            check_route(router_opts.route_type);
+            if (!router_opts.disable_check_route) {
+                check_route(router_opts.route_type);
+            }
             get_serial_num();
 
             //Update status
diff --git a/vpr/src/base/vpr_types.h b/vpr/src/base/vpr_types.h
@@ -947,6 +947,7 @@ struct t_router_opts {
     float reconvergence_cpd_threshold;
     std::string first_iteration_timing_report_file;
     bool strict_checks;
+    bool disable_check_route;
 };
 
 struct t_analysis_opts {
diff --git a/vpr/src/place/place.cpp b/vpr/src/place/place.cpp
@@ -2118,13 +2118,12 @@ static bool find_to(t_type_ptr to_type, t_type_ptr from_type, float rlim, const
         //The candidates are stored in a flat_map so we can efficiently find the set of valid
         //candidates with upper/lower bound.
         auto y_lower_iter = to_compressed_block_grid.grid[cx_to].lower_bound(min_cy);
-        auto y_upper_iter = to_compressed_block_grid.grid[cx_to].upper_bound(max_cy);
-
-        // If no block was found at this x location continue
         if (y_lower_iter == to_compressed_block_grid.grid[cx_to].end()) {
             continue;
         }
 
+        auto y_upper_iter = to_compressed_block_grid.grid[cx_to].upper_bound(max_cy);
+
         if (y_lower_iter->first > min_cy) {
             //No valid blocks at this x location which are within rlim_y
             //
@@ -3585,9 +3584,8 @@ static int check_placement_costs(const t_placer_costs& costs,
         comp_td_costs(delay_model, &timing_cost_check);
         //VTR_LOG("timing_cost recomputed from scratch: %g\n", timing_cost_check);
         if (fabs(timing_cost_check - costs.timing_cost) > costs.timing_cost * ERROR_TOL) {
-            VTR_LOG_ERROR("timing_cost_check: %g and timing_cost: %g differ in check_place.\n",
-                          timing_cost_check, costs.timing_cost);
-            error++;
+            VTR_LOG_WARN("timing_cost_check: %g and timing_cost: %g differ in check_place.\n",
+                         timing_cost_check, costs.timing_cost);
         }
     }
     return error;
diff --git a/vpr/src/route/check_route.cpp b/vpr/src/route/check_route.cpp
@@ -118,13 +118,13 @@ void check_route(enum e_route_type route_type) {
             } else { //Continuing along existing branch
                 connects = check_adjacent(prev_node, inode);
                 if (!connects) {
-                    vpr_throw(VPR_ERROR_ROUTE, __FILE__, __LINE__,
-                              "in check_route: found non-adjacent segments in traceback while checking net %d:\n"
-                              "  %s\n"
-                              "  %s\n",
-                              size_t(net_id),
-                              describe_rr_node(prev_node).c_str(),
-                              describe_rr_node(inode).c_str());
+                    VTR_LOG_WARN(
+                        "in check_route: found non-adjacent segments in traceback while checking net %d:\n"
+                        "  %s\n"
+                        "  %s\n",
+                        size_t(net_id),
+                        describe_rr_node(prev_node).c_str(),
+                        describe_rr_node(inode).c_str());
                 }
 
                 connected_to_route[inode] = true; /* Mark as in path. */
diff --git a/vpr/src/route/check_rr_graph.cpp b/vpr/src/route/check_rr_graph.cpp
@@ -502,11 +502,11 @@ static void check_unbuffered_edges(int from_node) {
         }
 
         if (trans_matched == false) {
-            vpr_throw(VPR_ERROR_ROUTE, __FILE__, __LINE__,
-                      "in check_unbuffered_edges:\n"
-                      "connection from node %d to node %d uses an unbuffered switch (switch type %d '%s')\n"
-                      "but there is no corresponding unbuffered switch edge in the other direction.\n",
-                      from_node, to_node, from_switch_type, device_ctx.rr_switch_inf[from_switch_type].name);
+            VTR_LOG_WARN(
+                "in check_unbuffered_edges:\n"
+                "connection from node %d to node %d uses an unbuffered switch (switch type %d '%s')\n"
+                "but there is no corresponding unbuffered switch edge in the other direction.\n",
+                from_node, to_node, from_switch_type, device_ctx.rr_switch_inf[from_switch_type].name);
         }
 
     } /* End for all from_node edges */
diff --git a/vpr/src/route/rr_graph.cpp b/vpr/src/route/rr_graph.cpp
@@ -893,6 +893,7 @@ void load_rr_switch_from_arch_switch(int arch_switch_idx,
     device_ctx.rr_switch_inf[rr_switch_idx].set_type(device_ctx.arch_switch_inf[arch_switch_idx].type());
     device_ctx.rr_switch_inf[rr_switch_idx].R = device_ctx.arch_switch_inf[arch_switch_idx].R;
     device_ctx.rr_switch_inf[rr_switch_idx].Cin = device_ctx.arch_switch_inf[arch_switch_idx].Cin;
+    device_ctx.rr_switch_inf[rr_switch_idx].Cinternal = device_ctx.arch_switch_inf[arch_switch_idx].Cinternal; //now we can retrieve Cinternal from the arch and implement into the rr calculations.
     device_ctx.rr_switch_inf[rr_switch_idx].Cout = device_ctx.arch_switch_inf[arch_switch_idx].Cout;
     device_ctx.rr_switch_inf[rr_switch_idx].Tdel = rr_switch_Tdel;
     device_ctx.rr_switch_inf[rr_switch_idx].mux_trans_size = device_ctx.arch_switch_inf[arch_switch_idx].mux_trans_size;
diff --git a/vpr/src/route/rr_graph_reader.cpp b/vpr/src/route/rr_graph_reader.cpp
@@ -234,11 +234,13 @@ void process_switches(pugi::xml_node parent, const pugiutil::loc_data& loc_data)
             rr_switch.R = get_attribute(SwitchSubnode, "R", loc_data).as_float();
             rr_switch.Cin = get_attribute(SwitchSubnode, "Cin", loc_data).as_float();
             rr_switch.Cout = get_attribute(SwitchSubnode, "Cout", loc_data).as_float();
+            rr_switch.Cinternal = get_attribute(SwitchSubnode, "Cinternal", loc_data).as_float();
             rr_switch.Tdel = get_attribute(SwitchSubnode, "Tdel", loc_data).as_float();
         } else {
             rr_switch.R = 0;
             rr_switch.Cin = 0;
             rr_switch.Cout = 0;
+            rr_switch.Cinternal = 0;
             rr_switch.Tdel = 0;
         }
         SwitchSubnode = get_single_child(Switch, "sizing", loc_data);
diff --git a/vpr/src/route/rr_graph_writer.cpp b/vpr/src/route/rr_graph_writer.cpp
@@ -189,7 +189,8 @@ void write_rr_switches(fstream& fp) {
         }
         fp << ">" << endl;
 
-        fp << "\t\t\t<timing R=\"" << setprecision(FLOAT_PRECISION) << rr_switch.R << "\" Cin=\"" << setprecision(FLOAT_PRECISION) << rr_switch.Cin << "\" Cout=\"" << setprecision(FLOAT_PRECISION) << rr_switch.Cout << "\" Tdel=\"" << setprecision(FLOAT_PRECISION) << rr_switch.Tdel << "\"/>" << endl;
+        fp << "\t\t\t<timing R=\"" << setprecision(FLOAT_PRECISION) << rr_switch.R << "\" Cin=\"" << setprecision(FLOAT_PRECISION) << rr_switch.Cin << "\" Cout=\"" << setprecision(FLOAT_PRECISION) << rr_switch.Cout << "\" Cinternal=\"" << setprecision(FLOAT_PRECISION) << rr_switch.Cinternal << //will print display the value of Cinternal
+            "\" Tdel=\"" << setprecision(FLOAT_PRECISION) << rr_switch.Tdel << "\"/>" << endl;
         fp << "\t\t\t<sizing mux_trans_size=\"" << setprecision(FLOAT_PRECISION) << rr_switch.mux_trans_size << "\" buf_size=\"" << setprecision(FLOAT_PRECISION) << rr_switch.buf_size << "\"/>" << endl;
         fp << "\t\t</switch>" << endl;
     }
diff --git a/vpr/src/timing/timing_graph_builder.cpp b/vpr/src/timing/timing_graph_builder.cpp
@@ -40,8 +40,8 @@ TimingGraphBuilder::TimingGraphBuilder(const AtomNetlist& netlist,
     //pass
 }
 
-std::unique_ptr<TimingGraph> TimingGraphBuilder::timing_graph() {
-    build();
+std::unique_ptr<TimingGraph> TimingGraphBuilder::timing_graph(bool allow_dangling_combinational_nodes) {
+    build(allow_dangling_combinational_nodes);
     opt_memory_layout();
 
     VTR_ASSERT(tg_);
@@ -50,9 +50,13 @@ std::unique_ptr<TimingGraph> TimingGraphBuilder::timing_graph() {
     return std::move(tg_);
 }
 
-void TimingGraphBuilder::build() {
+void TimingGraphBuilder::build(bool allow_dangling_combinational_nodes) {
     tg_ = std::make_unique<tatum::TimingGraph>();
 
+    // Optionally allow dangling combinational nodes.
+    // Set by `--allow_dangling_combinational_nodes on`. Default value is false
+    tg_->set_allow_dangling_combinational_nodes(allow_dangling_combinational_nodes);
+
     for (AtomBlockId blk : netlist_.blocks()) {
         AtomBlockType blk_type = netlist_.block_type(blk);
 
diff --git a/vpr/src/timing/timing_graph_builder.h b/vpr/src/timing/timing_graph_builder.h
@@ -10,10 +10,10 @@ class TimingGraphBuilder {
     TimingGraphBuilder(const AtomNetlist& netlist,
                        AtomLookup& netlist_lookup);
 
-    std::unique_ptr<tatum::TimingGraph> timing_graph();
+    std::unique_ptr<tatum::TimingGraph> timing_graph(bool allow_dangling_combinational_nodes);
 
   private:
-    void build();
+    void build(bool allow_dangling_combinational_nodes);
     void opt_memory_layout();
 
     void add_io_to_timing_graph(const AtomBlockId blk);
diff --git a/vpr/src/timing/timing_util.cpp b/vpr/src/timing/timing_util.cpp
@@ -571,6 +571,10 @@ float calc_relaxed_criticality(const std::map<DomainPair, float>& domains_max_re
             max_req += shift;
         }
 
+        if (!std::isfinite(slack)) {
+            continue;
+        }
+
         float crit = std::numeric_limits<float>::quiet_NaN();
         if (max_req > 0.) {
             //Standard case

Original file line number	Diff line number	Diff line change
`@@ -347,8 +347,8 @@ static void SetupRouterOpts(const t_options& Options, t_router_opts* RouterOpts)`
`347`	`347`	`RouterOpts->max_convergence_count = Options.router_max_convergence_count;`
`348`	`348`	`RouterOpts->reconvergence_cpd_threshold = Options.router_reconvergence_cpd_threshold;`
`349`	`349`	`RouterOpts->first_iteration_timing_report_file = Options.router_first_iteration_timing_report_file;`
`350`		`-`
`351`	`350`	`RouterOpts->strict_checks = Options.strict_checks;`
	`351`	`+ RouterOpts->disable_check_route = Options.disable_check_route;`
`352`	`352`	`}`
`353`	`353`
`354`	`354`	`static void SetupAnnealSched(const t_options& Options,`