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

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

Author: angelomatni1

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",
     ],
 )

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<Proc*> {
+    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

xls/scheduling/run_pipeline_schedule.cc

@@ -468,12 +468,20 @@ absl::StatusOr<PipelineSchedule> RunPipelineScheduleInternal(
   }
 
   std::unique_ptr<SDCScheduler> 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<PipelineSchedule> 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<PipelineSchedule> 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<PipelineSchedule> 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<PipelineSchedule> 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<PipelineSchedule> 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;
       }