From aaf5383848187ea6d95e308c13def4f95be3a4e2 Mon Sep 17 00:00:00 2001 From: Angelo Matni Date: Mon, 22 Dec 2025 11:07:30 -0800 Subject: [PATCH] Reset the scheduler so that slack variables used to explain infeasibility do not corrupt future attempts to schedule. Also, always check if the user wants to try finding the minimum feasible clock period that would satisfy the scheduler while still propagating the scheduler's error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New example, more helpful error message: ``` Error: INVALID_ARGUMENT: cannot achieve the specified clock period. Try `--clock_period_ps=721`. The reason the scheduler gave is: INVALID_ARGUMENT: __alu__AluProc_0_next: cannot satisfy the given I/O constraints. Would succeed with: {alu__p3_data_ins→alu__p5_outs with maximum latency ≥ 3}, {alu__p3_ins→alu__p5_outs with maximum latency ≥ 3} ``` PiperOrigin-RevId: 847823548 --- xls/scheduling/BUILD | 2 + .../pipeline_scheduling_pass_test.cc | 111 ++++++++++++++++++ xls/scheduling/run_pipeline_schedule.cc | 48 +++++--- 3 files changed, 143 insertions(+), 18 deletions(-) diff --git a/xls/scheduling/BUILD b/xls/scheduling/BUILD index f693f321df..0fe1b8a337 100644 --- a/xls/scheduling/BUILD +++ b/xls/scheduling/BUILD @@ -685,6 +685,7 @@ cc_test( "@org_theopenroadproject//:opensta", ], deps = [ + ":pipeline_schedule", ":pipeline_scheduling_pass", ":scheduling_options", ":scheduling_pass", @@ -706,6 +707,7 @@ cc_test( "//xls/passes:pass_base", "@com_google_absl//absl/status:status_matchers", "@com_google_absl//absl/status:statusor", + "@com_google_absl//absl/strings:str_format", "@googletest//:gtest", ], ) diff --git a/xls/scheduling/pipeline_scheduling_pass_test.cc b/xls/scheduling/pipeline_scheduling_pass_test.cc index 0233595107..1348e26db7 100644 --- a/xls/scheduling/pipeline_scheduling_pass_test.cc +++ b/xls/scheduling/pipeline_scheduling_pass_test.cc @@ -22,6 +22,7 @@ #include "gtest/gtest.h" #include "absl/status/status_matchers.h" #include "absl/status/statusor.h" +#include "absl/strings/str_format.h" #include "xls/common/file/get_runfile_path.h" #include "xls/common/status/matchers.h" #include "xls/common/status/status_macros.h" @@ -37,6 +38,7 @@ #include "xls/ir/source_location.h" #include "xls/ir/value.h" #include "xls/passes/pass_base.h" +#include "xls/scheduling/pipeline_schedule.h" #include "xls/scheduling/scheduling_options.h" #include "xls/scheduling/scheduling_pass.h" @@ -431,5 +433,114 @@ TEST_F(PipelineSchedulingPassTest, MultiProcScopedChannels) { SchedulingOptions().pipeline_stages(2).schedule_all_procs(true))); } +TEST_F(PipelineSchedulingPassTest, MinimizeClockOnFailure) { + auto p = CreatePackage(); + FunctionBuilder fb("main", p.get()); + BValue x = fb.Param("x", p->GetBitsType(32)); + BValue y = fb.Param("y", p->GetBitsType(32)); + BValue add = fb.Add(x, y); + BValue div = fb.UDiv(x, y); + BValue div_two = fb.UDiv(x, div); + BValue ored = fb.Or({add, div_two}); + XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.BuildWithReturnValue(ored)); + + // Fail if trying to schedule with a clock period that is too small. + auto failed_scheduled_result = RunPipelineSchedulingPass( + f, SchedulingOptions().clock_period_ps(1).pipeline_stages(2)); + EXPECT_THAT(failed_scheduled_result.status().message(), + AllOf(HasSubstr("cannot achieve the specified clock period"), + HasSubstr("clock_period_ps=3"))); + + // Succeed if trying to schedule with a large enough clock period. + XLS_ASSERT_OK_AND_ASSIGN( + auto success_scheduled_result, + RunPipelineSchedulingPass( + f, SchedulingOptions().clock_period_ps(3).pipeline_stages(2))); + EXPECT_THAT(success_scheduled_result, + Pair(true, SchedulingContextWithElements(UnorderedElementsAre( + Pair(f, VerifiedPipelineSchedule()))))); + PipelineSchedule f_schedule = + success_scheduled_result.second.package_schedule().GetSchedule(f); + EXPECT_THAT( + f_schedule.GetCycleMap(), + UnorderedElementsAre(Pair(x.node(), 0), Pair(y.node(), 0), + Pair(add.node(), 0), Pair(div.node(), 0), + Pair(div_two.node(), 1), Pair(ored.node(), 1))); +} + +TEST_F(PipelineSchedulingPassTest, MinimizeClockOnFailureWithIOConstraints) { + auto p = CreatePackage(); + auto make_proc = [p = p.get()]( + std::string_view name, Channel* channel_in, + Channel* channel_out) -> absl::StatusOr { + ProcBuilder pb(name, p); + BValue tok = pb.Literal(Value::Token()); + BValue st = pb.StateElement("st", Value(UBits(0, 32))); + BValue recv = pb.Receive(channel_in, tok, SourceInfo(), "recv"); + BValue recv_tok = pb.TupleIndex(recv, 0); + BValue recv_data = pb.TupleIndex(recv, 1); + for (int i = 0; i < 3; ++i) { + recv_data = pb.Add(recv_data, pb.Literal(UBits(1, 32))); + recv_data = pb.Subtract(recv_data, pb.Literal(UBits(1, 32))); + } + BValue add_st = pb.Add(st, recv_data); + pb.Send(channel_out, recv_tok, add_st, SourceInfo(), "send"); + return pb.Build({add_st}); + }; + + XLS_ASSERT_OK_AND_ASSIGN( + Channel * ch0, p->CreateStreamingChannel("ch0", ChannelOps::kReceiveOnly, + p->GetBitsType(32))); + XLS_ASSERT_OK_AND_ASSIGN( + Channel * ch1, p->CreateStreamingChannel("ch1", ChannelOps::kSendReceive, + p->GetBitsType(32))); + XLS_ASSERT_OK_AND_ASSIGN( + Channel * ch2, p->CreateStreamingChannel("ch2", ChannelOps::kSendOnly, + p->GetBitsType(32))); + + XLS_ASSERT_OK(make_proc("proc0", ch0, ch1)); + XLS_ASSERT_OK(make_proc("proc1", ch1, ch2)); + + auto failed_result = RunPipelineSchedulingPass( + p.get(), SchedulingOptions() + .clock_period_ps(2) + .pipeline_stages(4) + .add_constraint(IOConstraint( + /*source_channel=*/"ch0", + /*source_direction=*/IODirection::kReceive, + /*target_channel=*/"ch1", + /*target_direction=*/IODirection::kSend, + /*minimum_latency=*/1, /*maximum_latency=*/1)) + .add_constraint(IOConstraint( + /*source_channel=*/"ch1", + /*source_direction=*/IODirection::kReceive, + /*target_channel=*/"ch2", + /*target_direction=*/IODirection::kSend, + /*minimum_latency=*/1, /*maximum_latency=*/1))); + EXPECT_THAT( + failed_result.status().message(), + AllOf(HasSubstr("cannot achieve the specified clock period"), + HasSubstr("clock_period_ps=4"), + HasSubstr("cannot satisfy the given I/O constraints. Would succeed " + "with: {ch0→ch1 with maximum latency ≥ 3}"))); + + XLS_ASSERT_OK(RunPipelineSchedulingPass( + p.get(), SchedulingOptions() + .clock_period_ps(4) + .pipeline_stages(4) + .add_constraint(IOConstraint( + /*source_channel=*/"ch0", + /*source_direction=*/IODirection::kReceive, + /*target_channel=*/"ch1", + /*target_direction=*/IODirection::kSend, + /*minimum_latency=*/1, /*maximum_latency=*/1)) + .add_constraint(IOConstraint( + /*source_channel=*/"ch1", + /*source_direction=*/IODirection::kReceive, + /*target_channel=*/"ch2", + /*target_direction=*/IODirection::kSend, + /*minimum_latency=*/1, /*maximum_latency=*/1)))); +} + } // namespace } // namespace xls diff --git a/xls/scheduling/run_pipeline_schedule.cc b/xls/scheduling/run_pipeline_schedule.cc index 85bb3e395d..46fc664ad8 100644 --- a/xls/scheduling/run_pipeline_schedule.cc +++ b/xls/scheduling/run_pipeline_schedule.cc @@ -468,12 +468,20 @@ absl::StatusOr RunPipelineScheduleInternal( } std::unique_ptr sdc_scheduler; + // TODO: replace with handling reverting model constraint changes made while + // trying to explain infeasibility, thereby never needing to invalidate the + // SDCScheduler object regardless of settings. + bool invalidated = false; auto initialize_sdc_scheduler = [&]() -> absl::Status { - if (sdc_scheduler == nullptr) { - XLS_ASSIGN_OR_RETURN(sdc_scheduler, - SDCScheduler::Create(f, io_delay_added)); - XLS_RETURN_IF_ERROR(sdc_scheduler->AddConstraints(options.constraints())); + // We need to recreate the SDC scheduler because slack variables may be + // introduced if trying to explain infeasibility upon failing to schedule. + if (sdc_scheduler != nullptr && invalidated) { + sdc_scheduler.reset(nullptr); + invalidated = false; } + XLS_ASSIGN_OR_RETURN(sdc_scheduler, + SDCScheduler::Create(f, io_delay_added)); + XLS_RETURN_IF_ERROR(sdc_scheduler->AddConstraints(options.constraints())); return absl::OkStatus(); }; @@ -620,12 +628,9 @@ absl::StatusOr RunPipelineScheduleInternal( /*check_feasibility=*/false, worst_case_throughput, options.dynamic_throughput_objective_weight()); + invalidated = !schedule_cycle_map.ok() && + options.failure_behavior().explain_infeasibility; if (!schedule_cycle_map.ok()) { - if (absl::IsInvalidArgument(schedule_cycle_map.status())) { - // The scheduler was able to explain the failure; report it up without - // further analysis. - return std::move(schedule_cycle_map).status(); - } if (options.clock_period_ps().has_value()) { // The user specified a specific clock period; see if we can confirm // that that's the issue. @@ -649,8 +654,9 @@ absl::StatusOr RunPipelineScheduleInternal( // Just increasing the clock period suffices. return absl::InvalidArgumentError(absl::StrFormat( "cannot achieve the specified clock period. Try " - "`--clock_period_ps=%d`.", - *min_clock_period_ps)); + "`--clock_period_ps=%d`. The reason the scheduler gave is: " + "%v", + *min_clock_period_ps, schedule_cycle_map.status())); } if (absl::IsInvalidArgument(min_clock_period_ps.status())) { // We failed with an explained error at the longest possible clock @@ -659,8 +665,10 @@ absl::StatusOr RunPipelineScheduleInternal( return xabsl::StatusBuilder(std::move(min_clock_period_ps).status()) .SetPrepend() << absl::StrFormat( - "cannot achieve the specified clock period; try " - "increasing `--clock_period_ps`. Also, "); + "cannot achieve the specified clock period; the " + "reason the scheduler gave is: %v. Try " + "increasing `--clock_period_ps`. Also, ", + schedule_cycle_map.status()); } // We fail with an unexplained error even at the longest possible // clock period. Report the original error. @@ -686,9 +694,11 @@ absl::StatusOr RunPipelineScheduleInternal( .status(); if (pessimistic_status.ok()) { // Just increasing the clock period suffices. - return absl::InvalidArgumentError( - "cannot achieve the specified clock period. Try increasing " - "`--clock_period_ps`."); + return absl::InvalidArgumentError(absl::StrFormat( + "cannot achieve the specified clock period. The reason the " + "scheduler gave is: %v. Try increasing " + "`--clock_period_ps`.", + schedule_cycle_map.status())); } if (absl::IsInvalidArgument(pessimistic_status)) { // We failed with an explained error at the pessimistic clock period. @@ -697,8 +707,10 @@ absl::StatusOr RunPipelineScheduleInternal( return xabsl::StatusBuilder(std::move(pessimistic_status)) .SetPrepend() << absl::StrFormat( - "cannot achieve the specified clock period; try " - "increasing `--clock_period_ps`. Also, "); + "cannot achieve the specified clock period; the reason " + "the scheduler gave is: %v. Try " + "increasing `--clock_period_ps`. Also, ", + schedule_cycle_map.status()); } return pessimistic_status; }