Avoid adding cycle constraints for untimed nodes in pipeline scheduling.

Untimed nodes (e.g., literal) do not have a specific cycle and should not be subject to NodeInCycleConstraints. This change prevents adding such constraints for untimed nodes during pipeline scheduling.

PiperOrigin-RevId: 916141903

Author: allight

xls/scheduling/BUILD

@@ -348,6 +348,7 @@ cc_test(
     deps = [
         ":schedule_bounds",
         ":schedule_graph",
+        ":scheduling_options",
         "//xls/common:xls_gunit_main",
         "//xls/common/status:matchers",
         "//xls/common/status:status_macros",
@@ -691,6 +692,7 @@ cc_library(
     deps = [
         ":pipeline_schedule",
         ":run_pipeline_schedule",
+        ":schedule_graph",
         ":scheduling_options",
         ":scheduling_pass",
         "//xls/common/status:ret_check",

xls/scheduling/pipeline_schedule.cc

@@ -265,17 +265,32 @@ absl::Status PipelineSchedule::Verify() const {
       continue;
     }
     XLS_RET_CHECK(IsScheduled(node));
+    if (node->Is<MinDelay>()) {
+      // NB Our SDC scheduler will usually put the min-delay after its operand
+      // but the ASAP and potentially other schedulers (due to the specific way
+      // they represent delay and the constraints the min-delay node represents)
+      // may place it in the same cycle as the operand or even place it
+      // somewhere in between the operand and the nodes users. To allow for any
+      // choice we check the difference between the users and the operand
+      // directly and don't care about the min-delay itself.
+      int64_t cycle_operand = cycle(node->operand(0));
+      for (Node* user : node->users()) {
+        XLS_RET_CHECK_LE(cycle_operand,
+                         cycle(user) - node->As<MinDelay>()->delay())
+            << "The user '" << user->ToString() << "' at cycle " << cycle(user)
+            << " of the MinDelay node " << node
+            << " is not correctly scheduled. It is not "
+            << node->As<MinDelay>()->delay()
+            << " cycles after the min-delay operand " << node->operand(0)
+            << " at cycle " << cycle_operand << ".";
+      }
+    }
     for (Node* operand : node->operands()) {
       if (IsUntimed(operand)) {
         continue;
       }
       XLS_RET_CHECK_LE(cycle(operand), cycle(node))
           << "Operand " << operand << " scheduled after user " << node;
-
-      if (node->Is<MinDelay>()) {
-        XLS_RET_CHECK_LE(cycle(operand),
-                         cycle(node) - node->As<MinDelay>()->delay());
-      }
     }
   }
   if (function_base()->IsProc()) {

xls/scheduling/pipeline_scheduling_pass.cc

@@ -32,6 +32,7 @@
 #include "xls/passes/pass_base.h"
 #include "xls/scheduling/pipeline_schedule.h"
 #include "xls/scheduling/run_pipeline_schedule.h"
+#include "xls/scheduling/schedule_graph.h"
 #include "xls/scheduling/scheduling_options.h"
 #include "xls/scheduling/scheduling_pass.h"
 
@@ -61,7 +62,7 @@ absl::Status AddCycleConstraints(const PipelineSchedule& schedule,
           }
         }
       }
-      if (!already_constrained) {
+      if (!already_constrained && !IsUntimed(node)) {
         scheduling_options.add_constraint(NodeInCycleConstraint(node, c));
       }
     }

xls/scheduling/schedule_bounds.cc

@@ -88,6 +88,10 @@ class ConstraintConverter {
                       constraint);
   }
 
+  void AddNodeConstraint(const NodeSchedulingConstraint& constraint) {
+    node_constraints_.push_back(constraint);
+  }
+
   absl::Status AddSpecificConstraint(const NodeInCycleConstraint& nic) {
     node_constraints_.emplace_back(nic);
     return absl::OkStatus();
@@ -381,6 +385,18 @@ ScheduleBounds::ConvertSchedulingConstraints(
     XLS_RETURN_IF_ERROR(converter.AddConstraint(constraint))
         << "Could not add constraint for " << graph.name();
   }
+  // Finally add any min-delay constraints as a final pass.
+  for (const ScheduleNode& node : graph.nodes()) {
+    if (node.node->Is<MinDelay>()) {
+      converter.AddNodeConstraint(
+          NodeSchedulingConstraint(NodeDifferenceConstraint{
+              .anchor = node.node->operand(0),
+              .subject = node.node,
+              .min_after = node.node->As<MinDelay>()->delay(),
+              .max_after = max_upper_bound,
+          }));
+    }
+  }
   XLS_ASSIGN_OR_RETURN(std::vector<NodeSchedulingConstraint> node_constraints,
                        std::move(converter).Finalize());
   VLOG(2) << "  Node constraints are: ["
@@ -557,14 +573,13 @@ absl::StatusOr<std::optional<int64_t>> PropagateGenericBounds(
         node_in_cycle_delay = in_cycle_delay.at(operand.node) + operand_delay;
         continue;
       }
-      int64_t min_delay = operand.node->Is<MinDelay>()
-                              ? operand.node->As<MinDelay>()->delay()
-                              : 0;
-      if (operand_cb + min_delay > node_cb) {
+      // NB MinDelay nodes are handled by constraints so no need to do anything
+      // here.
+      if (operand_cb > node_cb) {
         VLOG(4) << absl::StreamFormat(
             "    tightened lb to %d because of operand %s", operand_cb,
             operand.node->GetName());
-        XLS_RETURN_IF_ERROR(TightenBound(node_cb, operand_cb + min_delay));
+        XLS_RETURN_IF_ERROR(TightenBound(node_cb, operand_cb));
         changed = true;
         node_in_cycle_delay = 0;
         continue;

xls/scheduling/schedule_bounds_test.cc

@@ -42,6 +42,7 @@
 #include "xls/ir/source_location.h"
 #include "xls/ir/value.h"
 #include "xls/scheduling/schedule_graph.h"
+#include "xls/scheduling/scheduling_options.h"
 
 namespace m = xls::op_matchers;
 namespace xls {
@@ -502,6 +503,138 @@ TEST_F(ScheduleBoundsTest, PushNodeLaterChain) {
   EXPECT_THAT(sched.bounds(h.node()), Bounds(7, 7));
 }
 
+TEST_F(ScheduleBoundsTest, MinDelayBasic) {
+  auto p = CreatePackage();
+  FunctionBuilder fb(TestName(), p.get());
+  auto a = fb.Param("a", p->GetBitsType(32));
+  auto b = fb.Param("b", p->GetBitsType(32));
+  fb.Add(a, b);
+  auto tkn = fb.Literal(Value::Token());
+  auto min_delay = fb.MinDelay(tkn, /*delay=*/3);
+  auto assert = fb.Assert(min_delay, fb.Literal(UBits(1, 1)), "msg");
+  XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.Build());
+
+  ControllableDelayEstimator delay;
+  delay.SetDelay(m::Add(), 1);
+  delay.SetDelay(Op::kMinDelay, 0);
+  delay.SetDelay(Op::kAssert, 0);
+  delay.SetDelay(FreeOperations(), 0);
+
+  XLS_ASSERT_OK_AND_ASSIGN(
+      ScheduleBounds sched,
+      ScheduleBounds::ComputeAsapAndAlapBounds(f,
+                                               /*clock_period_ps=*/3, delay));
+  EXPECT_THAT(sched.bounds(min_delay.node()), Bounds(3, 3));
+  EXPECT_THAT(sched.bounds(assert.node()), Bounds(3, 3));
+}
+
+TEST_F(ScheduleBoundsTest, MinDelayChain) {
+  auto p = CreatePackage();
+  FunctionBuilder fb(TestName(), p.get());
+  auto tkn = fb.Literal(Value::Token());
+  auto md1 = fb.MinDelay(tkn, /*delay=*/2);
+  auto md2 = fb.MinDelay(md1, /*delay=*/3);
+  fb.Assert(md2, fb.Literal(UBits(1, 1)), "msg");
+  XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.Build());
+
+  ControllableDelayEstimator delay;
+  delay.SetDelay(Op::kMinDelay, 0);
+  delay.SetDelay(Op::kAssert, 0);
+  delay.SetDelay(FreeOperations(), 0);
+
+  XLS_ASSERT_OK_AND_ASSIGN(
+      ScheduleBounds sched,
+      ScheduleBounds::ComputeAsapAndAlapBounds(f,
+                                               /*clock_period_ps=*/3, delay));
+  EXPECT_THAT(sched.bounds(md1.node()), Bounds(2, 2));
+  EXPECT_THAT(sched.bounds(md2.node()), Bounds(5, 5));
+}
+
+TEST_F(ScheduleBoundsTest, MinDelayAndIIInteraction) {
+  auto p = CreatePackage();
+  ProcBuilder pb(NewStyleProc(), TestName(), p.get());
+  XLS_ASSERT_OK_AND_ASSIGN(ReceiveChannelInterface * ch_in,
+                           pb.AddInputChannel("in", p->GetBitsType(32)));
+  XLS_ASSERT_OK_AND_ASSIGN(SendChannelInterface * ch_out,
+                           pb.AddOutputChannel("out", p->GetBitsType(32)));
+
+  BValue state = pb.StateElement("st", Value(UBits(0, 32)));
+  BValue tkn = pb.Literal(Value::Token());
+  BValue rcv = pb.Receive(ch_in, tkn);
+  BValue rcv_tkn = pb.TupleIndex(rcv, 0);
+  BValue rcv_data = pb.TupleIndex(rcv, 1);
+  BValue md = pb.MinDelay(rcv_tkn, /*delay=*/3);
+  BValue send = pb.Send(ch_out, md, rcv_data);
+  // Make the next state depend on send token so backedge constraint covers the
+  // whole chain.
+  BValue next_state = pb.Add(state, pb.TupleIndex(pb.Receive(ch_in, send), 1));
+  pb.Next(state, next_state);
+  XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build());
+
+  ControllableDelayEstimator delay;
+  delay.SetDelay(m::Add(), 1);
+  delay.SetDelay(Op::kMinDelay, 0);
+  delay.SetDelay(FreeOperations(), 0);
+
+  // Set II = 4.
+  // Backedge constraint: Next - state <= II - 1 = 3.
+  // MinDelay constraint: send >= md >= rcv_tkn + 3.
+  // Since rcv_tkn is derived from tkn (state read), this forces send to be
+  // scheduled at least 3 cycles after the receive.
+  XLS_ASSERT_OK_AND_ASSIGN(
+      ScheduleBounds sched,
+      ScheduleBounds::ComputeAsapAndAlapBounds(
+          proc,
+          /*clock_period_ps=*/3, delay, /*ii=*/4, {BackedgeConstraint()}));
+  EXPECT_THAT(sched.bounds(md.node()), Bounds(3, 3));
+  EXPECT_THAT(sched.bounds(send.node()), Bounds(3, 3));
+}
+
+TEST_F(ScheduleBoundsTest, MinDelayAndIOConstraintConflict) {
+  auto p = CreatePackage();
+  ProcBuilder pb(NewStyleProc(), TestName(), p.get());
+  XLS_ASSERT_OK_AND_ASSIGN(ReceiveChannelInterface * ch_a,
+                           pb.AddInputChannel("a", p->GetBitsType(32)));
+  XLS_ASSERT_OK_AND_ASSIGN(SendChannelInterface * ch_b,
+                           pb.AddOutputChannel("b", p->GetBitsType(32)));
+  XLS_ASSERT_OK_AND_ASSIGN(SendChannelInterface * ch_c,
+                           pb.AddOutputChannel("c", p->GetBitsType(32)));
+
+  BValue tkn = pb.Literal(Value::Token());
+  BValue rcv_a = pb.Receive(ch_a, tkn);
+  BValue rcv_a_tkn = pb.TupleIndex(rcv_a, 0);
+  BValue rcv_a_data = pb.TupleIndex(rcv_a, 1);
+
+  BValue md1 = pb.MinDelay(rcv_a_tkn, /*delay=*/2);
+  BValue send_b = pb.Send(ch_b, md1, rcv_a_data);
+
+  BValue md2 = pb.MinDelay(send_b, /*delay=*/2);
+  pb.Send(ch_c, md2, rcv_a_data);
+
+  XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build());
+
+  ControllableDelayEstimator delay;
+  delay.SetDelay(Op::kMinDelay, 0);
+  delay.SetDelay(FreeOperations(), 0);
+
+  // IO constraint says send_c (Channel "c") must be at most 3 cycles after
+  // rcv_a (Channel "a"). MinDelay requires:
+  //   send_b >= rcv_a + 2
+  //   send_c >= send_b + 2
+  // So send_c >= rcv_a + 4.
+  // This conflicts with maximum_latency = 3.
+  IOConstraint io_const("a", IODirection::kReceive, "c", IODirection::kSend,
+                        /*minimum_latency=*/0, /*maximum_latency=*/3);
+
+  EXPECT_THAT(
+      ScheduleBounds::ComputeAsapAndAlapBounds(proc,
+                                               /*clock_period_ps=*/3, delay,
+                                               /*ii=*/std::nullopt, {io_const}),
+      StatusIs(absl::StatusCode::kInternal,
+               ContainsRegex(".*failed to converge.*|.*potentially "
+                             "incompatible with constraint.*")));
+}
+
 TEST_F(ScheduleBoundsTest, StringifyConstraints) {
   auto p = CreatePackage();
   FunctionBuilder fb(TestName(), p.get());