timing: Move towards DelayPairs for timing reporting (#1359)

common/kernel/nextpnr_types.h

@@ -80,7 +80,7 @@ struct PortRef
 // minimum and maximum delay
 struct DelayPair
 {
-    DelayPair(){};
+    DelayPair() : min_delay(0), max_delay(0) {};
     explicit DelayPair(delay_t delay) : min_delay(delay), max_delay(delay) {}
     DelayPair(delay_t min_delay, delay_t max_delay) : min_delay(min_delay), max_delay(max_delay) {}
     delay_t minDelay() const { return min_delay; }
@@ -94,13 +94,25 @@ struct DelayPair
     {
         return {min_delay - other.min_delay, max_delay - other.max_delay};
     }
+    DelayPair &operator+=(const DelayPair &rhs)
+    {
+        min_delay += rhs.min_delay;
+        max_delay += rhs.max_delay;
+        return *this;
+    }
+    DelayPair &operator-=(const DelayPair &rhs)
+    {
+        min_delay -= rhs.min_delay;
+        max_delay -= rhs.max_delay;
+        return *this;
+    }
 };
 
 // four-quadrant, min and max rise and fall delay
 struct DelayQuad
 {
     DelayPair rise, fall;
-    DelayQuad() {}
+    DelayQuad() : rise(0), fall(0) {}
     explicit DelayQuad(delay_t delay) : rise(delay), fall(delay) {}
     DelayQuad(delay_t min_delay, delay_t max_delay) : rise(min_delay, max_delay), fall(min_delay, max_delay) {}
     DelayQuad(DelayPair rise, DelayPair fall) : rise(rise), fall(fall) {}
@@ -120,6 +132,19 @@ struct DelayQuad
 
     DelayQuad operator+(const DelayQuad &other) const { return {rise + other.rise, fall + other.fall}; }
     DelayQuad operator-(const DelayQuad &other) const { return {rise - other.rise, fall - other.fall}; }
+    DelayQuad &operator+=(const DelayQuad &rhs)
+    {
+        rise += rhs.rise;
+        fall += rhs.fall;
+        return *this;
+    }
+
+    DelayQuad &operator-=(const DelayQuad &rhs)
+    {
+        rise -= rhs.rise;
+        fall -= rhs.fall;
+        return *this;
+    }
 };
 
 struct ClockConstraint;
@@ -336,15 +361,18 @@ struct CriticalPath
         // To cell.port
         std::pair<IdString, IdString> to;
         // Segment delay
-        delay_t delay;
+        DelayPair delay;
     };
 
     // Clock pair
     ClockPair clock_pair;
     // Total path delay
-    delay_t delay;
+    DelayPair delay;
-    // Period (max allowed delay)
+
-    delay_t period;
+    // if delay.minDelay() < bound.minDelay() then this is a hold violation
+    // if delay.maxDelay() > bound.maxDelay() then this is a setup violation
+    DelayPair bound;
+
     // Individual path segments
     std::vector<Segment> segments;
 };
@@ -357,7 +385,7 @@ struct NetSinkTiming
     // Cell and port (the sink)
     std::pair<IdString, IdString> cell_port;
     // Delay
-    delay_t delay;
+    DelayPair delay;
 };
 
 struct TimingResult
@@ -379,6 +407,9 @@ struct TimingResult
 
     // Histogram of slack
     dict<int, unsigned> slack_histogram;
+
+    // TODO: Hold time violations
+    // dict<IdString, CriticalPath> hold_violations;
 };
 
 // Represents the contents of a non-leaf cell in a design

common/kernel/report.cc

@@ -73,11 +73,11 @@ static Json::array json_report_critical_paths(const Context *ctx)
                                         {"port", segment.to.second.c_str(ctx)},
                                         {"loc", Json::array({toLoc.x, toLoc.y})}});
 
-            auto segmentJson = Json::object({
+            auto minDelay = ctx->getDelayNS(segment.delay.minDelay());
-                    {"delay", ctx->getDelayNS(segment.delay)},
+            auto maxDelay = ctx->getDelayNS(segment.delay.maxDelay());
-                    {"from", fromJson},
+
-                    {"to", toJson},
+            auto segmentJson =
-            });
+                    Json::object({{"delay", Json::array({minDelay, maxDelay})}, {"from", fromJson}, {"to", toJson}});
 
             if (segment.type == CriticalPath::Segment::Type::CLK_TO_Q) {
                 segmentJson["type"] = "clk-to-q";
@@ -130,10 +130,13 @@ static Json::array json_report_detailed_net_timings(const Context *ctx)
 
         Json::array endpointsJson;
         for (const auto &sink_timing : it.second) {
+            auto minDelay = ctx->getDelayNS(sink_timing.delay.minDelay());
+            auto maxDelay = ctx->getDelayNS(sink_timing.delay.maxDelay());
+
             auto endpointJson = Json::object({{"cell", sink_timing.cell_port.first.c_str(ctx)},
                                               {"port", sink_timing.cell_port.second.c_str(ctx)},
                                               {"event", clock_event_name(ctx, sink_timing.clock_pair.end)},
-                                              {"delay", ctx->getDelayNS(sink_timing.delay)}});
+                                              {"delay", Json::array({minDelay, maxDelay})}});
             endpointsJson.push_back(endpointJson);
         }
 
@@ -191,7 +194,10 @@ Report JSON structure:
           },
           "type": <path segment type "clk-to-q", "source", "logic", "routing" or "setup">,
           "net": <net name (for routing only!)>,
-          "delay": <segment delay [ns]>,
+          "delay": [
+            <minimum segment delay [ns]>,
+            <maximum segment delay [ns]>,
+          ],
         }
         ...
       ]
@@ -209,7 +215,10 @@ Report JSON structure:
           "cell": <sink cell name>,
           "port": <sink cell port name>,
           "event": <destination clock event name>,
-          "delay": <delay [ns]>,
+          "delay": [
+            <minimum segment delay [ns]>,
+            <maximum segment delay [ns]>,
+          ],
         }
         ...
       ]

common/kernel/timing.cc

@@ -502,6 +502,8 @@ void TimingAnalyser::identify_related_domains()
 
 void TimingAnalyser::reset_times()
 {
+    static const auto init_delay =
+            DelayPair(std::numeric_limits<delay_t>::max(), std::numeric_limits<delay_t>::lowest());
     for (auto &port : ports) {
         auto do_reset = [&](dict<domain_id_t, ArrivReqTime> &times) {
             for (auto &t : times) {
@@ -758,7 +760,7 @@ void TimingAnalyser::build_detailed_net_timing_report()
                     sink_timing.clock_pair.end.clock = capture.clock;
                     sink_timing.clock_pair.end.edge = capture.edge;
                     sink_timing.cell_port = std::make_pair(pd.cell_port.cell, pd.cell_port.port);
-                    sink_timing.delay = arr.second.value.max_delay;
+                    sink_timing.delay = arr.second.value;
 
                     net_timings[net->name].push_back(sink_timing);
                 }
@@ -802,23 +804,25 @@ CriticalPath TimingAnalyser::build_critical_path_report(domain_id_t domain_pair,
     auto &launch = domains.at(dp.key.launch).key;
     auto &capture = domains.at(dp.key.capture).key;
 
+    report.delay = DelayPair(0);
+
     report.clock_pair.start.clock = launch.clock;
     report.clock_pair.start.edge = launch.edge;
     report.clock_pair.end.clock = capture.clock;
     report.clock_pair.end.edge = capture.edge;
 
-    report.period = ctx->getDelayFromNS(1.0e9 / ctx->setting<float>("target_freq"));
+    report.bound = DelayPair(0, ctx->getDelayFromNS(1.0e9 / ctx->setting<float>("target_freq")));
     if (launch.edge != capture.edge) {
-        report.period = report.period / 2;
+        report.bound.max_delay = report.bound.max_delay / 2;
     }
 
     if (!launch.is_async() && ctx->nets.at(launch.clock)->clkconstr) {
         if (launch.edge == capture.edge) {
-            report.period = ctx->nets.at(launch.clock)->clkconstr->period.minDelay();
+            report.bound.max_delay = ctx->nets.at(launch.clock)->clkconstr->period.minDelay();
         } else if (capture.edge == RISING_EDGE) {
-            report.period = ctx->nets.at(launch.clock)->clkconstr->low.minDelay();
+            report.bound.max_delay = ctx->nets.at(launch.clock)->clkconstr->low.minDelay();
         } else if (capture.edge == FALLING_EDGE) {
-            report.period = ctx->nets.at(launch.clock)->clkconstr->high.minDelay();
+            report.bound.max_delay = ctx->nets.at(launch.clock)->clkconstr->high.minDelay();
         }
     }
 
@@ -895,13 +899,13 @@ CriticalPath TimingAnalyser::build_critical_path_report(domain_id_t domain_pair,
             seg_logic.type = CriticalPath::Segment::Type::LOGIC;
         }
 
-        seg_logic.delay = comb_delay.maxDelay();
+        seg_logic.delay = comb_delay.delayPair();
         seg_logic.from = std::make_pair(last_cell->name, last_port);
         seg_logic.to = std::make_pair(driver_cell->name, driver.port);
         seg_logic.net = IdString();
         report.segments.push_back(seg_logic);
 
-        auto net_delay = ctx->getNetinfoRouteDelay(net, sink);
+        auto net_delay = DelayPair(ctx->getNetinfoRouteDelay(net, sink));
 
         CriticalPath::Segment seg_route;
         seg_route.type = CriticalPath::Segment::Type::ROUTING;
@@ -919,7 +923,7 @@ CriticalPath TimingAnalyser::build_critical_path_report(domain_id_t domain_pair,
     auto sinkClass = ctx->getPortTimingClass(crit_path.back().cell, crit_path.back().port, clockCount);
     if (sinkClass == TMG_REGISTER_INPUT && clockCount > 0) {
         auto sinkClockInfo = ctx->getPortClockingInfo(crit_path.back().cell, crit_path.back().port, 0);
-        delay_t setup = sinkClockInfo.setup.maxDelay();
+        auto setup = sinkClockInfo.setup;
 
         CriticalPath::Segment seg_logic;
         seg_logic.type = CriticalPath::Segment::Type::SETUP;

common/kernel/timing.h

@@ -128,8 +128,6 @@ struct TimingAnalyser
     // get the N worst endpoints for a given domain pair
     std::vector<CellPortKey> get_worst_eps(domain_id_t domain_pair, int count);
 
-    const DelayPair init_delay{std::numeric_limits<delay_t>::max(), std::numeric_limits<delay_t>::lowest()};
-
     // Set arrival/required times if more/less than the current value
     void set_arrival_time(CellPortKey target, domain_id_t domain, DelayPair arrival, int path_length,
                           CellPortKey prev = CellPortKey());

common/kernel/timing_log.cc

@@ -68,7 +68,18 @@ static void log_crit_paths(const Context *ctx, TimingResult &result)
 
     // A helper function for reporting one critical path
     auto print_path_report = [ctx](const CriticalPath &path) {
-        delay_t total = 0, logic_total = 0, route_total = 0;
+        DelayPair total(0), logic_total(0), route_total(0);
+
+        // We print out the max delay since that's usually the interesting case
+        // But if we know this critical path has violated hold time we print the
+        // min delay instead
+        bool hold_violation = path.delay.minDelay() < path.bound.minDelay();
+        auto get_delay_ns = [hold_violation, ctx](const DelayPair &d) {
+            if (hold_violation) {
+                ctx->getDelayNS(d.minDelay());
+            }
+            return ctx->getDelayNS(d.maxDelay());
+        };
 
         log_info("curr total\n");
         for (const auto &segment : path.segments) {
@@ -83,10 +94,10 @@ static void log_crit_paths(const Context *ctx, TimingResult &result)
 
                 const std::string type_name = (segment.type == CriticalPath::Segment::Type::SETUP) ? "Setup" : "Source";
 
-                log_info("%4.1f %4.1f  %s %s.%s\n", ctx->getDelayNS(segment.delay), ctx->getDelayNS(total),
+                log_info("%4.1f %4.1f  %s %s.%s\n", get_delay_ns(segment.delay), get_delay_ns(total), type_name.c_str(),
-                         type_name.c_str(), segment.to.first.c_str(ctx), segment.to.second.c_str(ctx));
+                         segment.to.first.c_str(ctx), segment.to.second.c_str(ctx));
             } else if (segment.type == CriticalPath::Segment::Type::ROUTING) {
-                route_total += segment.delay;
+                route_total = route_total + segment.delay;
 
                 const auto &driver = ctx->cells.at(segment.from.first);
                 const auto &sink = ctx->cells.at(segment.to.first);
@@ -94,9 +105,8 @@ static void log_crit_paths(const Context *ctx, TimingResult &result)
                 auto driver_loc = ctx->getBelLocation(driver->bel);
                 auto sink_loc = ctx->getBelLocation(sink->bel);
 
-                log_info("%4.1f %4.1f    Net %s (%d,%d) -> (%d,%d)\n", ctx->getDelayNS(segment.delay),
+                log_info("%4.1f %4.1f    Net %s (%d,%d) -> (%d,%d)\n", get_delay_ns(segment.delay), get_delay_ns(total),
-                         ctx->getDelayNS(total), segment.net.c_str(ctx), driver_loc.x, driver_loc.y, sink_loc.x,
+                         segment.net.c_str(ctx), driver_loc.x, driver_loc.y, sink_loc.x, sink_loc.y);
-                         sink_loc.y);
                 log_info("               Sink %s.%s\n", segment.to.first.c_str(ctx), segment.to.second.c_str(ctx));
 
                 const NetInfo *net = ctx->nets.at(segment.net).get();
@@ -134,7 +144,7 @@ static void log_crit_paths(const Context *ctx, TimingResult &result)
                 }
             }
         }
-        log_info("%.1f ns logic, %.1f ns routing\n", ctx->getDelayNS(logic_total), ctx->getDelayNS(route_total));
+        log_info("%.1f ns logic, %.1f ns routing\n", get_delay_ns(logic_total), get_delay_ns(route_total));
     };
 
     // Single domain paths
@@ -223,7 +233,7 @@ static void log_fmax(Context *ctx, TimingResult &result, bool warn_on_failure)
                 continue;
             }
 
-            delay_t path_delay = 0;
+            DelayPair path_delay(0);
             for (const auto &segment : report.segments) {
                 path_delay += segment.delay;
             }
@@ -232,13 +242,13 @@ static void log_fmax(Context *ctx, TimingResult &result, bool warn_on_failure)
             // result is negative then only the latter matters. Otherwise
             // the compensated path delay is taken.
             auto clock_delay = result.clock_delays.at(key);
-            path_delay -= clock_delay;
+            path_delay -= DelayPair(clock_delay);
 
             float fmax = std::numeric_limits<float>::infinity();
-            if (path_delay < 0) {
+            if (path_delay.maxDelay() < 0) {
                 fmax = 1e3f / ctx->getDelayNS(clock_delay);
-            } else if (path_delay > 0) {
+            } else if (path_delay.maxDelay() > 0) {
-                fmax = 1e3f / ctx->getDelayNS(path_delay);
+                fmax = 1e3f / ctx->getDelayNS(path_delay.maxDelay());
             }
 
             // Both clocks are related so they should have the same
@@ -306,12 +316,12 @@ static void log_fmax(Context *ctx, TimingResult &result, bool warn_on_failure)
     for (auto &report : result.xclock_paths) {
         const ClockEvent &a = report.clock_pair.start;
         const ClockEvent &b = report.clock_pair.end;
-        delay_t path_delay = 0;
+        DelayPair path_delay(0);
         for (const auto &segment : report.segments) {
             path_delay += segment.delay;
         }
         auto ev_a = clock_event_name(ctx, a, start_field_width), ev_b = clock_event_name(ctx, b, end_field_width);
-        log_info("Max delay %s -> %s: %0.02f ns\n", ev_a.c_str(), ev_b.c_str(), ctx->getDelayNS(path_delay));
+        log_info("Max delay %s -> %s: %0.02f ns\n", ev_a.c_str(), ev_b.c_str(), ctx->getDelayNS(path_delay.maxDelay()));
     }
     log_break();
 }