caiyu/nextpnr
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

timing: Fix hold slack not matching reported path delay

Browse Source
This commit is contained in:
Rowan Goemans 2024-09-20 19:08:00 +02:00 committed by myrtle
parent 098dcaedec
commit 93e233dad9
4 changed files with 65 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
common/kernel/nextpnr_types.h
View File

@ -451,16 +451,14 @@ struct CriticalPath
// To cell.port // To cell.port
std::pair<IdString, IdString> to; std::pair<IdString, IdString> to;
// Segment delay // Segment delay
DelayPair delay; delay_t delay;
}; };
// Clock pair // Clock pair
ClockPair clock_pair; ClockPair clock_pair;
// Total path delay
// if sum < 0 this is a hold/min violation
// if delay.maxDelay() >= max_delay this is a setup/max violation
DelayPair delay;
// if sum[segments.delay] < 0 this is a hold/min violation
// if sum[segments.delay] > max_delay this is a setup/max violation
delay_t max_delay; delay_t max_delay;
// Individual path segments // Individual path segments

15
common/kernel/report.cc
View File

@ -73,11 +73,11 @@ static Json::array json_report_critical_paths(const Context *ctx)
{"port", segment.to.second.c_str(ctx)}, {"port", segment.to.second.c_str(ctx)},
{"loc", Json::array({toLoc.x, toLoc.y})}}); {"loc", Json::array({toLoc.x, toLoc.y})}});
auto minDelay = ctx->getDelayNS(segment.delay.minDelay()); auto segmentJson = Json::object({
auto maxDelay = ctx->getDelayNS(segment.delay.maxDelay()); {"delay", ctx->getDelayNS(segment.delay)},
{"from", fromJson},
auto segmentJson = {"to", toJson},
Json::object({{"delay", Json::array({minDelay, maxDelay})}, {"from", fromJson}, {"to", toJson}}); });
segmentJson["type"] = CriticalPath::Segment::type_to_str(segment.type); segmentJson["type"] = CriticalPath::Segment::type_to_str(segment.type);
if (segment.type == CriticalPath::Segment::Type::ROUTING) { if (segment.type == CriticalPath::Segment::Type::ROUTING) {
@ -186,10 +186,7 @@ Report JSON structure:
}, },
"type": <path segment type "clk-to-q", "source", "logic", "routing" or "setup">, "type": <path segment type "clk-to-q", "source", "logic", "routing" or "setup">,
"net": <net name (for routing only!)>, "net": <net name (for routing only!)>,
"delay": [ "delay": <segment delay [ns]>,
<minimum segment delay [ns]>,
<maximum segment delay [ns]>,
],
} }
... ...
] ]

47
common/kernel/timing.cc
View File

@ -895,8 +895,6 @@ CriticalPath TimingAnalyser::build_critical_path_report(domain_id_t domain_pair,
const auto &launch = domains.at(dp.key.launch).key; const auto &launch = domains.at(dp.key.launch).key;
const auto &capture = domains.at(dp.key.capture).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.clock = launch.clock;
report.clock_pair.start.edge = launch.edge; report.clock_pair.start.edge = launch.edge;
report.clock_pair.end.clock = capture.clock; report.clock_pair.end.clock = capture.clock;
@ -980,7 +978,7 @@ CriticalPath TimingAnalyser::build_critical_path_report(domain_id_t domain_pair,
if (!is_zero_delay(clock_delay)) { if (!is_zero_delay(clock_delay)) {
CriticalPath::Segment seg_c2c; CriticalPath::Segment seg_c2c;
seg_c2c.type = CriticalPath::Segment::Type::CLK_TO_CLK; seg_c2c.type = CriticalPath::Segment::Type::CLK_TO_CLK;
seg_c2c.delay = DelayPair(clock_delay); seg_c2c.delay = clock_delay;
seg_c2c.from = std::make_pair(sp_cell->name, sp_clk_info.clock_port); seg_c2c.from = std::make_pair(sp_cell->name, sp_clk_info.clock_port);
seg_c2c.to = std::make_pair(ep_cell->name, ep_clk_info.clock_port); seg_c2c.to = std::make_pair(ep_cell->name, ep_clk_info.clock_port);
seg_c2c.net = IdString(); seg_c2c.net = IdString();
@ -998,7 +996,7 @@ CriticalPath TimingAnalyser::build_critical_path_report(domain_id_t domain_pair,
if (!is_zero_delay(clock_skew)) { if (!is_zero_delay(clock_skew)) {
CriticalPath::Segment seg_skew; CriticalPath::Segment seg_skew;
seg_skew.type = CriticalPath::Segment::Type::CLK_SKEW; seg_skew.type = CriticalPath::Segment::Type::CLK_SKEW;
seg_skew.delay = DelayPair(clock_skew); seg_skew.delay = clock_skew;
seg_skew.from = std::make_pair(sp_cell->name, sp_clk_info.clock_port); seg_skew.from = std::make_pair(sp_cell->name, sp_clk_info.clock_port);
seg_skew.to = std::make_pair(ep_cell->name, ep_clk_info.clock_port); seg_skew.to = std::make_pair(ep_cell->name, ep_clk_info.clock_port);
if (same_clock) { if (same_clock) {
@ -1035,7 +1033,7 @@ CriticalPath TimingAnalyser::build_critical_path_report(domain_id_t domain_pair,
seg_logic.type = CriticalPath::Segment::Type::LOGIC; seg_logic.type = CriticalPath::Segment::Type::LOGIC;
} }
seg_logic.delay = comb_delay.delayPair(); seg_logic.delay = longest_path ? comb_delay.maxDelay() : comb_delay.minDelay();
seg_logic.from = std::make_pair(prev_cell->name, prev_port); seg_logic.from = std::make_pair(prev_cell->name, prev_port);
seg_logic.to = std::make_pair(driver_cell->name, driver.port); seg_logic.to = std::make_pair(driver_cell->name, driver.port);
seg_logic.net = IdString(); seg_logic.net = IdString();
@ -1045,7 +1043,7 @@ CriticalPath TimingAnalyser::build_critical_path_report(domain_id_t domain_pair,
CriticalPath::Segment seg_route; CriticalPath::Segment seg_route;
seg_route.type = CriticalPath::Segment::Type::ROUTING; seg_route.type = CriticalPath::Segment::Type::ROUTING;
seg_route.delay = net_delay; seg_route.delay = longest_path ? net_delay.maxDelay() : net_delay.minDelay();
seg_route.from = std::make_pair(driver_cell->name, driver.port); seg_route.from = std::make_pair(driver_cell->name, driver.port);
seg_route.to = std::make_pair(sink_cell->name, sink.port); seg_route.to = std::make_pair(sink_cell->name, sink.port);
seg_route.net = net->name; seg_route.net = net->name;
@ -1058,13 +1056,13 @@ CriticalPath TimingAnalyser::build_critical_path_report(domain_id_t domain_pair,
if (register_end) { if (register_end) {
CriticalPath::Segment seg_logic; CriticalPath::Segment seg_logic;
seg_logic.delay = DelayPair(0); seg_logic.delay = 0;
if (longest_path) { if (longest_path) {
seg_logic.type = CriticalPath::Segment::Type::SETUP; seg_logic.type = CriticalPath::Segment::Type::SETUP;
seg_logic.delay += ep_clk_info.setup; seg_logic.delay += ep_clk_info.setup.maxDelay();
} else { } else {
seg_logic.type = CriticalPath::Segment::Type::HOLD; seg_logic.type = CriticalPath::Segment::Type::HOLD;
seg_logic.delay -= ep_clk_info.hold; seg_logic.delay -= ep_clk_info.hold.maxDelay();
} }
seg_logic.from = std::make_pair(prev_cell->name, prev_port); seg_logic.from = std::make_pair(prev_cell->name, prev_port);
seg_logic.to = seg_logic.from; seg_logic.to = seg_logic.from;
@ -1222,6 +1220,7 @@ std::vector<CriticalPath> TimingAnalyser::get_min_delay_violations()
for (auto &[launch_id, arr] : port.arrival) { for (auto &[launch_id, arr] : port.arrival) {
const auto &launch = domains.at(launch_id); const auto &launch = domains.at(launch_id);
const auto &launch_clock = launch.key.clock; const auto &launch_clock = launch.key.clock;
const auto dom_pair_id = domain_pair_id(launch_id, capture_id);
auto clocks = std::make_pair(launch_clock, capture_clock); auto clocks = std::make_pair(launch_clock, capture_clock);
auto related_clocks = clock_delays.count(clocks) > 0; auto related_clocks = clock_delays.count(clocks) > 0;
@ -1236,17 +1235,37 @@ std::vector<CriticalPath> TimingAnalyser::get_min_delay_violations()
} }
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;
if (violated) { if (hold_slack <= 0) {
const auto dom_pair_id = domain_pair_id(launch_id, capture_id); auto report = build_critical_path_report(dom_pair_id, ep.first, false);
violations.emplace_back(build_critical_path_report(dom_pair_id, ep.first, false)); violations.emplace_back(report);
} }
} }
} }
} }
return violations; std::vector<std::pair<size_t, delay_t>> sum_indices;
sum_indices.reserve(violations.size());
for (size_t i = 0; i < violations.size(); ++i) {
delay_t delay = 0;
for (const auto &seg : violations[i].segments) {
delay += seg.delay;
}
sum_indices.emplace_back(i, delay);
}
std::sort(sum_indices.begin(), sum_indices.end(),
[](auto &left, auto &right) { return left.second < right.second; });
std::vector<CriticalPath> sorted_violations;
sorted_violations.reserve(violations.size());
for (const auto &pair : sum_indices) {
sorted_violations.push_back(std::move(violations[pair.first]));
}
return sorted_violations;
} }
domain_id_t TimingAnalyser::domain_id(IdString cell, IdString clock_port, ClockEdge edge) domain_id_t TimingAnalyser::domain_id(IdString cell, IdString clock_port, ClockEdge edge)

55
common/kernel/timing_log.cc
View File

@ -69,38 +69,29 @@ static void log_crit_paths(const Context *ctx, TimingResult &result)
// A helper function for reporting one critical path // A helper function for reporting one critical path
auto print_path_report = [ctx](const CriticalPath &path) { auto print_path_report = [ctx](const CriticalPath &path) {
DelayPair total(0), logic_total(0), route_total(0); delay_t 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 min_delay_violation = path.delay.minDelay() < 0;
auto get_delay_ns = [min_delay_violation, ctx](const DelayPair &d) {
if (min_delay_violation) {
ctx->getDelayNS(d.minDelay());
}
return ctx->getDelayNS(d.maxDelay());
};
log_info(" type curr total name\n"); log_info(" type curr total name\n");
for (const auto &segment : path.segments) { for (const auto &segment : path.segments) {
total += segment.delay; delay_t delay = segment.delay;
total += delay;
if (segment.type == CriticalPath::Segment::Type::CLK_TO_Q || if (segment.type == CriticalPath::Segment::Type::CLK_TO_Q ||
segment.type == CriticalPath::Segment::Type::SOURCE || segment.type == CriticalPath::Segment::Type::SOURCE ||
segment.type == CriticalPath::Segment::Type::LOGIC || segment.type == CriticalPath::Segment::Type::LOGIC ||
segment.type == CriticalPath::Segment::Type::SETUP || segment.type == CriticalPath::Segment::Type::SETUP ||
segment.type == CriticalPath::Segment::Type::HOLD) { segment.type == CriticalPath::Segment::Type::HOLD) {
logic_total += segment.delay; logic_total += delay;
log_info("%10s % 5.2f % 5.2f Source %s.%s\n", CriticalPath::Segment::type_to_str(segment.type).c_str(), log_info("%10s % 5.2f % 5.2f Source %s.%s\n", CriticalPath::Segment::type_to_str(segment.type).c_str(),
get_delay_ns(segment.delay), get_delay_ns(total), segment.to.first.c_str(ctx), ctx->getDelayNS(delay), ctx->getDelayNS(total), segment.to.first.c_str(ctx),
segment.to.second.c_str(ctx)); segment.to.second.c_str(ctx));
} else if (segment.type == CriticalPath::Segment::Type::ROUTING || } else if (segment.type == CriticalPath::Segment::Type::ROUTING ||
segment.type == CriticalPath::Segment::Type::CLK_TO_CLK || segment.type == CriticalPath::Segment::Type::CLK_TO_CLK ||
segment.type == CriticalPath::Segment::Type::CLK_SKEW) { segment.type == CriticalPath::Segment::Type::CLK_SKEW) {
route_total = route_total + segment.delay; route_total = route_total + delay;
const auto &driver = ctx->cells.at(segment.from.first); const auto &driver = ctx->cells.at(segment.from.first);
const auto &sink = ctx->cells.at(segment.to.first); const auto &sink = ctx->cells.at(segment.to.first);
@ -109,8 +100,8 @@ static void log_crit_paths(const Context *ctx, TimingResult &result)
auto sink_loc = ctx->getBelLocation(sink->bel); auto sink_loc = ctx->getBelLocation(sink->bel);
log_info("%10s % 5.2f % 5.2f Net %s (%d,%d) -> (%d,%d)\n", log_info("%10s % 5.2f % 5.2f Net %s (%d,%d) -> (%d,%d)\n",
CriticalPath::Segment::type_to_str(segment.type).c_str(), get_delay_ns(segment.delay), CriticalPath::Segment::type_to_str(segment.type).c_str(), ctx->getDelayNS(delay),
get_delay_ns(total), segment.net.c_str(ctx), driver_loc.x, driver_loc.y, sink_loc.x, ctx->getDelayNS(total), 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), log_info(" Sink %s.%s\n", segment.to.first.c_str(ctx),
segment.to.second.c_str(ctx)); segment.to.second.c_str(ctx));
@ -150,7 +141,7 @@ static void log_crit_paths(const Context *ctx, TimingResult &result)
} }
} }
} }
log_info("%.2f ns logic, %.2f ns routing\n", get_delay_ns(logic_total), get_delay_ns(route_total)); log_info("%.2f ns logic, %.2f ns routing\n", ctx->getDelayNS(logic_total), ctx->getDelayNS(route_total));
}; };
// Single domain paths // Single domain paths
@ -180,16 +171,16 @@ static void log_crit_paths(const Context *ctx, TimingResult &result)
auto num_min_violations = result.min_delay_violations.size(); auto num_min_violations = result.min_delay_violations.size();
if (num_min_violations > 0) { if (num_min_violations > 0) {
log_break(); log_break();
log_info("Hold time/min time violations:\n"); log_info("%zu Hold/min time violations (showing 10 worst paths):\n", num_min_violations);
for (size_t i = 0; i < std::min((size_t)10, num_min_violations); ++i) { for (size_t i = 0; i < std::min((size_t)10, num_min_violations); ++i) {
auto &report = result.min_delay_violations.at(i); auto &report = result.min_delay_violations.at(i);
log_break(); log_break();
std::string start = clock_event_name(ctx, report.clock_pair.start); std::string start = clock_event_name(ctx, report.clock_pair.start);
std::string end = clock_event_name(ctx, report.clock_pair.end); std::string end = clock_event_name(ctx, report.clock_pair.end);
if (report.clock_pair.start == report.clock_pair.end) { if (report.clock_pair.start == report.clock_pair.end) {
log_nonfatal_error("Hold time violation for clock '%s':\n", start.c_str()); log_nonfatal_error("Hold/min time violation for clock '%s':\n", start.c_str());
} else { } else {
log_nonfatal_error("Hold time violation for path '%s' -> '%s':\n", start.c_str(), end.c_str()); log_nonfatal_error("Hold/min time violation for path '%s' -> '%s':\n", start.c_str(), end.c_str());
} }
print_path_report(report); print_path_report(report);
} }
@ -230,13 +221,13 @@ static void log_fmax(Context *ctx, TimingResult &result, bool warn_on_failure)
log_break(); log_break();
// Clock to clock delays for xpaths // Clock to clock delays for xpaths
dict<ClockPair, DelayPair> xclock_delays; dict<ClockPair, delay_t> xclock_delays;
for (auto &report : result.xclock_paths) { for (auto &report : result.xclock_paths) {
// Check if this path has a clock-2-clock delay // Check if this path has a clock-2-clock delay
// clock-2-clock delays are always the first segment in the path // clock-2-clock delays are always the first segment in the path
// But we walk the entire path anyway. // But we walk the entire path anyway.
bool has_clock_to_clock = false; bool has_clock_to_clock = false;
DelayPair clock_delay = DelayPair(0); delay_t clock_delay = 0;
for (const auto &seg : report.segments) { for (const auto &seg : report.segments) {
if (seg.type == CriticalPath::Segment::Type::CLK_TO_CLK) { if (seg.type == CriticalPath::Segment::Type::CLK_TO_CLK) {
has_clock_to_clock = true; has_clock_to_clock = true;
@ -266,7 +257,7 @@ static void log_fmax(Context *ctx, TimingResult &result, bool warn_on_failure)
continue; continue;
} }
DelayPair path_delay(0); delay_t path_delay = 0;
for (const auto &segment : report.segments) { for (const auto &segment : report.segments) {
path_delay += segment.delay; path_delay += segment.delay;
} }
@ -277,10 +268,10 @@ static void log_fmax(Context *ctx, TimingResult &result, bool warn_on_failure)
auto clock_delay = xclock_delays.at(report.clock_pair); auto clock_delay = xclock_delays.at(report.clock_pair);
float fmax = std::numeric_limits<float>::infinity(); float fmax = std::numeric_limits<float>::infinity();
if (path_delay.maxDelay() < 0) { if (path_delay < 0) {
fmax = 1e3f / ctx->getDelayNS(clock_delay.maxDelay()); fmax = 1e3f / ctx->getDelayNS(clock_delay);
} else if (path_delay.maxDelay() > 0) { } else if (path_delay > 0) {
fmax = 1e3f / ctx->getDelayNS(path_delay.maxDelay()); fmax = 1e3f / ctx->getDelayNS(path_delay);
} }
// Both clocks are related so they should have the same // Both clocks are related so they should have the same
@ -322,7 +313,7 @@ static void log_fmax(Context *ctx, TimingResult &result, bool warn_on_failure)
auto ev_a = clock_event_name(ctx, pair.first.start, max_width_xca); auto ev_a = clock_event_name(ctx, pair.first.start, max_width_xca);
auto ev_b = clock_event_name(ctx, pair.first.end, max_width_xcb); auto ev_b = clock_event_name(ctx, pair.first.end, max_width_xcb);
delay_t delay = pair.second.maxDelay(); delay_t delay = pair.second;
if (pair.first.start.edge != pair.first.end.edge) { if (pair.first.start.edge != pair.first.end.edge) {
delay /= 2; delay /= 2;
} }
@ -348,12 +339,12 @@ static void log_fmax(Context *ctx, TimingResult &result, bool warn_on_failure)
for (auto &report : result.xclock_paths) { for (auto &report : result.xclock_paths) {
const ClockEvent &a = report.clock_pair.start; const ClockEvent &a = report.clock_pair.start;
const ClockEvent &b = report.clock_pair.end; const ClockEvent &b = report.clock_pair.end;
DelayPair path_delay(0); delay_t path_delay = 0;
for (const auto &segment : report.segments) { for (const auto &segment : report.segments) {
path_delay += segment.delay; 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); 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.maxDelay())); log_info("Max delay %s -> %s: %0.02f ns\n", ev_a.c_str(), ev_b.c_str(), ctx->getDelayNS(path_delay));
} }
log_break(); log_break();
} }