From 0484d66165d67de49ee35212ba3920c1b782f8fe Mon Sep 17 00:00:00 2001 From: Eric Astor Date: Mon, 4 Nov 2024 16:23:53 -0800 Subject: [PATCH] [XLS] When using a target clock for guidance, apply the user's relaxation parameter to our estimated "smallest possible" clock period This could previously prevent the period-relaxation percent from applying if the user's target clock was estimated to be feasible. PiperOrigin-RevId: 693127795 --- xls/scheduling/run_pipeline_schedule.cc | 26 +++++++++++++++---------- xls/scheduling/sdc_scheduler.cc | 4 ++-- xls/scheduling/sdc_scheduler.h | 1 + 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/xls/scheduling/run_pipeline_schedule.cc b/xls/scheduling/run_pipeline_schedule.cc index 5ff883ee83..301c6c39f7 100644 --- a/xls/scheduling/run_pipeline_schedule.cc +++ b/xls/scheduling/run_pipeline_schedule.cc @@ -413,9 +413,10 @@ absl::StatusOr RunPipelineSchedule( } else { // We don't know the exact target clock period - either none was provided, // or we want to fall back to the minimum feasible clock period if the - // target is infeasible. Determine the minimum clock period (no smaller than - // the target, if provided) for which the function can be scheduled in the - // given pipeline length. + // target is infeasible. Determine the minimum clock period for which the + // function can be scheduled in the given pipeline length (adjusted for the + // user-specified relaxation percent if needed), and use that if it's + // smaller than the target clock period. // // NOTE: We currently use the SDC scheduler to determine the minimum clock // period (if not specified), even if we're not using it for the final @@ -427,20 +428,25 @@ absl::StatusOr RunPipelineSchedule( f, options.pipeline_stages(), /*worst_case_throughput=*/f->IsProc() ? f->GetInitiationInterval() : std::nullopt, - io_delay_added, *sdc_scheduler, options.failure_behavior(), - /*target_clock_period_ps=*/options.clock_period_ps())); + io_delay_added, *sdc_scheduler, options.failure_behavior())); min_clock_period_ps_for_tracing = clock_period_ps; - if (clock_period_ps != options.clock_period_ps() && - options.period_relaxation_percent().has_value()) { - // We found the minimum feasible clock period; apply the user-specified - // relaxation to allow less evenly distributed slack. + if (options.period_relaxation_percent().has_value()) { + // Apply the user-specified relaxation to allow less evenly distributed + // slack. int64_t relaxation_percent = options.period_relaxation_percent().value(); clock_period_ps += (clock_period_ps * relaxation_percent + 50) / 100; } + if (options.clock_period_ps().has_value()) { + // If the user specified a clock period, and it's at least as long as our + // feasible clock period, use that instead; no need to squeeze the stages + // for a tighter clock than the user's target. + clock_period_ps = std::max(clock_period_ps, *options.clock_period_ps()); + } + if (options.clock_period_ps().has_value() && - clock_period_ps != *options.clock_period_ps()) { + clock_period_ps > *options.clock_period_ps()) { CHECK(options.minimize_clock_on_failure().value_or(false)); CHECK(options.recover_after_minimizing_clock().value_or(false)); LOG(WARNING) << "Target clock period was " << *options.clock_period_ps() diff --git a/xls/scheduling/sdc_scheduler.cc b/xls/scheduling/sdc_scheduler.cc index f9b43ee502..6cd11eeb95 100644 --- a/xls/scheduling/sdc_scheduler.cc +++ b/xls/scheduling/sdc_scheduler.cc @@ -192,7 +192,7 @@ SDCSchedulingModel::SDCSchedulingModel(FunctionBase* func, topo_sort_(TopoSort(func_)), model_(model_name), delay_map_(delay_map), - last_stage_(model_.AddContinuousVariable(0.0, kInfinity, "last_stage")), + last_stage_(model_.AddContinuousVariable(0.0, kMaxStages, "last_stage")), cycle_at_sinknode_(model_.AddContinuousVariable(-kInfinity, kInfinity, "cycle_at_sinknode")) { // when subclassed for Iterative SDC, delay_map_ and distances_to_node_ @@ -203,7 +203,7 @@ SDCSchedulingModel::SDCSchedulingModel(FunctionBase* func, for (Node* node : topo_sort_) { cycle_var_.emplace( - node, model_.AddContinuousVariable(0.0, kInfinity, node->GetName())); + node, model_.AddContinuousVariable(0.0, kMaxStages, node->GetName())); model_.AddLinearConstraint( cycle_var_.at(node) <= last_stage_, absl::StrFormat("pipeline_length:%s", node->GetName())); diff --git a/xls/scheduling/sdc_scheduler.h b/xls/scheduling/sdc_scheduler.h index 314ec145b7..c7b3992386 100644 --- a/xls/scheduling/sdc_scheduler.h +++ b/xls/scheduling/sdc_scheduler.h @@ -43,6 +43,7 @@ class SDCSchedulingModel { using DelayMap = absl::flat_hash_map; static constexpr double kInfinity = std::numeric_limits::infinity(); + static constexpr double kMaxStages = (1 << 20); public: SDCSchedulingModel(FunctionBase* func, const DelayMap& delay_map,