From 87f002a454581f6c2a90a472aa40e75d0fc73334 Mon Sep 17 00:00:00 2001 From: Rowan Goemans Date: Tue, 17 Sep 2024 21:26:43 +0200 Subject: [PATCH] timing: Fix slack calculations timing: Fix max_delay_by_domain_pair function timing: Fix hold time check --- common/kernel/timing.cc | 173 +++++++++++++++++++++--------------- common/kernel/timing.h | 6 ++ common/kernel/timing_log.cc | 6 +- 3 files changed, 110 insertions(+), 75 deletions(-) diff --git a/common/kernel/timing.cc b/common/kernel/timing.cc index 957c4f5a..008eff50 100644 --- a/common/kernel/timing.cc +++ b/common/kernel/timing.cc @@ -118,9 +118,7 @@ void TimingAnalyser::get_cell_delays() continue; pd.cell_arcs.emplace_back(CellArc::SETUP, info.clock_port, DelayQuad(info.setup, info.setup), info.edge); - auto hold_factor = 1; - auto hold_faked = DelayPair(hold_factor * info.hold.min_delay, hold_factor * info.hold.max_delay); - pd.cell_arcs.emplace_back(CellArc::HOLD, info.clock_port, DelayQuad(hold_faked, hold_faked), + pd.cell_arcs.emplace_back(CellArc::HOLD, info.clock_port, DelayQuad(info.hold, info.hold), info.edge); } } @@ -576,7 +574,9 @@ void TimingAnalyser::walk_forward() // Include the clock delay if clock_skew analysis is enabled if (with_clock_skew) { auto clock_delay = ports.at(CellPortKey(sp.first.cell, fanin.other_port)).route_delay; - init_arrival += DelayPair(clock_delay.min_delay); + clock_delay.min_delay = clock_delay_fac * clock_delay.min_delay; + clock_delay.max_delay = clock_delay_fac * clock_delay.max_delay; + init_arrival += clock_delay; } break; } @@ -634,12 +634,14 @@ void TimingAnalyser::walk_backward() if (fanin.type == CellArc::SETUP && fanin.other_port == ep.second) { if (with_clock_skew) { auto clock_delay = ports.at(CellPortKey(ep.first.cell, fanin.other_port)).route_delay; - init_required -= DelayPair(clock_delay.min_delay); + clock_delay.min_delay = clock_delay_fac * clock_delay.min_delay; + clock_delay.max_delay = clock_delay_fac * clock_delay.max_delay; + init_required += clock_delay; } init_required.min_delay -= fanin.value.maxDelay(); } if (fanin.type == CellArc::HOLD && fanin.other_port == ep.second) - init_required.max_delay -= fanin.value.maxDelay(); + init_required.max_delay += fanin.value.maxDelay(); } clock_key = CellPortKey(ep.first.cell, ep.second); } @@ -677,31 +679,58 @@ dict TimingAnalyser::max_delay_by_domain_pairs() for (domain_id_t capture_id = 0; capture_id < domain_id_t(domains.size()); ++capture_id) { const auto &capture = domains.at(capture_id); - const auto &capture_clock = capture.key.clock; for (auto &ep : capture.endpoints) { - auto &port = ports.at(ep.first); + auto &ep_port = ports.at(ep.first); - auto &req = port.required.at(capture_id); + auto &req = ep_port.required.at(capture_id); - for (auto &[launch_id, arr] : port.arrival) { - - // const auto &launch = domains.at(launch_id); - // const auto &launch_clock = launch.key.clock; - - // auto clocks = std::make_pair(launch_clock, capture_clock); - // auto related_clocks = clock_delays.count(clocks) > 0; - - // delay_t clock_to_clock = 0; - // if (related_clocks) { - // clock_to_clock = clock_delays.at(clocks); - // } + for (auto &[launch_id, arr] : ep_port.arrival) { + const auto &launch = domains.at(capture_id); auto dp = domain_pair_id(launch_id, capture_id); - auto delay = arr.value.maxDelay() - req.value.maxDelay(); - if (!domain_delay.count(dp) || domain_delay.at(dp) < delay) + auto clocks = std::make_pair(launch.key.clock, capture.key.clock); + auto same_clock = capture_id == launch_id; + auto related_clocks = clock_delays.count(clocks) > 0; + delay_t clock_to_clock = 0; + if (related_clocks) { + clock_to_clock = clock_delays.at(clocks); + } + + auto delay = arr.value.maxDelay() - req.value.minDelay() + clock_to_clock; + + // If domains are unrelated or not the same clock we need to make sure + // to remove the clock delays from the arrival and required times + // because the delays have no common reference. + if (with_clock_skew && !same_clock && !related_clocks) { + for (auto &fanin : ep_port.cell_arcs) { + if (fanin.type == CellArc::SETUP) { + auto clock_delay = ports.at(CellPortKey(ep.first.cell, fanin.other_port)).route_delay; + clock_delay.min_delay = clock_delay_fac * clock_delay.min_delay; + clock_delay.max_delay = clock_delay_fac * clock_delay.max_delay; + delay += clock_delay.minDelay(); + } + } + + // walk back to startpoint + auto crit_path = walk_crit_path(domain_pair_id(launch_id, capture_id), ep.first, true); + auto &sp = crit_path.back(); + auto &sp_port = ports.at(CellPortKey{sp.cell->name, sp.port}); + + for (auto &fanin : sp_port.cell_arcs) { + if (fanin.type == CellArc::CLK_TO_Q) { + auto clock_delay = ports.at(CellPortKey(sp.cell->name, fanin.other_port)).route_delay; + clock_delay.min_delay = clock_delay_fac * clock_delay.min_delay; + clock_delay.max_delay = clock_delay_fac * clock_delay.max_delay; + delay -= clock_delay.maxDelay(); + } + } + } + + if (!domain_delay.count(dp) || domain_delay.at(dp) < delay) { domain_delay[dp] = delay; + } } } } @@ -825,36 +854,9 @@ std::vector TimingAnalyser::get_worst_eps(domain_id_t domain_pair, return worst_eps; } -CriticalPath TimingAnalyser::build_critical_path_report(domain_id_t domain_pair, CellPortKey endpoint, - bool longest_path) +std::vector TimingAnalyser::walk_crit_path(domain_id_t domain_pair, CellPortKey endpoint, bool longest_path) { - CriticalPath report; - const auto &dp = domain_pairs.at(domain_pair); - const auto &launch = domains.at(dp.key.launch).key; - const 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.bound = DelayPair(0, ctx->getDelayFromNS(1.0e9 / ctx->setting("target_freq"))); - if (launch.edge != capture.edge) { - 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.bound.max_delay = ctx->nets.at(launch.clock)->clkconstr->period.minDelay(); - } else if (capture.edge == RISING_EDGE) { - report.bound.max_delay = ctx->nets.at(launch.clock)->clkconstr->low.minDelay(); - } else if (capture.edge == FALLING_EDGE) { - report.bound.max_delay = ctx->nets.at(launch.clock)->clkconstr->high.minDelay(); - } - } // Walk the min or max path backwards to find a single crit path pool> visited; @@ -890,6 +892,42 @@ CriticalPath TimingAnalyser::build_critical_path_report(domain_id_t domain_pair, } } while (!is_startpoint); + return crit_path_rev; +} + +CriticalPath TimingAnalyser::build_critical_path_report(domain_id_t domain_pair, CellPortKey endpoint, + bool longest_path) +{ + CriticalPath report; + + const auto &dp = domain_pairs.at(domain_pair); + const auto &launch = domains.at(dp.key.launch).key; + const 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.bound = DelayPair(0, ctx->getDelayFromNS(1.0e9 / ctx->setting("target_freq"))); + if (launch.edge != capture.edge) { + 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.bound.max_delay = ctx->nets.at(launch.clock)->clkconstr->period.minDelay(); + } else if (capture.edge == RISING_EDGE) { + report.bound.max_delay = ctx->nets.at(launch.clock)->clkconstr->low.minDelay(); + } else if (capture.edge == FALLING_EDGE) { + report.bound.max_delay = ctx->nets.at(launch.clock)->clkconstr->high.minDelay(); + } + } + + // Walk the min or max path backwards to find a single crit path + auto crit_path_rev = walk_crit_path(domain_pair, endpoint, longest_path); auto crit_path = boost::adaptors::reverse(crit_path_rev); // Get timing and clocking info on the startpoint @@ -963,13 +1001,12 @@ CriticalPath TimingAnalyser::build_critical_path_report(domain_id_t domain_pair, // and either the clock domains are the same or related clock if (with_clock_skew && register_start && register_end && (same_clock || related_clock)) { - auto clock_delay_launch = ctx->getNetinfoRouteDelay(sp_clk_net, PortRef{sp_cell, sp_clk_info.clock_port}); - auto clock_delay_capture = ctx->getNetinfoRouteDelay(ep_clk_net, PortRef{ep_cell, ep_clk_info.clock_port}); + auto clock_delay_launch = + clock_delay_fac * ctx->getNetinfoRouteDelay(sp_clk_net, PortRef{sp_cell, sp_clk_info.clock_port}); + auto clock_delay_capture = + clock_delay_fac * ctx->getNetinfoRouteDelay(ep_clk_net, PortRef{ep_cell, ep_clk_info.clock_port}); delay_t clock_skew = clock_delay_launch - clock_delay_capture; - printf("delay launch: %f\n", ctx->getDelayNS(clock_delay_launch)); - printf("delay capture: %f\n", ctx->getDelayNS(clock_delay_capture)); - printf("clock to clock: %f\n", ctx->getDelayNS(clock_skew)); if (clock_skew != 0) { CriticalPath::Segment seg_skew; @@ -989,6 +1026,7 @@ CriticalPath TimingAnalyser::build_critical_path_report(domain_id_t domain_pair, const CellInfo *prev_cell = sp_cell; IdString prev_port = sp_port.name; + bool is_startpoint = true; for (auto sink : crit_path) { auto sink_cell = sink.cell; auto &port = sink_cell->ports.at(sink.port); @@ -1188,16 +1226,16 @@ std::vector TimingAnalyser::get_min_delay_violations() const auto &capture = domains.at(capture_id); const auto &capture_clock = capture.key.clock; - for (auto &ep : capture.endpoints) { - CellInfo *ci = cell_info(ep.first); + for (const auto &ep : capture.endpoints) { + const CellInfo *ci = cell_info(ep.first); int clkInfoCount = 0; - TimingPortClass cls = ctx->getPortTimingClass(ci, ep.first.port, clkInfoCount); + const TimingPortClass cls = ctx->getPortTimingClass(ci, ep.first.port, clkInfoCount); if (cls != TMG_REGISTER_INPUT) continue; - auto &port = ports.at(ep.first); + const auto &port = ports.at(ep.first); - auto &req = port.required.at(capture_id); + const auto &req = port.required.at(capture_id); for (auto &[launch_id, arr] : port.arrival) { const auto &launch = domains.at(launch_id); @@ -1206,8 +1244,8 @@ std::vector TimingAnalyser::get_min_delay_violations() auto clocks = std::make_pair(launch_clock, capture_clock); auto related_clocks = clock_delays.count(clocks) > 0; - // Don't consider clocks without known relationships - if (launch_id != capture_id && !related_clocks) { + // Don't consider async paths and clocks without known relationships + if (launch_id == async_clock_id && launch_id != capture_id && !related_clocks) { continue; } @@ -1216,14 +1254,9 @@ std::vector TimingAnalyser::get_min_delay_violations() clock_to_clock = clock_delays.at(clocks); } - auto hold_slack = arr.value.minDelay() + req.value.maxDelay() + clock_to_clock; + auto hold_slack = arr.value.minDelay() - req.value.maxDelay() + clock_to_clock; auto violated = hold_slack <= 0; - // printf("arr min: %f, req max: %f, c2c; %f, slack: %f, arr path len: %d, req path len: %d\n", - // ctx->getDelayNS(arr.value.minDelay()), ctx->getDelayNS(req.value.maxDelay()), - // ctx->getDelayNS(clock_to_clock), ctx->getDelayNS(hold_slack), arr.path_length, - // req.path_length); - if (violated) { const auto dom_pair_id = domain_pair_id(launch_id, capture_id); violations.emplace_back(build_critical_path_report(dom_pair_id, ep.first, false)); @@ -1298,7 +1331,7 @@ void timing_analysis(Context *ctx, bool print_slack_histogram, bool print_fmax, { TimingAnalyser tmg(ctx); tmg.setup_only = false; - tmg.with_clock_skew = false; + tmg.with_clock_skew = true; tmg.setup(ctx->detailed_timing_report, print_slack_histogram, print_path || print_fmax); auto &result = tmg.get_timing_result(); diff --git a/common/kernel/timing.h b/common/kernel/timing.h index fc60a5ee..9bba709e 100644 --- a/common/kernel/timing.h +++ b/common/kernel/timing.h @@ -101,6 +101,8 @@ struct TimingAnalyser // Enable analysis of clock skew between FFs. // Only do this after legal placement bool with_clock_skew = true; + // REMOVE ME once approved + delay_t clock_delay_fac = 100; bool setup_only = false; bool have_loops = false; @@ -122,6 +124,10 @@ struct TimingAnalyser void compute_slack(); void compute_criticality(); + // Walk the endpoint back to a startpoint and get back the input ports walked + // and the startpoint. + std::vector walk_crit_path(domain_id_t domain_pair, CellPortKey endpoint, bool longest_path); + void build_detailed_net_timing_report(); // longest_path indicate whether to follow the longest or shortest path from endpoint to startpoint // longest paths are interesting for setup violations and shortest paths are interesting for hold violations diff --git a/common/kernel/timing_log.cc b/common/kernel/timing_log.cc index e21fcb3d..c01b1159 100644 --- a/common/kernel/timing_log.cc +++ b/common/kernel/timing_log.cc @@ -82,7 +82,7 @@ static void log_crit_paths(const Context *ctx, TimingResult &result) return ctx->getDelayNS(d.maxDelay()); }; - log_info(" type curr total\n"); + log_info(" type curr total\n"); for (const auto &segment : path.segments) { total += segment.delay; @@ -232,9 +232,6 @@ static void log_fmax(Context *ctx, TimingResult &result, bool warn_on_failure) // Clock to clock delays for xpaths dict xclock_delays; for (auto &report : result.xclock_paths) { - const auto &clock1_name = report.clock_pair.start.clock; - const auto &clock2_name = report.clock_pair.end.clock; - // Check if this path has a clock-2-clock delay // clock-2-clock delays are always the first segment in the path // But we walk the entire path anyway. @@ -265,7 +262,6 @@ static void log_fmax(Context *ctx, TimingResult &result, bool warn_on_failure) const auto &clock_a = report.clock_pair.start.clock; const auto &clock_b = report.clock_pair.end.clock; - const auto key = std::make_pair(clock_a, clock_b); if (!xclock_delays.count(report.clock_pair)) { continue; }