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; }