From 8d0f52fbf9b0bf4356ad342d8cc3b5fa6e423520 Mon Sep 17 00:00:00 2001 From: Rowan Goemans Date: Wed, 11 Sep 2024 08:23:46 +0200 Subject: [PATCH] timing: Move towards DelayPairs for timing reporting (#1359) --- common/kernel/nextpnr_types.h | 47 +++++++++++++++++++++++++++++------ common/kernel/report.cc | 25 +++++++++++++------ common/kernel/timing.cc | 22 +++++++++------- common/kernel/timing.h | 18 ++++++-------- common/kernel/timing_log.cc | 40 ++++++++++++++++++----------- 5 files changed, 102 insertions(+), 50 deletions(-) diff --git a/common/kernel/nextpnr_types.h b/common/kernel/nextpnr_types.h index 163ad2f266..ec1ac11101 100644 --- a/common/kernel/nextpnr_types.h +++ b/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; @@ -200,7 +225,7 @@ struct PseudoCell virtual bool getDelay(IdString fromPort, IdString toPort, DelayQuad &delay) const = 0; virtual TimingPortClass getPortTimingClass(IdString port, int &clockInfoCount) const = 0; virtual TimingClockingInfo getPortClockingInfo(IdString port, int index) const = 0; - virtual ~PseudoCell(){}; + virtual ~PseudoCell() {}; }; struct RegionPlug : PseudoCell @@ -336,15 +361,18 @@ struct CriticalPath // To cell.port std::pair to; // Segment delay - delay_t delay; + DelayPair delay; }; // Clock pair ClockPair clock_pair; // Total path delay - delay_t delay; - // Period (max allowed delay) - delay_t period; + DelayPair delay; + + // 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 segments; }; @@ -357,7 +385,7 @@ struct NetSinkTiming // Cell and port (the sink) std::pair cell_port; // Delay - delay_t delay; + DelayPair delay; }; struct TimingResult @@ -379,6 +407,9 @@ struct TimingResult // Histogram of slack dict slack_histogram; + + // TODO: Hold time violations + // dict hold_violations; }; // Represents the contents of a non-leaf cell in a design diff --git a/common/kernel/report.cc b/common/kernel/report.cc index 5fa51b6f59..917740b96c 100644 --- a/common/kernel/report.cc +++ b/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({ - {"delay", ctx->getDelayNS(segment.delay)}, - {"from", fromJson}, - {"to", toJson}, - }); + auto minDelay = ctx->getDelayNS(segment.delay.minDelay()); + auto maxDelay = ctx->getDelayNS(segment.delay.maxDelay()); + + 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": , "net": , - "delay": , + "delay": [ + , + , + ], } ... ] @@ -209,7 +215,10 @@ Report JSON structure: "cell": , "port": , "event": , - "delay": , + "delay": [ + , + , + ], } ... ] diff --git a/common/kernel/timing.cc b/common/kernel/timing.cc index 7256ed1b18..105d20f906 100644 --- a/common/kernel/timing.cc +++ b/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::max(), std::numeric_limits::lowest()); for (auto &port : ports) { auto do_reset = [&](dict ×) { 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("target_freq")); + report.bound = DelayPair(0, ctx->getDelayFromNS(1.0e9 / ctx->setting("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; diff --git a/common/kernel/timing.h b/common/kernel/timing.h index 3dabe4f1d3..dae3ba27e1 100644 --- a/common/kernel/timing.h +++ b/common/kernel/timing.h @@ -27,8 +27,8 @@ NEXTPNR_NAMESPACE_BEGIN struct CellPortKey { - CellPortKey(){}; - CellPortKey(IdString cell, IdString port) : cell(cell), port(port){}; + CellPortKey() {}; + CellPortKey(IdString cell, IdString port) : cell(cell), port(port) {}; explicit CellPortKey(const PortRef &pr) { NPNR_ASSERT(pr.cell != nullptr); @@ -49,7 +49,7 @@ struct ClockDomainKey { IdString clock; ClockEdge edge; - ClockDomainKey(IdString clock_net, ClockEdge edge) : clock(clock_net), edge(edge){}; + ClockDomainKey(IdString clock_net, ClockEdge edge) : clock(clock_net), edge(edge) {}; // probably also need something here to deal with constraints inline bool is_async() const { return clock == IdString(); } @@ -63,7 +63,7 @@ typedef int domain_id_t; struct ClockDomainPairKey { domain_id_t launch, capture; - ClockDomainPairKey(domain_id_t launch, domain_id_t capture) : launch(launch), capture(capture){}; + ClockDomainPairKey(domain_id_t launch, domain_id_t capture) : launch(launch), capture(capture) {}; inline bool operator==(const ClockDomainPairKey &other) const { return (launch == other.launch) && (capture == other.capture); @@ -128,8 +128,6 @@ struct TimingAnalyser // get the N worst endpoints for a given domain pair std::vector get_worst_eps(domain_id_t domain_pair, int count); - const DelayPair init_delay{std::numeric_limits::max(), std::numeric_limits::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()); @@ -174,9 +172,9 @@ struct TimingAnalyser ClockEdge edge; CellArc(ArcType type, IdString other_port, DelayQuad value) - : type(type), other_port(other_port), value(value), edge(RISING_EDGE){}; + : type(type), other_port(other_port), value(value), edge(RISING_EDGE) {}; CellArc(ArcType type, IdString other_port, DelayQuad value, ClockEdge edge) - : type(type), other_port(other_port), value(value), edge(edge){}; + : type(type), other_port(other_port), value(value), edge(edge) {}; }; // Timing data for every cell port @@ -200,7 +198,7 @@ struct TimingAnalyser struct PerDomain { - PerDomain(ClockDomainKey key) : key(key){}; + PerDomain(ClockDomainKey key) : key(key) {}; ClockDomainKey key; // these are pairs (signal port; clock port) std::vector> startpoints, endpoints; @@ -208,7 +206,7 @@ struct TimingAnalyser struct PerDomainPair { - PerDomainPair(ClockDomainPairKey key) : key(key){}; + PerDomainPair(ClockDomainPairKey key) : key(key) {}; ClockDomainPairKey key; DelayPair period{0}; delay_t worst_setup_slack, worst_hold_slack; diff --git a/common/kernel/timing_log.cc b/common/kernel/timing_log.cc index 1bc1e1168a..24aac66589 100644 --- a/common/kernel/timing_log.cc +++ b/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), - type_name.c_str(), segment.to.first.c_str(ctx), segment.to.second.c_str(ctx)); + log_info("%4.1f %4.1f %s %s.%s\n", get_delay_ns(segment.delay), get_delay_ns(total), type_name.c_str(), + 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), - ctx->getDelayNS(total), segment.net.c_str(ctx), driver_loc.x, driver_loc.y, sink_loc.x, - sink_loc.y); + log_info("%4.1f %4.1f Net %s (%d,%d) -> (%d,%d)\n", get_delay_ns(segment.delay), get_delay_ns(total), + segment.net.c_str(ctx), driver_loc.x, driver_loc.y, sink_loc.x, 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::infinity(); - if (path_delay < 0) { + if (path_delay.maxDelay() < 0) { fmax = 1e3f / ctx->getDelayNS(clock_delay); - } else if (path_delay > 0) { - fmax = 1e3f / ctx->getDelayNS(path_delay); + } else if (path_delay.maxDelay() > 0) { + 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(); }