From 81ff4fdf7faeafac2d5763a322cf9a39052100c8 Mon Sep 17 00:00:00 2001 From: Alex Light Date: Sat, 23 May 2026 11:09:24 -0700 Subject: [PATCH] Automated rollback of commit 85608789b2232eb2742e1d912eb3b63443da0cfe. PiperOrigin-RevId: 920228436 --- docs_src/codegen_options.md | 9 +- xls/build_rules/xls_providers.bzl | 1 - xls/fdo/iterative_sdc_scheduler.h | 4 +- xls/modules/zstd/BUILD | 2 - xls/scheduling/BUILD | 143 +-- xls/scheduling/asap_scheduler.cc | 183 ---- xls/scheduling/asap_scheduler.h | 87 -- xls/scheduling/asap_scheduler_test.cc | 135 --- xls/scheduling/min_cut_scheduler.cc | 76 +- xls/scheduling/min_cut_scheduler.h | 30 +- xls/scheduling/min_cut_scheduler_test.cc | 55 +- xls/scheduling/pipeline_schedule.cc | 3 - xls/scheduling/pipeline_schedule_test.cc | 953 +++++++----------- .../pipeline_scheduling_pass_test.cc | 5 - xls/scheduling/random_scheduler.cc | 100 -- xls/scheduling/random_scheduler.h | 78 -- xls/scheduling/random_scheduler_test.cc | 115 --- xls/scheduling/run_pipeline_schedule.cc | 767 +++++++------- xls/scheduling/schedule_bounds.cc | 5 - xls/scheduling/scheduler.h | 73 -- xls/scheduling/scheduling_options.cc | 12 - xls/scheduling/scheduling_options.h | 39 +- xls/scheduling/sdc_scheduler.cc | 106 +- xls/scheduling/sdc_scheduler.h | 55 +- xls/scheduling/sdc_scheduler_test.cc | 151 --- xls/tools/scheduling_options_flags.cc | 28 +- xls/tools/scheduling_options_flags.proto | 2 - 27 files changed, 847 insertions(+), 2370 deletions(-) delete mode 100644 xls/scheduling/asap_scheduler.cc delete mode 100644 xls/scheduling/asap_scheduler.h delete mode 100644 xls/scheduling/asap_scheduler_test.cc delete mode 100644 xls/scheduling/random_scheduler.cc delete mode 100644 xls/scheduling/random_scheduler.h delete mode 100644 xls/scheduling/random_scheduler_test.cc delete mode 100644 xls/scheduling/scheduler.h delete mode 100644 xls/scheduling/sdc_scheduler_test.cc diff --git a/docs_src/codegen_options.md b/docs_src/codegen_options.md index 3f754b59a3..5ee7c7a82d 100644 --- a/docs_src/codegen_options.md +++ b/docs_src/codegen_options.md @@ -213,14 +213,7 @@ control the scheduler. - `min_cut`: Approximates the minimum number of registers using a min-cut based algorithm. Does not respect io_constraints. - - `random`: Internal testing algorithm that selects stages randomly. Does - respect the same io constraints that `asap` does. - -- `--find_bounds_strategy` controls what scheduler algorithm is used to - determine feasible boundary conditions for scheduling, such as minimum clock - period or minimum worst case throughput. By default it will use the same - scheduler as `--scheduling_strategy` but in many cases `asap` will provide - faster results without any loss of accuracy. + - `random`: Internal testing algorithm that selects stages randomly. # Feedback-driven Optimization (FDO) Options diff --git a/xls/build_rules/xls_providers.bzl b/xls/build_rules/xls_providers.bzl index 3a73901a71..c307d0f2ff 100644 --- a/xls/build_rules/xls_providers.bzl +++ b/xls/build_rules/xls_providers.bzl @@ -149,7 +149,6 @@ CODEGEN_FIELDS = { "source_annotation_strategy": "The strategy to use for generating source annotations. Use 'comment'" + "to generate comments, 'directive' to generate SystemVerilog `line directives, " + "or 'none'", - "find_bounds_strategy": "Scheduler algorithm to use for finding minimum bounds.", } SCHEDULING_FIELDS = { diff --git a/xls/fdo/iterative_sdc_scheduler.h b/xls/fdo/iterative_sdc_scheduler.h index 23d16e5122..a784237fdf 100644 --- a/xls/fdo/iterative_sdc_scheduler.h +++ b/xls/fdo/iterative_sdc_scheduler.h @@ -38,9 +38,9 @@ class IterativeSDCSchedulingModel : public SDCSchedulingModel { public: // Delay map is no longer needed as the delay calculation is completely // handled by the delay manager. - IterativeSDCSchedulingModel(const ScheduleGraph& graph, + IterativeSDCSchedulingModel(ScheduleGraph graph, const DelayManager& delay_manager) - : SDCSchedulingModel(graph, DelayMap(), + : SDCSchedulingModel(std::move(graph), DelayMap(), /*initiation_interval=*/std::nullopt, // Use kDefaultSdcSolutionTolerance as the tolerance // as we unconditionally use glop with iterative diff --git a/xls/modules/zstd/BUILD b/xls/modules/zstd/BUILD index 00fa1cb127..396f873b67 100644 --- a/xls/modules/zstd/BUILD +++ b/xls/modules/zstd/BUILD @@ -44,8 +44,6 @@ COMMON_CODEGEN_ARGS = { "clock_margin_percent": "20", "multi_proc": "true", "materialize_internal_fifos": "true", - "scheduling_strategy": "sdc", - "find_bounds_strategy": "asap", } xls_dslx_library( diff --git a/xls/scheduling/BUILD b/xls/scheduling/BUILD index 10dbb69a01..3ed729d8c5 100644 --- a/xls/scheduling/BUILD +++ b/xls/scheduling/BUILD @@ -136,11 +136,8 @@ cc_library( srcs = ["min_cut_scheduler.cc"], hdrs = ["min_cut_scheduler.h"], deps = [ - ":asap_scheduler", ":function_partition", ":schedule_bounds", - ":schedule_graph", - ":schedule_util", ":scheduling_options", "//xls/common/logging:log_lines", "//xls/common/status:ret_check", @@ -148,7 +145,6 @@ cc_library( "//xls/estimators/delay_model:delay_estimator", "//xls/ir", "@com_google_absl//absl/container:flat_hash_map", - "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/log", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", @@ -163,16 +159,7 @@ cc_test( srcs = ["min_cut_scheduler_test.cc"], deps = [ ":min_cut_scheduler", - ":schedule_graph", - ":scheduling_options", "//xls/common:xls_gunit_main", - "//xls/common/status:matchers", - "//xls/estimators/delay_model:delay_estimator", - "//xls/ir:bits", - "//xls/ir:function_builder", - "//xls/ir:ir_test_base", - "//xls/ir:value", - "@com_google_absl//absl/status:statusor", "@googletest//:gtest", ], ) @@ -185,7 +172,6 @@ cc_library( ":schedule_bounds", ":schedule_graph", ":schedule_util", - ":scheduler", ":scheduling_options", "//xls/common/status:ret_check", "//xls/common/status:status_macros", @@ -213,27 +199,6 @@ cc_library( ], ) -cc_test( - name = "sdc_scheduler_test", - srcs = ["sdc_scheduler_test.cc"], - deps = [ - ":schedule_graph", - ":scheduling_options", - ":sdc_scheduler", - "//xls/common:xls_gunit_main", - "//xls/common/status:matchers", - "//xls/estimators/delay_model:delay_estimator", - "//xls/ir:bits", - "//xls/ir:channel", - "//xls/ir:channel_ops", - "//xls/ir:function_builder", - "//xls/ir:ir_test_base", - "//xls/ir:value", - "@com_google_absl//absl/status:statusor", - "@googletest//:gtest", - ], -) - cc_library( name = "schedule_util", srcs = ["schedule_util.cc"], @@ -304,14 +269,11 @@ cc_library( srcs = ["run_pipeline_schedule.cc"], hdrs = ["run_pipeline_schedule.h"], deps = [ - ":asap_scheduler", ":min_cut_scheduler", ":pipeline_schedule", - ":random_scheduler", ":schedule_bounds", ":schedule_graph", ":schedule_util", - ":scheduler", ":scheduling_options", ":sdc_scheduler", "//xls/common:math_util", @@ -336,7 +298,7 @@ cc_library( "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/log", "@com_google_absl//absl/log:check", - "@com_google_absl//absl/random", + "@com_google_absl//absl/random:distributions", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", @@ -348,7 +310,6 @@ cc_library( cc_test( name = "pipeline_schedule_test", srcs = ["pipeline_schedule_test.cc"], - shard_count = 5, deps = [ ":pipeline_schedule", ":pipeline_schedule_cc_proto", @@ -795,8 +756,6 @@ cc_test( ":scheduling_pass", "//xls/common:xls_gunit_main", "//xls/common/file:get_runfile_path", - "//xls/common/logging:scoped_record_logs", - "//xls/common/logging:scoped_vlog_level", "//xls/common/status:matchers", "//xls/common/status:status_macros", "//xls/fdo:synthesizer", @@ -819,103 +778,3 @@ cc_test( "@googletest//:gtest", ], ) - -cc_library( - name = "scheduler", - hdrs = ["scheduler.h"], - deps = [ - ":schedule_bounds", - ":scheduling_options", - "@com_google_absl//absl/status", - "@com_google_absl//absl/status:statusor", - "@com_google_absl//absl/types:span", - ], -) - -cc_library( - name = "asap_scheduler", - srcs = ["asap_scheduler.cc"], - hdrs = ["asap_scheduler.h"], - deps = [ - ":schedule_bounds", - ":schedule_graph", - ":scheduler", - ":scheduling_options", - "//xls/common/status:ret_check", - "//xls/common/status:status_macros", - "//xls/estimators/delay_model:delay_estimator", - "//xls/ir", - "@com_google_absl//absl/log", - "@com_google_absl//absl/status", - "@com_google_absl//absl/status:statusor", - "@com_google_absl//absl/strings", - "@com_google_absl//absl/strings:str_format", - "@com_google_absl//absl/types:span", - ], -) - -cc_test( - name = "asap_scheduler_test", - srcs = ["asap_scheduler_test.cc"], - deps = [ - ":asap_scheduler", - ":schedule_graph", - ":scheduling_options", - "//xls/common:xls_gunit_main", - "//xls/common/status:matchers", - "//xls/estimators/delay_model:delay_estimator", - "//xls/ir:bits", - "//xls/ir:channel", - "//xls/ir:channel_ops", - "//xls/ir:function_builder", - "//xls/ir:ir_test_base", - "//xls/ir:value", - "@com_google_absl//absl/status:statusor", - "@googletest//:gtest", - ], -) - -cc_library( - name = "random_scheduler", - srcs = ["random_scheduler.cc"], - hdrs = ["random_scheduler.h"], - deps = [ - ":asap_scheduler", - ":schedule_bounds", - ":schedule_graph", - ":scheduler", - ":scheduling_options", - "//xls/common/status:ret_check", - "//xls/common/status:status_macros", - "//xls/estimators/delay_model:delay_estimator", - "//xls/ir", - "@com_google_absl//absl/algorithm:container", - "@com_google_absl//absl/log", - "@com_google_absl//absl/random", - "@com_google_absl//absl/status", - "@com_google_absl//absl/status:statusor", - "@com_google_absl//absl/types:span", - ], -) - -cc_test( - name = "random_scheduler_test", - srcs = ["random_scheduler_test.cc"], - deps = [ - ":random_scheduler", - ":schedule_graph", - ":scheduling_options", - "//xls/common:xls_gunit_main", - "//xls/common/status:matchers", - "//xls/estimators/delay_model:delay_estimator", - "//xls/ir:bits", - "//xls/ir:channel", - "//xls/ir:channel_ops", - "//xls/ir:function_builder", - "//xls/ir:ir_test_base", - "//xls/ir:value", - "@com_google_absl//absl/random", - "@com_google_absl//absl/status:statusor", - "@googletest//:gtest", - ], -) diff --git a/xls/scheduling/asap_scheduler.cc b/xls/scheduling/asap_scheduler.cc deleted file mode 100644 index 51fb332db1..0000000000 --- a/xls/scheduling/asap_scheduler.cc +++ /dev/null @@ -1,183 +0,0 @@ -// Copyright 2026 The XLS Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "xls/scheduling/asap_scheduler.h" - -#include -#include -#include -#include -#include - -#include "absl/log/log.h" -#include "absl/status/status.h" -#include "absl/status/statusor.h" -#include "absl/strings/str_format.h" -#include "absl/strings/str_join.h" -#include "xls/common/status/ret_check.h" -#include "xls/common/status/status_macros.h" -#include "xls/ir/function.h" -#include "xls/ir/function_base.h" -#include "xls/ir/node.h" -#include "xls/ir/nodes.h" -#include "xls/scheduling/schedule_bounds.h" -#include "xls/scheduling/schedule_graph.h" -#include "xls/scheduling/scheduling_options.h" - -namespace xls { -namespace { - -// Returns the nodes of `f` which must be scheduled in the first stage of a -// pipeline. For functions this is parameters. -std::vector FirstStageNodes(FunctionBase* f) { - if (Function* function = dynamic_cast(f)) { - return std::vector(function->params().begin(), - function->params().end()); - } - - return {}; -} -// Returns the nodes of `f` which must be scheduled in the final stage of a -// pipeline. For functions this is the return value. -std::vector FinalStageNodes(FunctionBase* f) { - if (Function* function = dynamic_cast(f)) { - // If the return value is a parameter, then we do not force the return value - // to be scheduled in the final stage because, as a parameter, the node must - // be in the first stage. - if (function->return_value()->Is()) { - return {}; - } - return {function->return_value()}; - } - - return {}; -} - -} // namespace - -// Tighten `bounds` to the ASAP/ALAP bounds for each node. If `schedule_length` -// is given, then the ALAP bounds are computed with the given length. Otherwise, -// we use the minimum viable pipeline length, per the ASAP bounds. -// -// Both schedules will be feasible if no other scheduling constraints are -// applied. -/* static */ absl::Status ASAPScheduler::TightenBounds( - sched::ScheduleBounds& bounds, FunctionBase* f, - std::optional schedule_length) { - // If we have a schedule length give everything that upper bound. - if (schedule_length.has_value()) { - for (Node* node : f->nodes()) { - XLS_RETURN_IF_ERROR( - bounds.TightenNodeUb(node, schedule_length.value() - 1)); - } - } - // If we have a schedule_length (and therefore a small-ish max stage) we can - // try to propagate forever since we will hit that schedule length before - // long. - std::optional propagation_fuel = - schedule_length ? std::nullopt : std::make_optional(6); - XLS_RETURN_IF_ERROR(bounds.PropagateBounds(propagation_fuel)) - << "Failed to schedule bounds for " << f->name() << "."; - VLOG(5) << "ASAP bounds: " - << f->DumpIr(sched::ScheduleBoundsAnnotator(bounds)); - return absl::OkStatus(); -} -absl::Status ASAPScheduler::GenerateHelpfulError( - absl::Status&& orig_status, std::optional pipeline_stages, - int64_t clock_period_ps, std::optional worst_case_throughput) { - xabsl::StatusBuilder status(std::move(orig_status)); - // Try to figure out what the actual required stages are. - if (pipeline_stages.has_value()) { - auto no_length = ComputeBounds(/*pipeline_stages=*/std::nullopt, - clock_period_ps, worst_case_throughput); - no_length.IgnoreError(); - if (no_length.ok()) { - return (status << absl::StrFormat( - "Function %s cannot be scheduled in %d " - "stages. Computed minimum is --pipeline_stages=%d", - graph().name(), pipeline_stages.value(), - no_length->max_lower_bound() + 1)) - .SetCode(absl::StatusCode::kInvalidArgument); - } - return (status << "Function " << graph().name() - << " cannot be scheduled in any number of stages via ASAP " - "bounds. Some constraints may be unsatisfiable or " - "require a full SDC schedule to resolve.") - .SetCode(absl::StatusCode::kResourceExhausted); - } - return status; -} - -absl::StatusOr ASAPScheduler::Schedule( - std::optional pipeline_stages, int64_t clock_period_ps, - SchedulingFailureBehavior failure_behavior, - std::optional worst_case_throughput) { - XLS_ASSIGN_OR_RETURN( - sched::ScheduleBounds bounds, - ComputeBounds(pipeline_stages, clock_period_ps, worst_case_throughput)); - ScheduleCycleMap cycle_map; - cycle_map.reserve(graph().nodes().size()); - XLS_RET_CHECK(!pipeline_stages.has_value() || pipeline_stages == 1 || - pipeline_stages >= bounds.max_lower_bound() + 1) - << "Pipeline stages must be at least 1 or greater than or equal to " - "the number of stages in the function. pipeline_stages: " - << (pipeline_stages ? *pipeline_stages : -1) - << " max_lower_bound: " << bounds.max_lower_bound(); - // Just schedule everything as soon as possible. - for (const auto& sn : graph().nodes()) { - cycle_map[sn.node] = bounds.lb(sn.node); - } - return cycle_map; -} - -absl::StatusOr ASAPScheduler::ComputeBounds( - std::optional pipeline_stages, int64_t clock_period_ps, - std::optional worst_case_throughput) { - XLS_RET_CHECK(std::holds_alternative(graph_.ir_scope())); - auto* f = std::get(graph_.ir_scope()); - // TODO(allight): This actually creates a copy of graph_ since it needs to - // have its topo-sort changed as constraints are added. Since that list is - // held with the graph it needs its own mutable copy. Since the graph is not - // actually terribly large and this copy is only done once per call this is - // fine. - VLOG(5) << "ASAP scheduler: graph: " << graph_.name(); - VLOG(5) << " clock_period_ps: " << clock_period_ps; - VLOG(5) << " worst_case_throughput: " - << worst_case_throughput.value_or(-1); - VLOG(5) << " constraints: " - << absl::StrJoin(constraints_, ", "); - XLS_ASSIGN_OR_RETURN( - auto bounds, - sched::ScheduleBounds::Create(graph_, clock_period_ps, delay_estimator_, - worst_case_throughput, constraints_)); - // Add first and last stage constraints. - using LastStageConstraint = - sched::ScheduleBounds::NodeSchedulingConstraint::LastStageConstraint; - for (Node* n : FirstStageNodes(f)) { - bounds.AddConstraint(NodeInCycleConstraint{n, 0}); - } - for (Node* n : FinalStageNodes(f)) { - bounds.AddConstraint(LastStageConstraint{n}); - } - absl::Status tighten_bounds_status = - TightenBounds(bounds, f, pipeline_stages); - if (!tighten_bounds_status.ok()) { - return GenerateHelpfulError(std::move(tighten_bounds_status), - pipeline_stages, clock_period_ps, - worst_case_throughput); - } - return bounds; -} - -} // namespace xls diff --git a/xls/scheduling/asap_scheduler.h b/xls/scheduling/asap_scheduler.h deleted file mode 100644 index cf46b17304..0000000000 --- a/xls/scheduling/asap_scheduler.h +++ /dev/null @@ -1,87 +0,0 @@ -// Copyright 2026 The XLS Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef XLS_SCHEDULING_ASAP_SCHEDULER_H_ -#define XLS_SCHEDULING_ASAP_SCHEDULER_H_ - -#include -#include -#include -#include -#include - -#include "absl/status/status.h" -#include "absl/status/statusor.h" -#include "absl/types/span.h" -#include "xls/estimators/delay_model/delay_estimator.h" -#include "xls/ir/function_base.h" -#include "xls/scheduling/schedule_bounds.h" -#include "xls/scheduling/schedule_graph.h" -#include "xls/scheduling/scheduler.h" -#include "xls/scheduling/scheduling_options.h" - -namespace xls { - -class ASAPScheduler : public Scheduler { - public: - ASAPScheduler(const ScheduleGraph& graph, DelayEstimator& delay_estimator) - : ASAPScheduler("ASAPScheduler", graph, delay_estimator) {} - ~ASAPScheduler() override = default; - - absl::Status AddConstraints( - absl::Span constraints) override { - constraints_.insert(constraints_.end(), constraints.begin(), - constraints.end()); - return absl::OkStatus(); - } - - absl::StatusOr Schedule( - std::optional pipeline_stages, int64_t clock_period_ps, - SchedulingFailureBehavior failure_behavior, - std::optional worst_case_throughput = std::nullopt) override; - - const ScheduleGraph& graph() const { return graph_; } - DelayEstimator& delay_estimator() const { return delay_estimator_; } - absl::Span constraints() const { - return constraints_; - } - - protected: - ASAPScheduler(std::string name, const ScheduleGraph& graph, - DelayEstimator& delay_estimator) - : Scheduler(std::move(name)), - graph_(graph), - delay_estimator_(delay_estimator) {} - - absl::Status GenerateHelpfulError( - absl::Status&& orig_status, std::optional pipeline_stages, - int64_t clock_period_ps, std::optional worst_case_throughput); - - // Exposed to allow for Min-cut and random to be built on top of this. - absl::StatusOr ComputeBounds( - std::optional pipeline_stages, int64_t clock_period_ps, - std::optional worst_case_throughput); - - // Helper to tighten bounds using the ASAP/ALAP bounds. - static absl::Status TightenBounds(sched::ScheduleBounds& bounds, - FunctionBase* f, - std::optional schedule_length); - const ScheduleGraph& graph_; - DelayEstimator& delay_estimator_; - std::vector constraints_; -}; - -} // namespace xls - -#endif // XLS_SCHEDULING_ASAP_SCHEDULER_H_ diff --git a/xls/scheduling/asap_scheduler_test.cc b/xls/scheduling/asap_scheduler_test.cc deleted file mode 100644 index 25dcef015f..0000000000 --- a/xls/scheduling/asap_scheduler_test.cc +++ /dev/null @@ -1,135 +0,0 @@ -// Copyright 2026 The XLS Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "xls/scheduling/asap_scheduler.h" - -#include -#include - -#include "gtest/gtest.h" -#include "absl/status/statusor.h" -#include "xls/common/status/matchers.h" -#include "xls/estimators/delay_model/delay_estimator.h" -#include "xls/ir/bits.h" -#include "xls/ir/channel.h" -#include "xls/ir/channel_ops.h" -#include "xls/ir/function_builder.h" -#include "xls/ir/ir_test_base.h" -#include "xls/ir/value.h" -#include "xls/scheduling/schedule_graph.h" -#include "xls/scheduling/scheduling_options.h" - -namespace xls { -namespace { - -class ASAPSchedulerTest : public IrTestBase {}; - -TEST_F(ASAPSchedulerTest, SimpleFunction) { - auto p = CreatePackage(); - FunctionBuilder fb(TestName(), p.get()); - auto x = fb.Param("x", p->GetBitsType(32)); - auto y = fb.Param("y", p->GetBitsType(32)); - auto add = fb.Add(x, y); - XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.Build()); - - TestDelayEstimator delay_estimator; - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleGraph graph, - ScheduleGraph::Create(f, /*dead_after_synthesis=*/{})); - ASAPScheduler scheduler(graph, delay_estimator); - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleCycleMap cycle_map, - scheduler.Schedule(/*pipeline_stages=*/std::nullopt, - /*clock_period_ps=*/2, SchedulingFailureBehavior{})); - EXPECT_EQ(cycle_map.at(x.node()), 0); - EXPECT_EQ(cycle_map.at(y.node()), 0); - EXPECT_EQ(cycle_map.at(add.node()), 0); -} - -TEST_F(ASAPSchedulerTest, SimpleProc) { - auto p = CreatePackage(); - ProcBuilder pb(TestName(), p.get()); - pb.StateElement("x", Value(UBits(0, 32))); - pb.Next(pb.GetStateParam(0), - pb.Add(pb.GetStateParam(0), pb.Literal(UBits(1, 32)))); - XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build()); - - TestDelayEstimator delay_estimator; - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleGraph graph, - ScheduleGraph::Create(proc, /*dead_after_synthesis=*/{})); - ASAPScheduler scheduler(graph, delay_estimator); - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleCycleMap cycle_map, - scheduler.Schedule(/*pipeline_stages=*/std::nullopt, - /*clock_period_ps=*/2, SchedulingFailureBehavior{})); - EXPECT_EQ(cycle_map.at(pb.GetStateParam(0).node()), 0); -} - -TEST_F(ASAPSchedulerTest, WithConstraint) { - auto p = CreatePackage(); - FunctionBuilder fb(TestName(), p.get()); - auto x = fb.Param("x", p->GetBitsType(32)); - auto y = fb.Param("y", p->GetBitsType(32)); - auto add = fb.Add(x, y); - XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.Build()); - - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleGraph graph, - ScheduleGraph::Create(f, /*dead_after_synthesis=*/{})); - TestDelayEstimator delay_estimator; - ASAPScheduler scheduler(graph, delay_estimator); - XLS_ASSERT_OK( - scheduler.AddConstraints({NodeInCycleConstraint(add.node(), 2)})); - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleCycleMap cycle_map, - scheduler.Schedule(/*pipeline_stages=*/std::nullopt, - /*clock_period_ps=*/2, SchedulingFailureBehavior{})); - EXPECT_EQ(cycle_map.at(x.node()), 0); - EXPECT_EQ(cycle_map.at(y.node()), 0); - EXPECT_EQ(cycle_map.at(add.node()), 2); -} - -TEST_F(ASAPSchedulerTest, WithIOConstraint) { - auto p = CreatePackage(); - XLS_ASSERT_OK_AND_ASSIGN( - Channel * ch_a, p->CreateStreamingChannel("a", ChannelOps::kReceiveOnly, - p->GetBitsType(32))); - XLS_ASSERT_OK_AND_ASSIGN(Channel * ch_b, - p->CreateStreamingChannel("b", ChannelOps::kSendOnly, - p->GetBitsType(32))); - - ProcBuilder pb(TestName(), p.get()); - BValue tkn = pb.Literal(Value::Token()); - BValue rcv = pb.Receive(ch_a, tkn); - BValue send = pb.Send(ch_b, pb.TupleIndex(rcv, 0), pb.TupleIndex(rcv, 1)); - XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build()); - - TestDelayEstimator delay_estimator; - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleGraph graph, - ScheduleGraph::Create(proc, /*dead_after_synthesis=*/{})); - ASAPScheduler scheduler(graph, delay_estimator); - XLS_ASSERT_OK(scheduler.AddConstraints({IOConstraint( - "a", IODirection::kReceive, "b", IODirection::kSend, 1, 1)})); - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleCycleMap cycle_map, - scheduler.Schedule(/*pipeline_stages=*/std::nullopt, - /*clock_period_ps=*/2, SchedulingFailureBehavior{})); - EXPECT_EQ(cycle_map.at(rcv.node()), 0); - EXPECT_EQ(cycle_map.at(send.node()), 1); -} - -} // namespace -} // namespace xls diff --git a/xls/scheduling/min_cut_scheduler.cc b/xls/scheduling/min_cut_scheduler.cc index 6a44aba8ee..982f738e6f 100644 --- a/xls/scheduling/min_cut_scheduler.cc +++ b/xls/scheduling/min_cut_scheduler.cc @@ -17,7 +17,6 @@ #include #include #include -#include #include #include #include @@ -25,7 +24,6 @@ #include #include "absl/container/flat_hash_map.h" -#include "absl/container/flat_hash_set.h" #include "absl/log/log.h" #include "absl/status/status.h" #include "absl/status/statusor.h" @@ -41,8 +39,6 @@ #include "xls/ir/proc.h" #include "xls/scheduling/function_partition.h" #include "xls/scheduling/schedule_bounds.h" -#include "xls/scheduling/schedule_graph.h" -#include "xls/scheduling/schedule_util.h" #include "xls/scheduling/scheduling_options.h" namespace xls { @@ -122,7 +118,35 @@ std::vector MiddleFirstOrder(int64_t first, int64_t last) { return ret; } -absl::StatusOr RunMinCutScheduler( +} // namespace + +std::vector> GetMinCutCycleOrders(int64_t length) { + if (length == 0) { + return {{}}; + } + if (length == 1) { + return {{0}}; + } + if (length == 2) { + return {{0, 1}, {1, 0}}; + } + // For lengths greater than 2, return forward, reverse and middle first + // orderings. + std::vector> orders; + std::vector forward(length); + std::iota(forward.begin(), forward.end(), 0); + orders.push_back(forward); + + std::vector reverse(length); + std::iota(reverse.begin(), reverse.end(), 0); + std::reverse(reverse.begin(), reverse.end()); + orders.push_back(reverse); + + orders.push_back(MiddleFirstOrder(0, length - 1)); + return orders; +} + +absl::StatusOr MinCutScheduler( FunctionBase* f, int64_t pipeline_stages, int64_t clock_period_ps, const DelayEstimator& delay_estimator, sched::ScheduleBounds* bounds, absl::Span constraints) { @@ -204,47 +228,5 @@ absl::StatusOr RunMinCutScheduler( } return cycle_map; } -} // namespace - -std::vector> GetMinCutCycleOrders(int64_t length) { - if (length == 0) { - return {{}}; - } - if (length == 1) { - return {{0}}; - } - if (length == 2) { - return {{0, 1}, {1, 0}}; - } - // For lengths greater than 2, return forward, reverse and middle first - // orderings. - std::vector> orders; - std::vector forward(length); - std::iota(forward.begin(), forward.end(), 0); - orders.push_back(forward); - - std::vector reverse(length); - std::iota(reverse.begin(), reverse.end(), 0); - std::reverse(reverse.begin(), reverse.end()); - orders.push_back(reverse); - - orders.push_back(MiddleFirstOrder(0, length - 1)); - return orders; -} - -absl::StatusOr MinCutScheduler::Schedule( - std::optional pipeline_stages, int64_t clock_period_ps, - SchedulingFailureBehavior failure_behavior, - std::optional worst_case_throughput) { - XLS_ASSIGN_OR_RETURN( - sched::ScheduleBounds bounds, - ComputeBounds(pipeline_stages, clock_period_ps, worst_case_throughput)); - XLS_RET_CHECK(std::holds_alternative(graph().ir_scope())); - XLS_RET_CHECK(pipeline_stages.has_value()) - << "Min cut does not support unspecified pipeline stages."; - auto* f = std::get(graph().ir_scope()); - return RunMinCutScheduler(f, pipeline_stages.value(), clock_period_ps, - delay_estimator(), &bounds, constraints()); -} } // namespace xls diff --git a/xls/scheduling/min_cut_scheduler.h b/xls/scheduling/min_cut_scheduler.h index ed7f88eca4..ea9a26140e 100644 --- a/xls/scheduling/min_cut_scheduler.h +++ b/xls/scheduling/min_cut_scheduler.h @@ -16,21 +16,26 @@ #define XLS_SCHEDULING_MIN_CUT_SCHEDULER_H_ #include -#include -#include -#include #include #include "absl/status/statusor.h" +#include "absl/types/span.h" #include "xls/estimators/delay_model/delay_estimator.h" #include "xls/ir/function_base.h" -#include "xls/scheduling/asap_scheduler.h" #include "xls/scheduling/schedule_bounds.h" -#include "xls/scheduling/schedule_graph.h" #include "xls/scheduling/scheduling_options.h" namespace xls { +// Schedules the given function into a pipeline with the given clock +// period. Attempts to split nodes into stages such that the total number of +// flops in the pipeline stages is minimized without violating the target clock +// period. +absl::StatusOr MinCutScheduler( + FunctionBase* f, int64_t pipeline_stages, int64_t clock_period_ps, + const DelayEstimator& delay_estimator, sched::ScheduleBounds* bounds, + absl::Span constraints); + // Returns the list of ordering of cycles (pipeline stages) in which to compute // min cut of the graph. Each min cut of the graph computes which XLS node // values are in registers after a particular stage in the pipeline schedule. A @@ -41,21 +46,6 @@ namespace xls { // are tried. This function returns this set of orderings. Exposed for testing. std::vector> GetMinCutCycleOrders(int64_t length); -// Schedules the given function into a pipeline with the given clock -// period. Attempts to split nodes into stages such that the total number of -// flops in the pipeline stages is minimized without violating the target clock -// period. -class MinCutScheduler : public ASAPScheduler { - public: - MinCutScheduler(const ScheduleGraph& graph, DelayEstimator& delay_estimator) - : ASAPScheduler("MinCutScheduler", graph, delay_estimator) {} - ~MinCutScheduler() override = default; - absl::StatusOr Schedule( - std::optional pipeline_stages, int64_t clock_period_ps, - SchedulingFailureBehavior failure_behavior, - std::optional worst_case_throughput = std::nullopt) override; -}; - } // namespace xls #endif // XLS_SCHEDULING_MIN_CUT_SCHEDULER_H_ diff --git a/xls/scheduling/min_cut_scheduler_test.cc b/xls/scheduling/min_cut_scheduler_test.cc index 79d901ec95..19bd850378 100644 --- a/xls/scheduling/min_cut_scheduler_test.cc +++ b/xls/scheduling/min_cut_scheduler_test.cc @@ -19,24 +19,13 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -#include "absl/status/statusor.h" -#include "xls/common/status/matchers.h" -#include "xls/estimators/delay_model/delay_estimator.h" -#include "xls/ir/bits.h" -#include "xls/ir/function_builder.h" -#include "xls/ir/ir_test_base.h" -#include "xls/ir/value.h" -#include "xls/scheduling/schedule_graph.h" -#include "xls/scheduling/scheduling_options.h" namespace xls { namespace { using ::testing::ElementsAre; -class MinCutSchedulerTest : public IrTestBase {}; - -TEST_F(MinCutSchedulerTest, MinCutCycleOrders) { +TEST(MinCutSchedulerTest, MinCutCycleOrders) { EXPECT_THAT(GetMinCutCycleOrders(0), ElementsAre(std::vector())); EXPECT_THAT(GetMinCutCycleOrders(1), ElementsAre(std::vector({0}))); EXPECT_THAT( @@ -60,47 +49,5 @@ TEST_F(MinCutSchedulerTest, MinCutCycleOrders) { std::vector({3, 1, 0, 2, 5, 4, 6, 7}))); } -TEST_F(MinCutSchedulerTest, SimpleFunction) { - auto p = CreatePackage(); - FunctionBuilder fb(TestName(), p.get()); - auto x = fb.Param("x", p->GetBitsType(32)); - auto y = fb.Param("y", p->GetBitsType(32)); - auto add = fb.Add(x, y); - XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.Build()); - - TestDelayEstimator delay_estimator; - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleGraph graph, - ScheduleGraph::Create(f, /*dead_after_synthesis=*/{})); - MinCutScheduler scheduler(graph, delay_estimator); - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleCycleMap cycle_map, - scheduler.Schedule(/*pipeline_stages=*/1, - /*clock_period_ps=*/2, SchedulingFailureBehavior{})); - EXPECT_TRUE(cycle_map.contains(x.node())); - EXPECT_TRUE(cycle_map.contains(y.node())); - EXPECT_TRUE(cycle_map.contains(add.node())); -} - -TEST_F(MinCutSchedulerTest, SimpleProc) { - auto p = CreatePackage(); - ProcBuilder pb(TestName(), p.get()); - pb.StateElement("x", Value(UBits(0, 32))); - pb.Next(pb.GetStateParam(0), - pb.Add(pb.GetStateParam(0), pb.Literal(UBits(1, 32)))); - XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build()); - - TestDelayEstimator delay_estimator; - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleGraph graph, - ScheduleGraph::Create(proc, /*dead_after_synthesis=*/{})); - MinCutScheduler scheduler(graph, delay_estimator); - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleCycleMap cycle_map, - scheduler.Schedule(/*pipeline_stages=*/1, - /*clock_period_ps=*/2, SchedulingFailureBehavior{})); - EXPECT_TRUE(cycle_map.contains(pb.GetStateParam(0).node())); -} - } // namespace } // namespace xls diff --git a/xls/scheduling/pipeline_schedule.cc b/xls/scheduling/pipeline_schedule.cc index a51359356c..8f5a4e0af6 100644 --- a/xls/scheduling/pipeline_schedule.cc +++ b/xls/scheduling/pipeline_schedule.cc @@ -310,9 +310,6 @@ absl::Status PipelineSchedule::Verify() const { << cycle(read) << " + WCT " << worst_case_throughput << ")"; } } - } else { - VLOG(5) << "No worst-case throughput set for proc " << proc->name() - << ", skipping verification of Next nodes."; } } // Verify initial nodes in cycle 0. Final nodes in final cycle. diff --git a/xls/scheduling/pipeline_schedule_test.cc b/xls/scheduling/pipeline_schedule_test.cc index e8a0403c52..a0e80bf64d 100644 --- a/xls/scheduling/pipeline_schedule_test.cc +++ b/xls/scheduling/pipeline_schedule_test.cc @@ -109,70 +109,15 @@ using ::testing::Contains; using ::testing::Each; using ::testing::Gt; using ::testing::HasSubstr; -using ::testing::IsEmpty; using ::testing::IsSupersetOf; using ::testing::Key; using ::testing::Not; using ::testing::UnorderedElementsAre; using ::testing::UnorderedPointwise; -struct Strategies { - SchedulingStrategy primary; - SchedulingStrategy bounds; - template - friend void AbslStringify(Sink& sink, const Strategies& s) { - absl::Format(&sink, "primary_%v_bounds_%v", s.primary, s.bounds); - } -}; -class PipelineScheduleTestBase - : public IrTestBase, - public testing::WithParamInterface { - public: - PipelineScheduleTestBase() = default; - PipelineScheduleTestBase(PipelineScheduleTestBase&& other) = delete; - PipelineScheduleTestBase(const PipelineScheduleTestBase& other) = delete; - PipelineScheduleTestBase& operator=(PipelineScheduleTestBase&& other) = - delete; - PipelineScheduleTestBase& operator=(const PipelineScheduleTestBase& other) = - delete; - SchedulingOptions options() const { - return SchedulingOptions(GetParam().primary, GetParam().bounds); - } - bool is_sdc() const { return GetParam().primary == SchedulingStrategy::SDC; } - bool is_asap() const { - return GetParam().primary == SchedulingStrategy::ASAP; - } - bool is_random() const { - return GetParam().primary == SchedulingStrategy::RANDOM; - } - bool is_min_cut() const { - return GetParam().primary == SchedulingStrategy::MIN_CUT; - } -}; - -// ASAP, SDC, and Random primary, ASAP and SDC bounds in all combos -class PipelineScheduleTest : public PipelineScheduleTestBase {}; +class PipelineScheduleTest : public IrTestBase {}; -// Tests for error messages from scheduling. Currently does SDC ASAP in all -// combos since those 2 specifically have similar error formats. -// -// TODO(allight): Really the error messages should be created outside of the -// scheduler themselves so that we can have nice test error messages without -// having to depend on a particular implementation of scheduling. -class PipelineScheduleErrorTest : public PipelineScheduleTestBase {}; -// Error message tests where only SDC can give all the information we are -// looking for. Most of these are checking for throughput change suggestions -// that SDC can extract from the model but ASAP cannot. -class SdcOnlyPipelineScheduleErrorTest : public PipelineScheduleTestBase {}; -// Pure asap -class AsapPipelineScheduleTest : public PipelineScheduleTestBase {}; -// Pure SDC -class SdcPipelineScheduleTest : public PipelineScheduleTestBase {}; -class SdcPrimaryPipelineScheduleTest : public PipelineScheduleTestBase {}; -// Random scheduler only. -class RandomPipelineScheduleTest : public PipelineScheduleTestBase {}; - -TEST_P(PipelineScheduleTest, SelectsEntry) { +TEST_F(PipelineScheduleTest, SelectsEntry) { auto p = CreatePackage(); FunctionBuilder fb(TestName(), p.get()); fb.Param("x", p->GetBitsType(32)); @@ -197,7 +142,7 @@ TEST_P(PipelineScheduleTest, SelectsEntry) { EXPECT_THAT(schedule.nodes_in_cycle(0), UnorderedElementsAre(m::Literal())); } -TEST_P(AsapPipelineScheduleTest, AsapScheduleTrivial) { +TEST_F(PipelineScheduleTest, AsapScheduleTrivial) { auto p = CreatePackage(); FunctionBuilder fb(TestName(), p.get()); fb.Param("x", p->GetBitsType(32)); @@ -213,7 +158,7 @@ TEST_P(AsapPipelineScheduleTest, AsapScheduleTrivial) { EXPECT_THAT(schedule.nodes_in_cycle(0), UnorderedElementsAre(m::Param())); } -TEST_P(PipelineScheduleErrorTest, OutrightInfeasibleSchedule) { +TEST_F(PipelineScheduleTest, OutrightInfeasibleSchedule) { // Create a schedule in which the critical path doesn't even fit in the // requested clock_period * stages. auto p = CreatePackage(); @@ -222,15 +167,17 @@ TEST_P(PipelineScheduleErrorTest, OutrightInfeasibleSchedule) { fb.Not(fb.Not(fb.Not(fb.Not(x)))); XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.Build()); - ASSERT_THAT( - RunPipelineSchedule(f, TestDelayEstimator(), - options().clock_period_ps(1).pipeline_stages(2)) - .status(), - StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("--pipeline_stages=4"))); + ASSERT_THAT(RunPipelineSchedule(f, TestDelayEstimator(), + SchedulingOptions(SchedulingStrategy::MIN_CUT) + .clock_period_ps(1) + .pipeline_stages(2)) + .status(), + StatusIs(absl::StatusCode::kResourceExhausted, + HasSubstr("cannot be scheduled in 2 stages. Computed " + "minimum stage count is 4."))); } -TEST_P(PipelineScheduleErrorTest, InfeasibleScheduleWithBinPacking) { +TEST_F(PipelineScheduleTest, InfeasibleScheduleWithBinPacking) { // Create a schedule in which the critical path fits in the requested // clock_period * stages, but there is no way to bin pack the instructions // into the stages such that the schedule is met. @@ -240,15 +187,17 @@ TEST_P(PipelineScheduleErrorTest, InfeasibleScheduleWithBinPacking) { fb.Not(fb.UDiv(fb.Not(x), fb.Not(x))); XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.Build()); - ASSERT_THAT( - RunPipelineSchedule(f, TestDelayEstimator(), - options().clock_period_ps(2).pipeline_stages(2)) - .status(), - StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("--pipeline_stages=3"))); + ASSERT_THAT(RunPipelineSchedule(f, TestDelayEstimator(), + SchedulingOptions(SchedulingStrategy::MIN_CUT) + .clock_period_ps(2) + .pipeline_stages(2)) + .status(), + StatusIs(absl::StatusCode::kResourceExhausted, + HasSubstr("cannot be scheduled in 2 stages. Computed " + "minimum stage count is 3."))); } -TEST_P(PipelineScheduleErrorTest, InfeasibleScheduleWithReturnValueUsers) { +TEST_F(PipelineScheduleTest, InfeasibleScheduleWithReturnValueUsers) { // Create function which has users of the return value node such that the // return value cannot be scheduled in the final cycle. auto p = CreatePackage(); @@ -260,22 +209,25 @@ TEST_P(PipelineScheduleErrorTest, InfeasibleScheduleWithReturnValueUsers) { ASSERT_THAT( RunPipelineSchedule(f, TestDelayEstimator(), - options().clock_period_ps(1).pipeline_stages(2)) + SchedulingOptions(SchedulingStrategy::MIN_CUT) + .clock_period_ps(1) + .pipeline_stages(2)) .status(), - StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("cannot achieve the specified clock period. Try " - "`--clock_period_ps=2`"))); + StatusIs(absl::StatusCode::kResourceExhausted, + HasSubstr("cannot be scheduled in any number of stages"))); } -TEST_P(AsapPipelineScheduleTest, AsapScheduleNoParameters) { +TEST_F(PipelineScheduleTest, AsapScheduleNoParameters) { auto p = CreatePackage(); FunctionBuilder fb(TestName(), p.get()); fb.Negate(fb.Add(fb.Literal(UBits(42, 8)), fb.Literal(UBits(100, 8)))); XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.Build()); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(f, TestDelayEstimator(), - options().clock_period_ps(1))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule( + f, TestDelayEstimator(), + SchedulingOptions(SchedulingStrategy::ASAP).clock_period_ps(1))); EXPECT_EQ(schedule.length(), 2); EXPECT_THAT(schedule.nodes_in_cycle(0), @@ -283,7 +235,7 @@ TEST_P(AsapPipelineScheduleTest, AsapScheduleNoParameters) { EXPECT_THAT(schedule.nodes_in_cycle(1), UnorderedElementsAre(m::Neg())); } -TEST_P(AsapPipelineScheduleTest, AsapScheduleIncrementChain) { +TEST_F(PipelineScheduleTest, AsapScheduleIncrementChain) { auto p = CreatePackage(); FunctionBuilder fb(TestName(), p.get()); auto x = fb.Param("x", p->GetBitsType(32)); @@ -291,9 +243,11 @@ TEST_P(AsapPipelineScheduleTest, AsapScheduleIncrementChain) { fb.Literal(UBits(1, 32))); XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.Build()); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(f, TestDelayEstimator(), - options().clock_period_ps(1))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule( + f, TestDelayEstimator(), + SchedulingOptions(SchedulingStrategy::ASAP).clock_period_ps(1))); EXPECT_EQ(schedule.length(), 3); EXPECT_THAT(schedule.nodes_in_cycle(0), @@ -303,7 +257,7 @@ TEST_P(AsapPipelineScheduleTest, AsapScheduleIncrementChain) { EXPECT_THAT(schedule.nodes_in_cycle(2), UnorderedElementsAre(m::Add())); } -TEST_P(SdcPrimaryPipelineScheduleTest, MinimizeRegisterBitslices) { +TEST_F(PipelineScheduleTest, MinimizeRegisterBitslices) { // When minimizing registers, bit-slices should be hoisted in the schedule if // their operand is not otherwise live, and sunk in the schedule if their // operand *is* otherwise live. @@ -319,9 +273,10 @@ TEST_P(SdcPrimaryPipelineScheduleTest, MinimizeRegisterBitslices) { XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.Build()); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(f, TestDelayEstimator(), - options().clock_period_ps(1))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(f, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(1))); EXPECT_EQ(schedule.length(), 2); EXPECT_THAT(schedule.nodes_in_cycle(0), @@ -332,7 +287,7 @@ TEST_P(SdcPrimaryPipelineScheduleTest, MinimizeRegisterBitslices) { UnorderedElementsAre(m::BitSlice(m::Param("x")), m::Neg(), m::Concat())); } -TEST_P(AsapPipelineScheduleTest, AsapScheduleComplex) { +TEST_F(PipelineScheduleTest, AsapScheduleComplex) { auto p = CreatePackage(); FunctionBuilder fb(TestName(), p.get()); Type* u32 = p->GetBitsType(32); @@ -343,9 +298,11 @@ TEST_P(AsapPipelineScheduleTest, AsapScheduleComplex) { XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.Build()); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(f, TestDelayEstimator(), - options().clock_period_ps(2))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule( + f, TestDelayEstimator(), + SchedulingOptions(SchedulingStrategy::ASAP).clock_period_ps(2))); EXPECT_EQ(schedule.length(), 3); EXPECT_THAT(schedule.nodes_in_cycle(0), @@ -356,7 +313,7 @@ TEST_P(AsapPipelineScheduleTest, AsapScheduleComplex) { EXPECT_THAT(schedule.nodes_in_cycle(2), UnorderedElementsAre(m::Neg())); } -TEST_P(PipelineScheduleTest, JustClockPeriodGiven) { +TEST_F(PipelineScheduleTest, JustClockPeriodGiven) { auto p = CreatePackage(); FunctionBuilder fb(TestName(), p.get()); Type* u32 = p->GetBitsType(32); @@ -367,9 +324,10 @@ TEST_P(PipelineScheduleTest, JustClockPeriodGiven) { XLS_ASSERT_OK_AND_ASSIGN(Function * func, fb.Build()); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(func, TestDelayEstimator(), - options().clock_period_ps(2))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(func, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(2))); // Returns the unique scheduled Ops in the given cycle. auto scheduled_ops = [&](int64_t cycle) { @@ -381,26 +339,15 @@ TEST_P(PipelineScheduleTest, JustClockPeriodGiven) { }; EXPECT_EQ(schedule.length(), 3); - if (is_sdc()) { - EXPECT_THAT(scheduled_ops(0), - UnorderedElementsAre(Op::kParam, Op::kOr, Op::kNot)); - EXPECT_THAT(scheduled_ops(1), UnorderedElementsAre(Op::kAdd, Op::kConcat, - Op::kUMul, Op::kSub)); - EXPECT_THAT(scheduled_ops(2), UnorderedElementsAre(Op::kNeg)); - EXPECT_THAT(scheduled_ops(3), UnorderedElementsAre()); - } else if (is_asap()) { - EXPECT_THAT(scheduled_ops(0), - UnorderedElementsAre(Op::kParam, Op::kOr, Op::kNot, Op::kAdd)); - EXPECT_THAT(scheduled_ops(1), - UnorderedElementsAre(Op::kConcat, Op::kUMul, Op::kSub)); - EXPECT_THAT(scheduled_ops(2), UnorderedElementsAre(Op::kNeg)); - EXPECT_THAT(scheduled_ops(3), IsEmpty()); - } else { - EXPECT_TRUE(is_random()); - } + EXPECT_THAT(scheduled_ops(0), + UnorderedElementsAre(Op::kParam, Op::kOr, Op::kNot)); + EXPECT_THAT(scheduled_ops(1), + UnorderedElementsAre(Op::kAdd, Op::kConcat, Op::kUMul, Op::kSub)); + EXPECT_THAT(scheduled_ops(2), UnorderedElementsAre(Op::kNeg)); + EXPECT_THAT(scheduled_ops(3), UnorderedElementsAre()); } -TEST_P(PipelineScheduleTest, TestVerifyTiming) { +TEST_F(PipelineScheduleTest, TestVerifyTiming) { auto p = CreatePackage(); FunctionBuilder fb(TestName(), p.get()); Type* u32 = p->GetBitsType(32); @@ -411,9 +358,10 @@ TEST_P(PipelineScheduleTest, TestVerifyTiming) { XLS_ASSERT_OK_AND_ASSIGN(Function * func, fb.Build()); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(func, TestDelayEstimator(), - options().clock_period_ps(5))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(func, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(5))); EXPECT_EQ(schedule.length(), 1); XLS_EXPECT_OK( @@ -438,7 +386,7 @@ TEST_P(PipelineScheduleTest, TestVerifyTiming) { "(3ps): add.3 (1ps) -> neg.4 (1ps) -> sub.5 (1ps)"))); } -TEST_P(PipelineScheduleTest, ClockPeriodAndPipelineLengthGiven) { +TEST_F(PipelineScheduleTest, ClockPeriodAndPipelineLengthGiven) { auto p = CreatePackage(); FunctionBuilder fb(TestName(), p.get()); Type* u32 = p->GetBitsType(32); @@ -451,8 +399,9 @@ TEST_P(PipelineScheduleTest, ClockPeriodAndPipelineLengthGiven) { XLS_ASSERT_OK_AND_ASSIGN( PipelineSchedule schedule, - RunPipelineSchedule(func, TestDelayEstimator(), - options().clock_period_ps(2).pipeline_stages(4))); + RunPipelineSchedule( + func, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(2).pipeline_stages(4))); // Returns the unique scheduled Ops in the given cycle. auto scheduled_ops = [&](int64_t cycle) { @@ -464,27 +413,15 @@ TEST_P(PipelineScheduleTest, ClockPeriodAndPipelineLengthGiven) { }; EXPECT_EQ(schedule.length(), 4); - if (is_sdc()) { - EXPECT_THAT(scheduled_ops(0), - UnorderedElementsAre(Op::kParam, Op::kOr, Op::kNeg)); - EXPECT_THAT(scheduled_ops(1), - UnorderedElementsAre(Op::kAdd, Op::kNot, Op::kSub)); - EXPECT_THAT(scheduled_ops(2), UnorderedElementsAre(Op::kConcat, Op::kUMul)); - EXPECT_THAT(scheduled_ops(3), UnorderedElementsAre(Op::kNeg)); - } else if (is_asap()) { - // Since ASAP doesn't minimize regs the assignments are slightly different. - EXPECT_THAT(scheduled_ops(0), - UnorderedElementsAre(Op::kParam, Op::kOr, Op::kNeg, Op::kAdd)); - EXPECT_THAT(scheduled_ops(1), UnorderedElementsAre(Op::kNot, Op::kSub)); - EXPECT_THAT(scheduled_ops(2), - UnorderedElementsAre(Op::kConcat, Op::kUMul, Op::kNeg)); - EXPECT_THAT(scheduled_ops(3), IsEmpty()); - } else { - EXPECT_TRUE(is_random()); - } + EXPECT_THAT(scheduled_ops(0), + UnorderedElementsAre(Op::kParam, Op::kOr, Op::kNeg)); + EXPECT_THAT(scheduled_ops(1), + UnorderedElementsAre(Op::kAdd, Op::kNot, Op::kSub)); + EXPECT_THAT(scheduled_ops(2), UnorderedElementsAre(Op::kConcat, Op::kUMul)); + EXPECT_THAT(scheduled_ops(3), UnorderedElementsAre(Op::kNeg)); } -TEST_P(PipelineScheduleTest, JustPipelineLengthGiven) { +TEST_F(PipelineScheduleTest, JustPipelineLengthGiven) { auto p = CreatePackage(); FunctionBuilder fb(TestName(), p.get()); Type* u32 = p->GetBitsType(32); @@ -495,9 +432,10 @@ TEST_P(PipelineScheduleTest, JustPipelineLengthGiven) { XLS_ASSERT_OK_AND_ASSIGN(Function * func, fb.Build()); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(func, TestDelayEstimator(), - options().pipeline_stages(6))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(func, TestDelayEstimator(), + SchedulingOptions().pipeline_stages(6))); // Returns the unique scheduled Ops in the given cycle. auto scheduled_ops = [&](int64_t cycle) { @@ -513,28 +451,15 @@ TEST_P(PipelineScheduleTest, JustPipelineLengthGiven) { // The maximum delay of any stage should be minimum feasible value. In this // case it is 1ps, which means there should be no dependent instructions in // single cycle. - if (is_random()) { - return; - } - EXPECT_TRUE(is_sdc() || is_asap()); - if (is_sdc()) { - EXPECT_THAT(scheduled_ops(0), UnorderedElementsAre(Op::kParam, Op::kOr)); - } else { - EXPECT_THAT(scheduled_ops(0), - UnorderedElementsAre(Op::kParam, Op::kOr, Op::kAdd)); - } + EXPECT_THAT(scheduled_ops(0), UnorderedElementsAre(Op::kParam, Op::kOr)); EXPECT_THAT(scheduled_ops(1), UnorderedElementsAre(Op::kNeg)); EXPECT_THAT(scheduled_ops(2), UnorderedElementsAre(Op::kNot)); - if (is_sdc()) { - EXPECT_THAT(scheduled_ops(3), UnorderedElementsAre(Op::kAdd, Op::kSub)); - } else { - EXPECT_THAT(scheduled_ops(3), UnorderedElementsAre(Op::kSub)); - } + EXPECT_THAT(scheduled_ops(3), UnorderedElementsAre(Op::kAdd, Op::kSub)); EXPECT_THAT(scheduled_ops(4), UnorderedElementsAre(Op::kConcat, Op::kUMul)); EXPECT_THAT(scheduled_ops(5), UnorderedElementsAre(Op::kNeg)); } -TEST_P(PipelineScheduleTest, LongPipelineLength) { +TEST_F(PipelineScheduleTest, LongPipelineLength) { // Generate an absurdly long pipeline schedule. Most stages are empty, but it // should not crash. auto p = CreatePackage(); @@ -546,39 +471,25 @@ TEST_P(PipelineScheduleTest, LongPipelineLength) { XLS_ASSERT_OK_AND_ASSIGN(Function * func, fb.Build()); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(func, TestDelayEstimator(), - options().pipeline_stages(100))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(func, TestDelayEstimator(), + SchedulingOptions().pipeline_stages(100))); EXPECT_EQ(schedule.length(), 100); // Most stages should be empty. - if (is_random()) { - return; - } - EXPECT_TRUE(is_sdc() || is_asap()); - if (is_sdc()) { - EXPECT_THAT(schedule.nodes_in_cycle(0), - UnorderedElementsAre(x.node(), bitslice.node())); - } else { - EXPECT_THAT( - schedule.nodes_in_cycle(0), - UnorderedElementsAre(x.node(), bitslice.node(), zero_ext.node())); - } + EXPECT_THAT(schedule.nodes_in_cycle(0), + UnorderedElementsAre(x.node(), bitslice.node())); // The bitslice is the narrowest among the chain of operations so it should - // precede the long chain of empty stages if we are SDC. It can fit in the - // first stage so ASAP will place it there. + // precede the long chain of empty stages. for (int64_t i = 1; i < 99; ++i) { EXPECT_THAT(schedule.nodes_in_cycle(i), UnorderedElementsAre()); } - if (is_sdc()) { - EXPECT_THAT(schedule.nodes_in_cycle(99), - UnorderedElementsAre(zero_ext.node())); - } else { - EXPECT_THAT(schedule.nodes_in_cycle(99), IsEmpty()); - } + EXPECT_THAT(schedule.nodes_in_cycle(99), + UnorderedElementsAre(zero_ext.node())); } -TEST_P(PipelineScheduleTest, ClockPeriodMargin) { +TEST_F(PipelineScheduleTest, ClockPeriodMargin) { auto p = CreatePackage(); FunctionBuilder fb(TestName(), p.get()); Type* u32 = p->GetBitsType(32); @@ -588,9 +499,10 @@ TEST_P(PipelineScheduleTest, ClockPeriodMargin) { XLS_ASSERT_OK_AND_ASSIGN(Function * func, fb.Build()); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(func, TestDelayEstimator(), - options().clock_period_ps(3))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(func, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(3))); EXPECT_EQ(schedule.length(), 2); { @@ -598,7 +510,7 @@ TEST_P(PipelineScheduleTest, ClockPeriodMargin) { PipelineSchedule schedule, RunPipelineSchedule( func, TestDelayEstimator(), - options().clock_period_ps(3).clock_margin_percent(0))); + SchedulingOptions().clock_period_ps(3).clock_margin_percent(0))); EXPECT_EQ(schedule.length(), 2); } { @@ -606,7 +518,7 @@ TEST_P(PipelineScheduleTest, ClockPeriodMargin) { PipelineSchedule schedule, RunPipelineSchedule( func, TestDelayEstimator(), - options().clock_period_ps(3).clock_margin_percent(33))); + SchedulingOptions().clock_period_ps(3).clock_margin_percent(33))); EXPECT_EQ(schedule.length(), 3); } { @@ -614,13 +526,13 @@ TEST_P(PipelineScheduleTest, ClockPeriodMargin) { PipelineSchedule schedule, RunPipelineSchedule( func, TestDelayEstimator(), - options().clock_period_ps(3).clock_margin_percent(66))); + SchedulingOptions().clock_period_ps(3).clock_margin_percent(66))); EXPECT_EQ(schedule.length(), 6); } EXPECT_THAT( RunPipelineSchedule( func, TestDelayEstimator(), - options().clock_period_ps(3).clock_margin_percent(200)) + SchedulingOptions().clock_period_ps(3).clock_margin_percent(200)) .status(), absl_testing::StatusIs( absl::StatusCode::kInvalidArgument, @@ -629,11 +541,7 @@ TEST_P(PipelineScheduleTest, ClockPeriodMargin) { "Original clock period: 3ps, clock margin: 200%"))); } -TEST_P(PipelineScheduleTest, PeriodRelaxation) { - if (is_random()) { - GTEST_SKIP() << "Relaxation not guarneteed to improve schedule quality " - "with random scheduler."; - } +TEST_F(PipelineScheduleTest, PeriodRelaxation) { auto p = CreatePackage(); FunctionBuilder fb(TestName(), p.get()); Type* u32 = p->GetBitsType(32); @@ -656,9 +564,10 @@ TEST_P(PipelineScheduleTest, PeriodRelaxation) { XLS_ASSERT_OK_AND_ASSIGN(Function * func, fb.Build()); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(func, TestDelayEstimator(), - options().pipeline_stages(2))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(func, TestDelayEstimator(), + SchedulingOptions().pipeline_stages(2))); EXPECT_EQ(schedule.length(), 2); int64_t reg_count_default = schedule.CountFinalInteriorPipelineRegisters(); @@ -667,7 +576,7 @@ TEST_P(PipelineScheduleTest, PeriodRelaxation) { PipelineSchedule schedule, RunPipelineSchedule( func, TestDelayEstimator(), - options().pipeline_stages(2).period_relaxation_percent( + SchedulingOptions().pipeline_stages(2).period_relaxation_percent( relax_percent))); EXPECT_EQ(schedule.length(), 2); int64_t reg_count_relaxed = schedule.CountFinalInteriorPipelineRegisters(); @@ -675,7 +584,7 @@ TEST_P(PipelineScheduleTest, PeriodRelaxation) { } } -TEST_P(PipelineScheduleTest, SerializeAndDeserialize) { +TEST_F(PipelineScheduleTest, SerializeAndDeserialize) { auto p = CreatePackage(); FunctionBuilder fb(TestName(), p.get()); Type* u32 = p->GetBitsType(32); @@ -684,9 +593,10 @@ TEST_P(PipelineScheduleTest, SerializeAndDeserialize) { auto z = fb.Param("z", u32); fb.Negate(fb.Concat({(fb.Not(fb.Negate(x | y)) - z) * x, z + z})); XLS_ASSERT_OK_AND_ASSIGN(Function * func, fb.Build()); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(func, TestDelayEstimator(), - options().pipeline_stages(3))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(func, TestDelayEstimator(), + SchedulingOptions().pipeline_stages(3))); ASSERT_TRUE(schedule.min_clock_period_ps().has_value()); XLS_ASSERT_OK_AND_ASSIGN(PipelineScheduleProto proto, @@ -705,7 +615,7 @@ TEST_P(PipelineScheduleTest, SerializeAndDeserialize) { EXPECT_EQ(clone.length(), schedule.length()); } -TEST_P(PipelineScheduleTest, NodeDelayInScheduleProto) { +TEST_F(PipelineScheduleTest, NodeDelayInScheduleProto) { // Tests that node and path delays are serialized in the schedule proto // using trivial pipeline: 3 stages of 2 x 1-bit inverters. auto p = CreatePackage(); @@ -713,9 +623,10 @@ TEST_P(PipelineScheduleTest, NodeDelayInScheduleProto) { auto x = fb.Param("x", p->GetBitsType(1)); fb.Not(fb.Not(fb.Not(fb.Not(fb.Not(fb.Not(x)))))); XLS_ASSERT_OK_AND_ASSIGN(Function * func, fb.Build()); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(func, TestDelayEstimator(), - options().pipeline_stages(3))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(func, TestDelayEstimator(), + SchedulingOptions().pipeline_stages(3))); XLS_ASSERT_OK_AND_ASSIGN(PipelineScheduleProto proto, schedule.ToProto(TestDelayEstimator())); @@ -728,7 +639,7 @@ TEST_P(PipelineScheduleTest, NodeDelayInScheduleProto) { } } -TEST_P(PipelineScheduleTest, ProcSchedule) { +TEST_F(PipelineScheduleTest, ProcSchedule) { Package p("p"); Type* u16 = p.GetBitsType(16); XLS_ASSERT_OK_AND_ASSIGN( @@ -744,9 +655,10 @@ TEST_P(PipelineScheduleTest, ProcSchedule) { BValue send = pb.Send(out_ch, out); XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build({st})); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(proc, TestDelayEstimator(), - options().clock_period_ps(1))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(proc, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(1))); EXPECT_EQ(schedule.length(), 3); @@ -754,7 +666,7 @@ TEST_P(PipelineScheduleTest, ProcSchedule) { EXPECT_EQ(schedule.cycle(send.node()), 2); } -TEST_P(PipelineScheduleTest, StatelessProcSchedule) { +TEST_F(PipelineScheduleTest, StatelessProcSchedule) { Package p("p"); Type* u16 = p.GetBitsType(16); XLS_ASSERT_OK_AND_ASSIGN( @@ -769,9 +681,10 @@ TEST_P(PipelineScheduleTest, StatelessProcSchedule) { BValue send = pb.Send(out_ch, out); XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build({})); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(proc, TestDelayEstimator(), - options().clock_period_ps(1))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(proc, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(1))); EXPECT_EQ(schedule.length(), 3); @@ -779,7 +692,7 @@ TEST_P(PipelineScheduleTest, StatelessProcSchedule) { EXPECT_EQ(schedule.cycle(send.node()), 2); } -TEST_P(PipelineScheduleTest, MultistateProcSchedule) { +TEST_F(PipelineScheduleTest, MultistateProcSchedule) { Package p("p"); Type* u16 = p.GetBitsType(16); XLS_ASSERT_OK_AND_ASSIGN( @@ -797,22 +710,20 @@ TEST_P(PipelineScheduleTest, MultistateProcSchedule) { XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build({pb.Add(st0, rcv), pb.Subtract(st1, rcv)})); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(proc, TestDelayEstimator(), - options().clock_period_ps(1))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(proc, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(1))); EXPECT_EQ(schedule.length(), 3); - if (is_random()) { - return; - } EXPECT_EQ(schedule.cycle(st0.node()), 0); EXPECT_EQ(schedule.cycle(st1.node()), 0); EXPECT_EQ(schedule.cycle(rcv.node()), 0); EXPECT_EQ(schedule.cycle(send.node()), 2); } -TEST_P(PipelineScheduleTest, ProcWithConditionalReceive) { +TEST_F(PipelineScheduleTest, ProcWithConditionalReceive) { // Test a proc with a conditional receive. Package p("p"); Type* u16 = p.GetBitsType(16); @@ -830,22 +741,20 @@ TEST_P(PipelineScheduleTest, ProcWithConditionalReceive) { BValue send = pb.Send(out_ch, out); XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build({st})); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(proc, TestDelayEstimator(), - options().clock_period_ps(1))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(proc, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(1))); EXPECT_EQ(schedule.length(), 3); EXPECT_EQ(schedule.cycle(rcv.node()), 0); - // Literals are "untimed" and not in the schedule for SDC. - // TODO(allight): We might want to match this with ASAP and remove this check. - if (is_sdc()) { - EXPECT_FALSE(schedule.IsScheduled(cond.node())); - } + // Literals are "untimed" and not in the schedule. + EXPECT_FALSE(schedule.IsScheduled(cond.node())); EXPECT_EQ(schedule.cycle(send.node()), 2); } -TEST_P(PipelineScheduleTest, ProcWithConditionalReceiveLongCondition) { +TEST_F(PipelineScheduleTest, ProcWithConditionalReceiveLongCondition) { // Test a proc with a conditional receive. The receive condition takes too // long to compute in the same cycle as the receive so the receive is pushed // to stage 1. @@ -864,19 +773,18 @@ TEST_P(PipelineScheduleTest, ProcWithConditionalReceiveLongCondition) { BValue out = pb.Negate(pb.Not(pb.Negate(rcv))); pb.Send(out_ch, out); XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build({st})); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(proc, TestDelayEstimator(), - options().clock_period_ps(1))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(proc, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(1))); EXPECT_EQ(schedule.length(), 5); EXPECT_EQ(schedule.cycle(cond.node()), 1); - // NB ASAP location is cycle 1 but since the condition is 1 bit and the recv'd - // value is 16 SDC pushes it back one cycle to save register bits. - EXPECT_GE(schedule.cycle(rcv.node()), 1); + EXPECT_EQ(schedule.cycle(rcv.node()), 2); } -TEST_P(PipelineScheduleTest, ReceiveFollowedBySend) { +TEST_F(PipelineScheduleTest, ReceiveFollowedBySend) { Package package = Package(TestName()); Type* u32 = package.GetBitsType(32); @@ -897,14 +805,15 @@ TEST_P(PipelineScheduleTest, ReceiveFollowedBySend) { XLS_ASSERT_OK_AND_ASSIGN(const DelayEstimator* delay_estimator, GetDelayEstimator("unit")); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(proc, *delay_estimator, - options().pipeline_stages(5))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(proc, *delay_estimator, + SchedulingOptions().pipeline_stages(5))); EXPECT_EQ(schedule.length(), 5); EXPECT_GE(schedule.cycle(send.node()), schedule.cycle(rcv.node())); } -TEST_P(PipelineScheduleErrorTest, SendFollowedByReceiveCannotBeInSameCycle) { +TEST_F(PipelineScheduleTest, SendFollowedByReceiveCannotBeInSameCycle) { Package package = Package(TestName()); Type* u32 = package.GetBitsType(32); @@ -924,20 +833,20 @@ TEST_P(PipelineScheduleErrorTest, SendFollowedByReceiveCannotBeInSameCycle) { XLS_ASSERT_OK_AND_ASSIGN(const DelayEstimator* delay_estimator, GetDelayEstimator("unit")); - ASSERT_THAT( - RunPipelineSchedule(proc, *delay_estimator, options().pipeline_stages(1)), - StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("--pipeline_stages=2"))); + ASSERT_THAT(RunPipelineSchedule(proc, *delay_estimator, + SchedulingOptions().pipeline_stages(1)), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("--pipeline_stages=2"))); XLS_ASSERT_OK_AND_ASSIGN( PipelineSchedule schedule, RunPipelineSchedule(proc, *delay_estimator, - options().clock_period_ps(10'000))); + SchedulingOptions().clock_period_ps(10'000))); EXPECT_EQ(schedule.length(), 2); EXPECT_EQ(schedule.cycle(send.node()), 0); EXPECT_EQ(schedule.cycle(rcv.node()), 1); } -TEST_P(PipelineScheduleErrorTest, SendFollowedByReceiveIfCannotBeInSameCycle) { +TEST_F(PipelineScheduleTest, SendFollowedByReceiveIfCannotBeInSameCycle) { Package package = Package(TestName()); Type* u32 = package.GetBitsType(32); @@ -957,20 +866,20 @@ TEST_P(PipelineScheduleErrorTest, SendFollowedByReceiveIfCannotBeInSameCycle) { XLS_ASSERT_OK_AND_ASSIGN(const DelayEstimator* delay_estimator, GetDelayEstimator("unit")); - ASSERT_THAT( - RunPipelineSchedule(proc, *delay_estimator, options().pipeline_stages(1)), - StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("--pipeline_stages=2"))); + ASSERT_THAT(RunPipelineSchedule(proc, *delay_estimator, + SchedulingOptions().pipeline_stages(1)), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("--pipeline_stages=2"))); XLS_ASSERT_OK_AND_ASSIGN( PipelineSchedule schedule, RunPipelineSchedule(proc, *delay_estimator, - options().clock_period_ps(10'000))); + SchedulingOptions().clock_period_ps(10'000))); EXPECT_EQ(schedule.length(), 2); EXPECT_EQ(schedule.cycle(send.node()), 0); EXPECT_EQ(schedule.cycle(rcv_if.node()), 1); } -TEST_P(PipelineScheduleErrorTest, +TEST_F(PipelineScheduleTest, SendFollowedByNonblockingReceiveCannotBeInSameCycle) { Package package = Package(TestName()); @@ -991,20 +900,20 @@ TEST_P(PipelineScheduleErrorTest, XLS_ASSERT_OK_AND_ASSIGN(const DelayEstimator* delay_estimator, GetDelayEstimator("unit")); - ASSERT_THAT( - RunPipelineSchedule(proc, *delay_estimator, options().pipeline_stages(1)), - StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("--pipeline_stages=2"))); + ASSERT_THAT(RunPipelineSchedule(proc, *delay_estimator, + SchedulingOptions().pipeline_stages(1)), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("--pipeline_stages=2"))); XLS_ASSERT_OK_AND_ASSIGN( PipelineSchedule schedule, RunPipelineSchedule(proc, *delay_estimator, - options().clock_period_ps(10'000))); + SchedulingOptions().clock_period_ps(10'000))); EXPECT_EQ(schedule.length(), 2); EXPECT_EQ(schedule.cycle(send.node()), 0); EXPECT_EQ(schedule.cycle(rcv.node()), 1); } -TEST_P(PipelineScheduleErrorTest, +TEST_F(PipelineScheduleTest, SendFollowedByNonblockingReceiveIfCannotBeInSameCycle) { Package package = Package(TestName()); @@ -1026,20 +935,20 @@ TEST_P(PipelineScheduleErrorTest, XLS_ASSERT_OK_AND_ASSIGN(const DelayEstimator* delay_estimator, GetDelayEstimator("unit")); - ASSERT_THAT( - RunPipelineSchedule(proc, *delay_estimator, options().pipeline_stages(1)), - StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("--pipeline_stages=2"))); + ASSERT_THAT(RunPipelineSchedule(proc, *delay_estimator, + SchedulingOptions().pipeline_stages(1)), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("--pipeline_stages=2"))); XLS_ASSERT_OK_AND_ASSIGN( PipelineSchedule schedule, RunPipelineSchedule(proc, *delay_estimator, - options().clock_period_ps(10'000))); + SchedulingOptions().clock_period_ps(10'000))); EXPECT_EQ(schedule.length(), 2); EXPECT_EQ(schedule.cycle(send.node()), 0); EXPECT_EQ(schedule.cycle(rcv_if.node()), 1); } -TEST_P(PipelineScheduleErrorTest, +TEST_F(PipelineScheduleTest, SendFollowedIndirectlyByReceiveCannotBeInSameCycle) { Package package = Package(TestName()); @@ -1063,20 +972,20 @@ TEST_P(PipelineScheduleErrorTest, XLS_ASSERT_OK_AND_ASSIGN(const DelayEstimator* delay_estimator, GetDelayEstimator("unit")); - ASSERT_THAT( - RunPipelineSchedule(proc, *delay_estimator, options().pipeline_stages(1)), - StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("--pipeline_stages=2"))); + ASSERT_THAT(RunPipelineSchedule(proc, *delay_estimator, + SchedulingOptions().pipeline_stages(1)), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("--pipeline_stages=2"))); XLS_ASSERT_OK_AND_ASSIGN( PipelineSchedule schedule, RunPipelineSchedule(proc, *delay_estimator, - options().clock_period_ps(10'000))); + SchedulingOptions().clock_period_ps(10'000))); EXPECT_EQ(schedule.length(), 2); EXPECT_EQ(schedule.cycle(send.node()), 0); EXPECT_EQ(schedule.cycle(rcv.node()), 1); } -TEST_P(PipelineScheduleTest, SendFollowedByDelayedReceive) { +TEST_F(PipelineScheduleTest, SendFollowedByDelayedReceive) { Package package = Package(TestName()); Type* u32 = package.GetBitsType(32); @@ -1097,18 +1006,15 @@ TEST_P(PipelineScheduleTest, SendFollowedByDelayedReceive) { XLS_ASSERT_OK_AND_ASSIGN(const DelayEstimator* delay_estimator, GetDelayEstimator("unit")); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(proc, *delay_estimator, - options().pipeline_stages(5))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(proc, *delay_estimator, + SchedulingOptions().pipeline_stages(5))); EXPECT_EQ(schedule.length(), 5); - if (is_random()) { - EXPECT_GE(schedule.cycle(rcv.node()) - schedule.cycle(send.node()), 3); - } else { - EXPECT_EQ(schedule.cycle(rcv.node()) - schedule.cycle(send.node()), 3); - } + EXPECT_EQ(schedule.cycle(rcv.node()) - schedule.cycle(send.node()), 3); } -TEST_P(PipelineScheduleTest, SendFollowedByDelayedReceiveWithState) { +TEST_F(PipelineScheduleTest, SendFollowedByDelayedReceiveWithState) { Package package = Package(TestName()); Type* u32 = package.GetBitsType(32); @@ -1131,14 +1037,16 @@ TEST_P(PipelineScheduleTest, SendFollowedByDelayedReceiveWithState) { XLS_ASSERT_OK_AND_ASSIGN(const DelayEstimator* delay_estimator, GetDelayEstimator("unit")); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(proc, *delay_estimator, - options().pipeline_stages(5))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(proc, *delay_estimator, + SchedulingOptions().pipeline_stages(5))); EXPECT_EQ(schedule.length(), 5); - EXPECT_EQ(schedule.cycle(send.node()) + 1, schedule.cycle(rcv.node())); + EXPECT_EQ(schedule.cycle(send.node()), 0); + EXPECT_EQ(schedule.cycle(rcv.node()), 1); } -TEST_P(PipelineScheduleErrorTest, SuggestIncreasedPipelineLengthWhenNeeded) { +TEST_F(PipelineScheduleTest, SuggestIncreasedPipelineLengthWhenNeeded) { Package package = Package(TestName()); Type* u32 = package.GetBitsType(32); @@ -1160,16 +1068,16 @@ TEST_P(PipelineScheduleErrorTest, SuggestIncreasedPipelineLengthWhenNeeded) { XLS_ASSERT_OK_AND_ASSIGN(const DelayEstimator* delay_estimator, GetDelayEstimator("unit")); - EXPECT_THAT(RunPipelineSchedule( - proc, *delay_estimator, - options().pipeline_stages(1).worst_case_throughput(3)), - StatusIs(absl::StatusCode::kInvalidArgument, - AllOf(HasSubstr("--pipeline_stages=3"), - Not(HasSubstr("--worst_case_throughput"))))); + EXPECT_THAT( + RunPipelineSchedule( + proc, *delay_estimator, + SchedulingOptions().pipeline_stages(1).worst_case_throughput(3)), + StatusIs(absl::StatusCode::kInvalidArgument, + AllOf(HasSubstr("--pipeline_stages=3"), + Not(HasSubstr("--worst_case_throughput"))))); } -TEST_P(SdcOnlyPipelineScheduleErrorTest, - SuggestReducedThroughputWhenFullThroughputFails) { +TEST_F(PipelineScheduleTest, SuggestReducedThroughputWhenFullThroughputFails) { Package package = Package(TestName()); Type* u32 = package.GetBitsType(32); @@ -1191,15 +1099,14 @@ TEST_P(SdcOnlyPipelineScheduleErrorTest, XLS_ASSERT_OK_AND_ASSIGN(const DelayEstimator* delay_estimator, GetDelayEstimator("unit")); - EXPECT_THAT(RunPipelineSchedule( - proc, *delay_estimator, - options().pipeline_stages(5).worst_case_throughput(1)), + EXPECT_THAT(RunPipelineSchedule(proc, *delay_estimator, + SchedulingOptions().pipeline_stages(5)), StatusIs(absl::StatusCode::kInvalidArgument, AllOf(HasSubstr("--worst_case_throughput=3"), Not(HasSubstr("--pipeline_stages"))))); } -TEST_P(PipelineScheduleTest, UnboundedThroughputWorks) { +TEST_F(PipelineScheduleTest, UnboundedThroughputWorks) { Package package = Package(TestName()); Type* u32 = package.GetBitsType(32); @@ -1225,16 +1132,12 @@ TEST_P(PipelineScheduleTest, UnboundedThroughputWorks) { PipelineSchedule schedule, RunPipelineSchedule( proc, *delay_estimator, - options().pipeline_stages(5).worst_case_throughput(0))); + SchedulingOptions().pipeline_stages(5).worst_case_throughput(0))); EXPECT_EQ(schedule.length(), 5); - if (is_random()) { - EXPECT_GE(schedule.cycle(rcv.node()) - schedule.cycle(send.node()), 2); - } else { - EXPECT_EQ(schedule.cycle(rcv.node()) - schedule.cycle(send.node()), 2); - } + EXPECT_EQ(schedule.cycle(rcv.node()) - schedule.cycle(send.node()), 2); } -TEST_P(PipelineScheduleTest, MinimizedThroughputWorksWithGivenPipelineLength) { +TEST_F(PipelineScheduleTest, MinimizedThroughputWorksWithGivenPipelineLength) { Package package = Package(TestName()); Type* u32 = package.GetBitsType(32); @@ -1259,7 +1162,7 @@ TEST_P(PipelineScheduleTest, MinimizedThroughputWorksWithGivenPipelineLength) { XLS_ASSERT_OK_AND_ASSIGN( PipelineSchedule schedule, RunPipelineSchedule(proc, *delay_estimator, - options() + SchedulingOptions() .pipeline_stages(5) .worst_case_throughput(0) .minimize_worst_case_throughput(true))); @@ -1268,7 +1171,7 @@ TEST_P(PipelineScheduleTest, MinimizedThroughputWorksWithGivenPipelineLength) { EXPECT_EQ(proc->GetInitiationInterval().value_or(1), 4); } -TEST_P(PipelineScheduleTest, MinimizedThroughputWorksWithGivenClockPeriod) { +TEST_F(PipelineScheduleTest, MinimizedThroughputWorksWithGivenClockPeriod) { Package package = Package(TestName()); Type* u32 = package.GetBitsType(32); @@ -1293,7 +1196,7 @@ TEST_P(PipelineScheduleTest, MinimizedThroughputWorksWithGivenClockPeriod) { XLS_ASSERT_OK_AND_ASSIGN( PipelineSchedule schedule, RunPipelineSchedule(proc, *delay_estimator, - options() + SchedulingOptions() .clock_period_ps(2) .worst_case_throughput(0) .minimize_worst_case_throughput(true))); @@ -1303,7 +1206,7 @@ TEST_P(PipelineScheduleTest, MinimizedThroughputWorksWithGivenClockPeriod) { EXPECT_EQ(proc->GetInitiationInterval().value_or(1), 3); } -TEST_P(SdcOnlyPipelineScheduleErrorTest, +TEST_F(PipelineScheduleTest, SuggestReducedThroughputWhenFullThroughputFailsWithClockGiven) { Package package = Package(TestName()); @@ -1327,16 +1230,13 @@ TEST_P(SdcOnlyPipelineScheduleErrorTest, XLS_ASSERT_OK_AND_ASSIGN(const DelayEstimator* delay_estimator, GetDelayEstimator("unit")); EXPECT_THAT(RunPipelineSchedule(proc, *delay_estimator, - options() - .clock_period_ps(1000) - .worst_case_throughput(1) - .minimize_clock_on_failure(false)), + SchedulingOptions().clock_period_ps(1000)), StatusIs(absl::StatusCode::kInvalidArgument, AllOf(HasSubstr("--worst_case_throughput=3"), Not(HasSubstr("--pipeline_stages"))))); } -TEST_P(SdcOnlyPipelineScheduleErrorTest, +TEST_F(PipelineScheduleTest, SuggestIncreasedPipelineLengthAndReducedThroughputWhenNeeded) { Package package = Package(TestName()); @@ -1360,17 +1260,13 @@ TEST_P(SdcOnlyPipelineScheduleErrorTest, XLS_ASSERT_OK_AND_ASSIGN(const DelayEstimator* delay_estimator, GetDelayEstimator("unit")); EXPECT_THAT(RunPipelineSchedule(proc, *delay_estimator, - options() - .pipeline_stages(1) - .worst_case_throughput(1) - .minimize_worst_case_throughput(true)), + SchedulingOptions().pipeline_stages(1)), StatusIs(absl::StatusCode::kInvalidArgument, AllOf(HasSubstr("--pipeline_stages=3"), HasSubstr("--worst_case_throughput=3")))); } -TEST_P(SdcOnlyPipelineScheduleErrorTest, - SuggestIncreasedPipelineLengthAndIndividualSlack) { +TEST_F(PipelineScheduleTest, SuggestIncreasedPipelineLengthAndIndividualSlack) { Package package = Package(TestName()); Type* u32 = package.GetBitsType(32); @@ -1394,33 +1290,21 @@ TEST_P(SdcOnlyPipelineScheduleErrorTest, XLS_ASSERT_OK_AND_ASSIGN(const DelayEstimator* delay_estimator, GetDelayEstimator("unit")); - auto sched = RunPipelineSchedule( - proc, *delay_estimator, - options().pipeline_stages(1).failure_behavior(SchedulingFailureBehavior{ - .explain_infeasibility = true, - .infeasible_per_state_backedge_slack_pool = 2.0})); - if (is_sdc()) { - EXPECT_THAT( - sched, - StatusIs( - absl::StatusCode::kInvalidArgument, - AllOf(HasSubstr("--pipeline_stages=3"), - HasSubstr("looking at paths between state and next_state " - "(needs 2 additional slack)")))); - } else { - EXPECT_THAT(sched, StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("--pipeline_stages=3"))); - } EXPECT_THAT( - sched, + RunPipelineSchedule( + proc, *delay_estimator, + SchedulingOptions().pipeline_stages(1).failure_behavior( + SchedulingFailureBehavior{ + .explain_infeasibility = true, + .infeasible_per_state_backedge_slack_pool = 2.0})), StatusIs(absl::StatusCode::kInvalidArgument, AllOf(HasSubstr("--pipeline_stages=3"), HasSubstr("looking at paths between state and next_state " "(needs 2 additional slack)")))); } -TEST_P( - SdcOnlyPipelineScheduleErrorTest, +TEST_F( + PipelineScheduleTest, SuggestIncreasedPipelineLengthWorstCaseThroughtputAndIndividualSlackPool2) { Package package = Package(TestName()); @@ -1457,7 +1341,7 @@ TEST_P( EXPECT_THAT( RunPipelineSchedule( proc, *delay_estimator, - options().pipeline_stages(1).failure_behavior( + SchedulingOptions().pipeline_stages(1).failure_behavior( SchedulingFailureBehavior{ .explain_infeasibility = true, .infeasible_per_state_backedge_slack_pool = 2.0})), @@ -1469,8 +1353,8 @@ TEST_P( "(needs 1 additional slack)")))); } -TEST_P( - SdcOnlyPipelineScheduleErrorTest, +TEST_F( + PipelineScheduleTest, SuggestIncreasedPipelineLengthWorstCaseThroughtputAndIndividualSlackPool3) { Package package = Package(TestName()); @@ -1517,7 +1401,7 @@ TEST_P( EXPECT_THAT( RunPipelineSchedule( proc, *delay_estimator, - options().pipeline_stages(1).failure_behavior( + SchedulingOptions().pipeline_stages(1).failure_behavior( SchedulingFailureBehavior{ .explain_infeasibility = true, .infeasible_per_state_backedge_slack_pool = @@ -1533,17 +1417,15 @@ TEST_P( "(needs 1 additional slack)")))); } -void SuggestIncreasedClockPeriodWhenNecessaryCommon( - std::string test_name, const DelayEstimator& delay_estimator, - SchedulingOptions options, std::string expected) { - Package package = Package(test_name); +TEST_F(PipelineScheduleTest, SuggestIncreasedClockPeriodWhenNecessary) { + Package package = Package(TestName()); Type* u32 = package.GetBitsType(32); XLS_ASSERT_OK_AND_ASSIGN( Channel * ch_out, package.CreateStreamingChannel("out", ChannelOps::kSendOnly, u32)); - ProcBuilder pb(test_name, &package); + ProcBuilder pb(TestName(), &package); BValue tkn = pb.Literal(Value::Token()); BValue state = pb.StateElement("state", Value(Bits(32))); @@ -1555,62 +1437,44 @@ void SuggestIncreasedClockPeriodWhenNecessaryCommon( // Each operation takes 500ps, so (with no pipeline depth restrictions), 500ps // is the fastest clock we can support. - EXPECT_THAT( - RunPipelineSchedule(proc, delay_estimator, options), - StatusIs(absl::StatusCode::kInvalidArgument, HasSubstr(expected))); -} -TEST_P(PipelineScheduleErrorTest, SuggestIncreasedClockPeriodWhenNecessary500) { - // Each operation takes 500ps, so (with no pipeline depth restrictions), 500ps - // is the fastest clock we can support. - SuggestIncreasedClockPeriodWhenNecessaryCommon( - TestName(), TestDelayEstimator(500), - options().clock_period_ps(100).worst_case_throughput(0), - "--clock_period_ps=500"); -} -TEST_P(PipelineScheduleErrorTest, - SuggestIncreasedClockPeriodWhenNecessary1000) { + EXPECT_THAT(RunPipelineSchedule(proc, TestDelayEstimator(500), + SchedulingOptions().clock_period_ps(100)), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("--clock_period_ps=500"))); + // Each operation takes 500ps, but we have a chain of three operations; in two // stages, the best we can do is a 1000ps clock. - SuggestIncreasedClockPeriodWhenNecessaryCommon( - TestName(), TestDelayEstimator(500), - options().clock_period_ps(100).pipeline_stages(2).worst_case_throughput( - 0), - "--clock_period_ps=1000"); -} -TEST_P(PipelineScheduleErrorTest, - SuggestIncreasedClockPeriodWhenNecessary3Stage) { - // Each operation takes 500ps, and our schedule fits nicely into 3 stages; we - // can get down to a 500ps clock at 3 or more pipeline stages. - SuggestIncreasedClockPeriodWhenNecessaryCommon( - TestName(), TestDelayEstimator(500), - options().clock_period_ps(100).pipeline_stages(3).worst_case_throughput( - 0), - "--clock_period_ps=500"); -} -TEST_P(PipelineScheduleErrorTest, - SuggestIncreasedClockPeriodWhenNecessary20Stage) { + EXPECT_THAT(RunPipelineSchedule( + proc, TestDelayEstimator(500), + SchedulingOptions().clock_period_ps(100).pipeline_stages(2)), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("--clock_period_ps=1000"))); + // Each operation takes 500ps, and our schedule fits nicely into 3 stages; we // can get down to a 500ps clock at 3 or more pipeline stages. - SuggestIncreasedClockPeriodWhenNecessaryCommon( - TestName(), TestDelayEstimator(500), - options().clock_period_ps(100).pipeline_stages(20).worst_case_throughput( - 0), - "--clock_period_ps=500"); -} -TEST_P(PipelineScheduleErrorTest, - SuggestIncreasedClockPeriodWhenNecessary4StageNoMinimize) { + EXPECT_THAT(RunPipelineSchedule( + proc, TestDelayEstimator(500), + SchedulingOptions().clock_period_ps(100).pipeline_stages(3)), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("--clock_period_ps=500"))); + EXPECT_THAT(RunPipelineSchedule( + proc, TestDelayEstimator(500), + SchedulingOptions().clock_period_ps(100).pipeline_stages(20)), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("--clock_period_ps=500"))); + // But... if told not to search for the smallest possible clock period, the // best we can do is signal that a longer clock period might help. - SuggestIncreasedClockPeriodWhenNecessaryCommon( - TestName(), TestDelayEstimator(500), - options() - .clock_period_ps(100) - .pipeline_stages(4) - .minimize_clock_on_failure(false), - "Try increasing `--clock_period_ps`"); + EXPECT_THAT(RunPipelineSchedule(proc, TestDelayEstimator(500), + SchedulingOptions() + .clock_period_ps(100) + .pipeline_stages(4) + .minimize_clock_on_failure(false)), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("Try increasing `--clock_period_ps`"))); } -TEST_P(SdcPrimaryPipelineScheduleTest, OptimizeForDynamicThroughput) { +TEST_F(PipelineScheduleTest, OptimizeForDynamicThroughput) { Package package = Package(TestName()); Type* u1 = package.GetBitsType(1); Type* u2 = package.GetBitsType(2); @@ -1642,14 +1506,11 @@ TEST_P(SdcPrimaryPipelineScheduleTest, OptimizeForDynamicThroughput) { // // NOTE: This is a VERY contrived example, but there are real examples of this // happening in the wild. - // - // NB This is only true for SDC scheduling. ASAP will put it in stage 0 - // regardless of the penalty. XLS_ASSERT_OK_AND_ASSIGN( PipelineSchedule schedule, RunPipelineSchedule( proc, *delay_estimator, - options().clock_period_ps(10).worst_case_throughput(2))); + SchedulingOptions().clock_period_ps(10).worst_case_throughput(2))); EXPECT_EQ(schedule.cycle(next_state.node()) - schedule.cycle(state.node()), 1); @@ -1658,7 +1519,7 @@ TEST_P(SdcPrimaryPipelineScheduleTest, OptimizeForDynamicThroughput) { XLS_ASSERT_OK_AND_ASSIGN( PipelineSchedule schedule2, RunPipelineSchedule(proc, *delay_estimator, - options() + SchedulingOptions() .clock_period_ps(10) .worst_case_throughput(2) .dynamic_throughput_objective_weight(1024.0))); @@ -1673,7 +1534,7 @@ TEST_P(SdcPrimaryPipelineScheduleTest, OptimizeForDynamicThroughput) { // receive nodes where the second receive node depends on the first node, and // the first receive node produces the next state node and the param is used by // the second receive node. -TEST_P(PipelineScheduleTest, ProcParamScheduledEarlyWithNextState) { +TEST_F(PipelineScheduleTest, ProcParamScheduledEarlyWithNextState) { Package p("p"); Type* u1 = p.GetBitsType(1); XLS_ASSERT_OK_AND_ASSIGN( @@ -1698,17 +1559,15 @@ TEST_P(PipelineScheduleTest, ProcParamScheduledEarlyWithNextState) { XLS_ASSERT_OK_AND_ASSIGN(const DelayEstimator* delay_estimator, GetDelayEstimator("unit")); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(proc, *delay_estimator, - options().pipeline_stages(3))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(proc, *delay_estimator, + SchedulingOptions().pipeline_stages(3))); EXPECT_EQ(schedule.length(), 3); - if (!is_random()) { - // The state's param node should be scheduled ASAP (i.e., the next-state - // node's stage), while still leaving its user in the later stage. - EXPECT_EQ(schedule.cycle(state.node()), - schedule.cycle(nb_rcv_valid.node())); - EXPECT_LT(schedule.cycle(state.node()), schedule.cycle(use_state.node())); - } + // The state's param node should be scheduled ASAP (i.e., the next-state + // node's stage), while still leaving its user in the later stage. + EXPECT_EQ(schedule.cycle(state.node()), schedule.cycle(nb_rcv_valid.node())); + EXPECT_LT(schedule.cycle(state.node()), schedule.cycle(use_state.node())); } // Proc next state does not depend on param. Force schedule of a param node's @@ -1718,14 +1577,7 @@ TEST_P(PipelineScheduleTest, ProcParamScheduledEarlyWithNextState) { // the param is used by the second receive node. We make the scheduler prefer to // schedule the next-state computation earlier by making it narrower than the // param value, then widening later. -// -// We also ask to minimize WCT to force the next state and state to be close -// even on schedulers incapable of minimizing cross cycle edges such as ASAP. -TEST_P(PipelineScheduleTest, ProcParamScheduledAfterNextState) { - if (is_random()) { - // TODO(allight): Fix this. - GTEST_SKIP() << "Random scheduler can fail this for unclear reasons."; - } +TEST_F(PipelineScheduleTest, ProcParamScheduledAfterNextState) { Package p("p"); Type* u1 = p.GetBitsType(1); XLS_ASSERT_OK_AND_ASSIGN( @@ -1754,18 +1606,15 @@ TEST_P(PipelineScheduleTest, ProcParamScheduledAfterNextState) { GetDelayEstimator("unit")); XLS_ASSERT_OK_AND_ASSIGN( PipelineSchedule schedule, - RunPipelineSchedule( - proc, *delay_estimator, - options().pipeline_stages(3).minimize_worst_case_throughput(true))); + RunPipelineSchedule(proc, *delay_estimator, + SchedulingOptions().pipeline_stages(3))); EXPECT_EQ(schedule.length(), 3); EXPECT_GT(schedule.cycle(state.node()), schedule.cycle(nb_rcv_valid.node())); } // If two param nodes are mutually dependent, they (and their next state nodes) // all need to be scheduled in the same stage. -// -// TODO(allight): This is only true for SDC I think. -TEST_P(PipelineScheduleTest, ProcParamsScheduledInSameStage) { +TEST_F(PipelineScheduleTest, ProcParamsScheduledInSameStage) { Package p("p"); Type* u1 = p.GetBitsType(1); XLS_ASSERT_OK_AND_ASSIGN( @@ -1791,17 +1640,17 @@ TEST_P(PipelineScheduleTest, ProcParamsScheduledInSameStage) { XLS_ASSERT_OK_AND_ASSIGN(const DelayEstimator* delay_estimator, GetDelayEstimator("unit")); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(proc, *delay_estimator, - options().pipeline_stages(3))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(proc, *delay_estimator, + SchedulingOptions().pipeline_stages(3))); EXPECT_EQ(schedule.length(), 3); - if (!is_random()) { - EXPECT_EQ(schedule.cycle(a.node()), schedule.cycle(b.node())); - } - EXPECT_EQ(schedule.cycle(next_a.node()), schedule.cycle(next_a.node())); + EXPECT_EQ(schedule.cycle(a.node()), schedule.cycle(b.node())); + EXPECT_EQ(schedule.cycle(a.node()), schedule.cycle(next_a.node())); + EXPECT_EQ(schedule.cycle(a.node()), schedule.cycle(next_b.node())); } -TEST_P(PipelineScheduleTest, FunctionScheduleWithInputAndOutputDelay) { +TEST_F(PipelineScheduleTest, FunctionScheduleWithInputAndOutputDelay) { Package p("p"); Type* u16 = p.GetBitsType(16); @@ -1815,9 +1664,10 @@ TEST_P(PipelineScheduleTest, FunctionScheduleWithInputAndOutputDelay) { XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.BuildWithReturnValue(negate)); // No additional input/output delay, we get [{x, y, prod, negate}] - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(f, TestDelayEstimator(), - options().clock_period_ps(2))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(f, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(2))); ASSERT_EQ(schedule.length(), 1); EXPECT_THAT( schedule.nodes_in_cycle(0), @@ -1826,9 +1676,10 @@ TEST_P(PipelineScheduleTest, FunctionScheduleWithInputAndOutputDelay) { // Additional input delay bumps prod to stage 2, we get // [{x,y}, {prod, negate}] XLS_ASSERT_OK_AND_ASSIGN( - schedule, RunPipelineSchedule( - f, TestDelayEstimator(), - options().clock_period_ps(2).additional_input_delay_ps(2))); + schedule, + RunPipelineSchedule( + f, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(2).additional_input_delay_ps(2))); ASSERT_EQ(schedule.length(), 2); EXPECT_THAT(schedule.nodes_in_cycle(0), UnorderedElementsAre(x.node(), y.node())); @@ -1839,7 +1690,7 @@ TEST_P(PipelineScheduleTest, FunctionScheduleWithInputAndOutputDelay) { // [{x,y}, {prod}, {negate}] XLS_ASSERT_OK_AND_ASSIGN( schedule, RunPipelineSchedule(f, TestDelayEstimator(), - options() + SchedulingOptions() .clock_period_ps(2) .additional_input_delay_ps(2) .additional_output_delay_ps(1))); @@ -1850,7 +1701,7 @@ TEST_P(PipelineScheduleTest, FunctionScheduleWithInputAndOutputDelay) { EXPECT_THAT(schedule.nodes_in_cycle(2), UnorderedElementsAre(negate.node())); } -TEST_P(PipelineScheduleTest, ProcScheduleWithInputDelay) { +TEST_F(PipelineScheduleTest, ProcScheduleWithInputDelay) { Package p("p"); Type* u16 = p.GetBitsType(16); @@ -1870,9 +1721,10 @@ TEST_P(PipelineScheduleTest, ProcScheduleWithInputDelay) { XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build({})); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(proc, TestDelayEstimator(), - options().clock_period_ps(4))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(proc, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(4))); EXPECT_EQ(schedule.length(), 2); EXPECT_EQ(schedule.cycle(rcv.node()), 0); EXPECT_EQ(schedule.cycle(send.node()), 1); @@ -1882,9 +1734,10 @@ TEST_P(PipelineScheduleTest, ProcScheduleWithInputDelay) { // Input delay of 1 is not large enough to bump all non-zero-latency nodes to // later stages. XLS_ASSERT_OK_AND_ASSIGN( - schedule, RunPipelineSchedule( - proc, TestDelayEstimator(), - options().clock_period_ps(4).additional_input_delay_ps(1))); + schedule, + RunPipelineSchedule( + proc, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(4).additional_input_delay_ps(1))); EXPECT_THAT(schedule.nodes_in_cycle(0), Contains(OperationDelayInPs(IsOkAndHolds(Gt(0))))); @@ -1896,18 +1749,15 @@ TEST_P(PipelineScheduleTest, ProcScheduleWithInputDelay) { // receive.3: (token, bits[16]) = receive(tkn, channel_id=0, id=3) // st: () = param(st, id=2) XLS_ASSERT_OK_AND_ASSIGN( - schedule, RunPipelineSchedule( - proc, TestDelayEstimator(), - options().clock_period_ps(4).additional_input_delay_ps(4))); + schedule, + RunPipelineSchedule( + proc, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(4).additional_input_delay_ps(4))); EXPECT_THAT(schedule.nodes_in_cycle(0), Each(OperationDelayInPs(IsOkAndHolds(0)))); } -TEST_P(PipelineScheduleTest, ProcScheduleWithInputAndOutputDelay) { - if (is_random()) { - GTEST_SKIP() << "Skipping test for random scheduler due to being unable to " - "realistically check output."; - } +TEST_F(PipelineScheduleTest, ProcScheduleWithInputAndOutputDelay) { Package p("p"); Type* u16 = p.GetBitsType(16); @@ -1928,18 +1778,20 @@ TEST_P(PipelineScheduleTest, ProcScheduleWithInputAndOutputDelay) { XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build({})); // No input delay, we get [{rcv, negate, send}] - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(proc, TestDelayEstimator(), - options().clock_period_ps(2))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(proc, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(2))); ASSERT_EQ(schedule.length(), 1); EXPECT_THAT(schedule.nodes_in_cycle(0), IsSupersetOf({rcv.node(), negate.node(), send.node()})); // Input delay bumps send to stage 1, we get [{rcv, negate}, {send}] XLS_ASSERT_OK_AND_ASSIGN( - schedule, RunPipelineSchedule( - proc, TestDelayEstimator(), - options().clock_period_ps(2).additional_input_delay_ps(1))); + schedule, + RunPipelineSchedule( + proc, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(2).additional_input_delay_ps(1))); ASSERT_EQ(schedule.length(), 2); EXPECT_THAT(schedule.nodes_in_cycle(0), IsSupersetOf({rcv.node(), negate.node()})); @@ -1950,7 +1802,8 @@ TEST_P(PipelineScheduleTest, ProcScheduleWithInputAndOutputDelay) { schedule, RunPipelineSchedule( proc, TestDelayEstimator(), - options().clock_period_ps(2).additional_output_delay_ps(1))); + SchedulingOptions().clock_period_ps(2).additional_output_delay_ps( + 1))); ASSERT_EQ(schedule.length(), 2); EXPECT_THAT(schedule.nodes_in_cycle(0), IsSupersetOf({rcv.node(), negate.node()})); @@ -1959,7 +1812,7 @@ TEST_P(PipelineScheduleTest, ProcScheduleWithInputAndOutputDelay) { // Specifying both input and output delay doesn't change anything. XLS_ASSERT_OK_AND_ASSIGN( schedule, RunPipelineSchedule(proc, TestDelayEstimator(), - options() + SchedulingOptions() .clock_period_ps(2) .additional_input_delay_ps(1) .additional_output_delay_ps(1))); @@ -1969,10 +1822,7 @@ TEST_P(PipelineScheduleTest, ProcScheduleWithInputAndOutputDelay) { EXPECT_THAT(schedule.nodes_in_cycle(1), IsSupersetOf({send.node()})); } -TEST_P(PipelineScheduleTest, ProcScheduleWithChannelSpecificDelay) { - if (is_random()) { - GTEST_SKIP() << "Unable to verify output for random scheduler."; - } +TEST_F(PipelineScheduleTest, ProcScheduleWithChannelSpecificDelay) { Package p("p"); Type* u16 = p.GetBitsType(16); @@ -1999,9 +1849,10 @@ TEST_P(PipelineScheduleTest, ProcScheduleWithChannelSpecificDelay) { XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build({})); // No input delay, we get [{rcv1, neg_rcv1, rcv2, neg_rcv2, sum, send}] - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule, - RunPipelineSchedule(proc, TestDelayEstimator(), - options().clock_period_ps(2))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule, + RunPipelineSchedule(proc, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(2))); ASSERT_EQ(schedule.length(), 1); EXPECT_THAT( schedule.nodes_in_cycle(0), @@ -2011,10 +1862,10 @@ TEST_P(PipelineScheduleTest, ProcScheduleWithChannelSpecificDelay) { // [{rcv1, neg_rcv1, rcv2, neg_rcv2}, {sum, send}] XLS_ASSERT_OK_AND_ASSIGN( schedule, - RunPipelineSchedule( - proc, TestDelayEstimator(), - options().clock_period_ps(2).add_additional_channel_delay_ps("in2", - 1))); + RunPipelineSchedule(proc, TestDelayEstimator(), + SchedulingOptions() + .clock_period_ps(2) + .add_additional_channel_delay_ps("in2", 1))); ASSERT_EQ(schedule.length(), 2); EXPECT_THAT(schedule.nodes_in_cycle(0), IsSupersetOf({rcv1.node(), neg_rcv1.node(), rcv2.node(), @@ -2026,10 +1877,10 @@ TEST_P(PipelineScheduleTest, ProcScheduleWithChannelSpecificDelay) { // [{rcv1, neg_rcv1, rcv2}, {neg_rcv2, sum, send}] XLS_ASSERT_OK_AND_ASSIGN( schedule, - RunPipelineSchedule( - proc, TestDelayEstimator(), - options().clock_period_ps(2).add_additional_channel_delay_ps("in2", - 2))); + RunPipelineSchedule(proc, TestDelayEstimator(), + SchedulingOptions() + .clock_period_ps(2) + .add_additional_channel_delay_ps("in2", 2))); ASSERT_EQ(schedule.length(), 2); EXPECT_THAT(schedule.nodes_in_cycle(0), IsSupersetOf({rcv1.node(), neg_rcv1.node(), rcv2.node()})); @@ -2037,7 +1888,7 @@ TEST_P(PipelineScheduleTest, ProcScheduleWithChannelSpecificDelay) { IsSupersetOf({neg_rcv2.node(), sum.node(), send.node()})); } -TEST_P(PipelineScheduleTest, ProcScheduleWithChannelDirectionSpecificDelay) { +TEST_F(PipelineScheduleTest, ProcScheduleWithChannelDirectionSpecificDelay) { Package p("p"); Type* u16 = p.GetBitsType(16); @@ -2056,7 +1907,7 @@ TEST_P(PipelineScheduleTest, ProcScheduleWithChannelDirectionSpecificDelay) { // Set different delays for send and receive on channel "ch". SchedulingOptions options = - this->options() + SchedulingOptions() .clock_period_ps(5) .add_additional_channel_delay_ps("ch:recv", 1) .add_additional_channel_delay_ps("ch:send", 5); @@ -2074,11 +1925,7 @@ TEST_P(PipelineScheduleTest, ProcScheduleWithChannelDirectionSpecificDelay) { EXPECT_EQ(schedule.cycle(send.node()), 1); } -TEST_P(PipelineScheduleTest, ProcScheduleWithConstraints) { - if (is_random()) { - GTEST_SKIP() << "Random scheduler does not fully respect this constraint " - "in all circumstances"; - } +TEST_F(PipelineScheduleTest, ProcScheduleWithConstraints) { Package p("p"); Type* u16 = p.GetBitsType(16); XLS_ASSERT_OK_AND_ASSIGN( @@ -2097,17 +1944,18 @@ TEST_P(PipelineScheduleTest, ProcScheduleWithConstraints) { for (int64_t i = 3; i <= 9; ++i) { XLS_ASSERT_OK_AND_ASSIGN( PipelineSchedule schedule, - RunPipelineSchedule(proc, TestDelayEstimator(), - options().pipeline_stages(10).add_constraint( - IOConstraint("in", IODirection::kReceive, "out", - IODirection::kSend, i, i)))); + RunPipelineSchedule( + proc, TestDelayEstimator(), + SchedulingOptions().pipeline_stages(10).add_constraint( + IOConstraint("in", IODirection::kReceive, "out", + IODirection::kSend, i, i)))); EXPECT_EQ(schedule.length(), 10); EXPECT_EQ(schedule.cycle(send.node()) - schedule.cycle(rcv.node()), i); } } -TEST_P(RandomPipelineScheduleTest, RandomScheduleRuns) { +TEST_F(PipelineScheduleTest, RandomSchedule) { auto p = CreatePackage(); FunctionBuilder fb(TestName(), p.get()); auto x = fb.Param("x", p->GetBitsType(32)); @@ -2117,15 +1965,18 @@ TEST_P(RandomPipelineScheduleTest, RandomScheduleRuns) { XLS_ASSERT_OK_AND_ASSIGN(Function * func, fb.Build()); - for (int32_t i = 0; i < 100; ++i) { + for (int32_t i = 0; i < 1000; ++i) { // Running the scheduler will call `VerifyTiming`. - XLS_ASSERT_OK(RunPipelineSchedule(func, TestDelayEstimator(), - options().seed(i).pipeline_stages(50)) - .status()); + XLS_ASSERT_OK( + RunPipelineSchedule(func, TestDelayEstimator(), + SchedulingOptions(SchedulingStrategy::RANDOM) + .seed(i) + .pipeline_stages(50)) + .status()); } } -TEST_P(SdcPipelineScheduleTest, SingleStageSchedule) { +TEST_F(PipelineScheduleTest, SingleStageSchedule) { auto p = CreatePackage(); FunctionBuilder fb(TestName(), p.get()); auto x = fb.Param("x", p->GetBitsType(32)); @@ -2145,11 +1996,7 @@ TEST_P(SdcPipelineScheduleTest, SingleStageSchedule) { EXPECT_EQ(schedule.value().nodes_in_cycle(0).size(), 21); } -TEST_P(PipelineScheduleTest, LoopbackChannelWithConstraint) { - if (is_random()) { - GTEST_SKIP() << "Random scheduler does not fully respect this constraint " - "in all circumstances"; - } +TEST_F(PipelineScheduleTest, LoopbackChannelWithConstraint) { auto p = CreatePackage(); Type* u16 = p->GetBitsType(16); XLS_ASSERT_OK_AND_ASSIGN( @@ -2171,7 +2018,7 @@ TEST_P(PipelineScheduleTest, LoopbackChannelWithConstraint) { PipelineSchedule schedule, RunPipelineSchedule( proc, TestDelayEstimator(), - options().pipeline_stages(10).add_constraint( + SchedulingOptions().pipeline_stages(10).add_constraint( IOConstraint("loopback", IODirection::kReceive, "loopback", IODirection::kSend, i, i)))); @@ -2181,7 +2028,7 @@ TEST_P(PipelineScheduleTest, LoopbackChannelWithConstraint) { } } -TEST_P(PipelineScheduleTest, PackageScheduleProtoSerializeAndDeserialize) { +TEST_F(PipelineScheduleTest, PackageScheduleProtoSerializeAndDeserialize) { auto p = CreatePackage(); auto make_test_fn = [](Package* p, std::string_view name) { FunctionBuilder fb(name, p); @@ -2194,12 +2041,14 @@ TEST_P(PipelineScheduleTest, PackageScheduleProtoSerializeAndDeserialize) { Function * func0, make_test_fn(p.get(), absl::StrCat(TestName(), "0"))); XLS_ASSERT_OK_AND_ASSIGN( Function * func1, make_test_fn(p.get(), absl::StrCat(TestName(), "1"))); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule0, - RunPipelineSchedule(func0, TestDelayEstimator(), - options().pipeline_stages(3))); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule1, - RunPipelineSchedule(func1, TestDelayEstimator(), - options().pipeline_stages(3))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule0, + RunPipelineSchedule(func0, TestDelayEstimator(), + SchedulingOptions().pipeline_stages(3))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule1, + RunPipelineSchedule(func1, TestDelayEstimator(), + SchedulingOptions().pipeline_stages(3))); PackageSchedule package_schedule(p.get(), {{func0, schedule0}, {func1, schedule1}}); @@ -2221,10 +2070,7 @@ TEST_P(PipelineScheduleTest, PackageScheduleProtoSerializeAndDeserialize) { Each(CyclesMatch(package_schedule.GetSchedules(), clone.GetSchedules()))); } -// TODO(allight): We should rewrite the tests to allow for running with ASAP -// scheduler too. -TEST_P(SdcPipelineScheduleTest, - SerializeAndDeserializeWithSynchronousSchedule) { +TEST_F(PipelineScheduleTest, SerializeAndDeserializeWithSynchronousSchedule) { auto p = CreatePackage(); TokenlessProcBuilder pb1("proc1", "tkn", p.get()); auto literal1 = pb1.Literal(UBits(1, 32)); @@ -2236,12 +2082,14 @@ TEST_P(SdcPipelineScheduleTest, auto add2 = pb2.Add(literal2, literal2); XLS_ASSERT_OK_AND_ASSIGN(Proc * proc2, pb2.Build()); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule1, - RunPipelineSchedule(proc1, TestDelayEstimator(), - options().clock_period_ps(1))); - XLS_ASSERT_OK_AND_ASSIGN(PipelineSchedule schedule2, - RunPipelineSchedule(proc2, TestDelayEstimator(), - options().clock_period_ps(1))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule1, + RunPipelineSchedule(proc1, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(1))); + XLS_ASSERT_OK_AND_ASSIGN( + PipelineSchedule schedule2, + RunPipelineSchedule(proc2, TestDelayEstimator(), + SchedulingOptions().clock_period_ps(1))); absl::flat_hash_map synchronous_offsets( {{proc1, 0}, {proc2, 42}}); @@ -2270,7 +2118,7 @@ TEST_P(SdcPipelineScheduleTest, EXPECT_EQ(clone.GetSynchronousCycle(add2.node()), 42); } -TEST_P(PipelineScheduleTest, ProcWithExplicitStateAccess) { +TEST_F(PipelineScheduleTest, ProcWithExplicitStateAccess) { Package p(TestName()); Type* u32 = p.GetBitsType(32); XLS_ASSERT_OK_AND_ASSIGN( @@ -2289,7 +2137,7 @@ TEST_P(PipelineScheduleTest, ProcWithExplicitStateAccess) { XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build()); - SchedulingOptions options = this->options(); + SchedulingOptions options(SchedulingStrategy::ASAP); options.clock_period_ps(2); XLS_ASSERT_OK_AND_ASSIGN( @@ -2299,7 +2147,7 @@ TEST_P(PipelineScheduleTest, ProcWithExplicitStateAccess) { EXPECT_EQ(schedule.length(), 1); } -TEST_P(PipelineScheduleTest, ProcWithMultipleStateReads) { +TEST_F(PipelineScheduleTest, ProcWithMultipleStateReads) { Package p(TestName()); Type* u32 = p.GetBitsType(32); XLS_ASSERT_OK_AND_ASSIGN( @@ -2322,7 +2170,7 @@ TEST_P(PipelineScheduleTest, ProcWithMultipleStateReads) { XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build()); - SchedulingOptions options = this->options(); + SchedulingOptions options(SchedulingStrategy::ASAP); options.clock_period_ps(2); XLS_ASSERT_OK_AND_ASSIGN( @@ -2332,7 +2180,7 @@ TEST_P(PipelineScheduleTest, ProcWithMultipleStateReads) { EXPECT_EQ(schedule.length(), 1); } -TEST_P(PipelineScheduleErrorTest, ProcWithZeroReadsErrors) { +TEST_F(PipelineScheduleTest, ProcWithZeroReadsErrors) { Package p(TestName()); TokenlessProcBuilder pb("the_proc", "tkn", &p); XLS_ASSERT_OK_AND_ASSIGN(StateElement * se, @@ -2343,7 +2191,7 @@ TEST_P(PipelineScheduleErrorTest, ProcWithZeroReadsErrors) { XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build()); - SchedulingOptions options = this->options(); + SchedulingOptions options(SchedulingStrategy::ASAP); options.clock_period_ps(2); EXPECT_THAT(RunPipelineSchedule(proc, TestDelayEstimator(), options), @@ -2351,56 +2199,5 @@ TEST_P(PipelineScheduleErrorTest, ProcWithZeroReadsErrors) { testing::HasSubstr("has no reads"))); } -INSTANTIATE_TEST_SUITE_P( - PipelineScheduleTest, PipelineScheduleTest, - testing::Values( - Strategies{SchedulingStrategy::ASAP, SchedulingStrategy::ASAP}, - Strategies{SchedulingStrategy::ASAP, SchedulingStrategy::SDC}, - Strategies{SchedulingStrategy::SDC, SchedulingStrategy::ASAP}, - Strategies{SchedulingStrategy::SDC, SchedulingStrategy::SDC}, - // TODO(allight): Min cut doesn't respect all constraints yet. - // Strategies{SchedulingStrategy::MIN_CUT, SchedulingStrategy::ASAP}, - // Strategies{SchedulingStrategy::MIN_CUT, SchedulingStrategy::SDC}, - Strategies{SchedulingStrategy::RANDOM, SchedulingStrategy::ASAP}, - Strategies{SchedulingStrategy::RANDOM, SchedulingStrategy::SDC}), - testing::PrintToStringParamName()); -INSTANTIATE_TEST_SUITE_P( - PipelineScheduleErrorTest, PipelineScheduleErrorTest, - testing::Values( - Strategies{SchedulingStrategy::ASAP, SchedulingStrategy::ASAP}, - Strategies{SchedulingStrategy::ASAP, SchedulingStrategy::SDC}, - Strategies{SchedulingStrategy::SDC, SchedulingStrategy::ASAP}, - Strategies{SchedulingStrategy::SDC, SchedulingStrategy::SDC}), - testing::PrintToStringParamName()); -// TODO(allight): Ideally this suite wouldn't need to exist and all error -// messages would be handled generically. -INSTANTIATE_TEST_SUITE_P(SdcOnlyPipelineScheduleErrorTest, - SdcOnlyPipelineScheduleErrorTest, - testing::Values(Strategies{SchedulingStrategy::SDC, - SchedulingStrategy::SDC}), - testing::PrintToStringParamName()); - -INSTANTIATE_TEST_SUITE_P(AsapPipelineScheduleTest, AsapPipelineScheduleTest, - testing::Values(Strategies{SchedulingStrategy::ASAP, - SchedulingStrategy::ASAP}), - testing::PrintToStringParamName()); - -INSTANTIATE_TEST_SUITE_P(SdcPipelineScheduleTest, SdcPipelineScheduleTest, - testing::Values(Strategies{SchedulingStrategy::SDC, - SchedulingStrategy::SDC}), - testing::PrintToStringParamName()); -INSTANTIATE_TEST_SUITE_P(RandomPipelineScheduleTest, RandomPipelineScheduleTest, - testing::Values(Strategies{SchedulingStrategy::RANDOM, - SchedulingStrategy::ASAP}, - Strategies{SchedulingStrategy::RANDOM, - SchedulingStrategy::SDC}), - testing::PrintToStringParamName()); -INSTANTIATE_TEST_SUITE_P(SdcPrimaryPipelineScheduleTest, - SdcPrimaryPipelineScheduleTest, - testing::Values(Strategies{SchedulingStrategy::SDC, - SchedulingStrategy::SDC}, - Strategies{SchedulingStrategy::SDC, - SchedulingStrategy::ASAP}), - testing::PrintToStringParamName()); } // namespace } // namespace xls diff --git a/xls/scheduling/pipeline_scheduling_pass_test.cc b/xls/scheduling/pipeline_scheduling_pass_test.cc index c922880f9b..9fad77be27 100644 --- a/xls/scheduling/pipeline_scheduling_pass_test.cc +++ b/xls/scheduling/pipeline_scheduling_pass_test.cc @@ -25,8 +25,6 @@ #include "absl/status/statusor.h" #include "ortools/pdlp/solvers.pb.h" #include "xls/common/file/get_runfile_path.h" -#include "xls/common/logging/scoped_record_logs.h" -#include "xls/common/logging/scoped_vlog_level.h" #include "xls/common/status/matchers.h" #include "xls/common/status/status_macros.h" #include "xls/fdo/synthesizer.h" @@ -270,9 +268,6 @@ TEST_F(PipelineSchedulingPassTest, FdoWithMultipleProcs) { fb.Add(fb.SMul(fb.Param("a", u64), fb.Param("b", u64)), fb.Param("c", u64)); return fb.Build(); }; - ScopedRecordLogs srl; - ScopedSetVlogLevel ssvl{{"run_pipeline_schedule", 5}, - {"iterative_sdc_scheduler", 5}}; XLS_ASSERT_OK_AND_ASSIGN(Function * func0, make_func("proc0")); XLS_ASSERT_OK_AND_ASSIGN(Function * func1, make_func("proc1")); diff --git a/xls/scheduling/random_scheduler.cc b/xls/scheduling/random_scheduler.cc deleted file mode 100644 index d6f683ddb4..0000000000 --- a/xls/scheduling/random_scheduler.cc +++ /dev/null @@ -1,100 +0,0 @@ -// Copyright 2026 The XLS Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "xls/scheduling/random_scheduler.h" - -#include -#include -#include -#include -#include - -#include "absl/algorithm/container.h" -#include "absl/log/log.h" -#include "absl/random/random.h" -#include "absl/status/statusor.h" -#include "xls/common/status/ret_check.h" -#include "xls/common/status/status_macros.h" -#include "xls/ir/node.h" -#include "xls/scheduling/schedule_bounds.h" -#include "xls/scheduling/schedule_graph.h" -#include "xls/scheduling/scheduling_options.h" - -namespace xls { - -absl::StatusOr> RandomScheduler::ShuffleNodes() { - std::vector nodes; - nodes.reserve(asap_.graph().nodes().size()); - absl::c_transform( - asap_.graph().nodes(), std::back_inserter(nodes), - [](const ScheduleNode& schedule_node) { return schedule_node.node; }); - absl::c_shuffle(nodes, bitgen_); - return nodes; -} - -absl::StatusOr RandomScheduler::GetRandomCycle(Node* node, int64_t low, - int64_t high) { - return absl::Uniform(bitgen_, low, high + 1); -} - -absl::StatusOr RandomScheduler::Schedule( - std::optional pipeline_stages, int64_t clock_period_ps, - SchedulingFailureBehavior failure_behavior, - std::optional worst_case_throughput) { - XLS_RET_CHECK( - std::holds_alternative(asap_.graph().ir_scope())); - auto* f = std::get(asap_.graph().ir_scope()); - // Use ASAP schedule to get max pipeline stages. - if (!pipeline_stages.has_value()) { - XLS_ASSIGN_OR_RETURN( - auto asap_schedule, - asap_.Schedule(pipeline_stages, clock_period_ps, failure_behavior, - worst_case_throughput)); - pipeline_stages = absl::c_max_element(asap_schedule, - [](auto item1, auto item2) { - return item1.second < item2.second; - }) - ->second + - 1; - VLOG(5) << "ASAP Says the schedule is " - << f->DumpIr(ScheduleCycleAnnotator(asap_schedule)); - VLOG(5) << "Max pipeline stages: " << *pipeline_stages; - } - XLS_ASSIGN_OR_RETURN(sched::ScheduleBounds bounds, - asap_.ComputeBounds(pipeline_stages, clock_period_ps, - worst_case_throughput)); - XLS_ASSIGN_OR_RETURN(std::vector all_nodes, ShuffleNodes()); - for (auto* node : all_nodes) { - if (bounds.lb(node) == bounds.ub(node)) { - continue; - } - XLS_ASSIGN_OR_RETURN( - int64_t cycle, GetRandomCycle(node, bounds.lb(node), bounds.ub(node))); - VLOG(5) << "Scheduling node " << node->ToString() << "[" << bounds.lb(node) - << ", " << bounds.ub(node) << "] into " << cycle; - bounds.AddConstraint(NodeInCycleConstraint{node, cycle}); - XLS_RETURN_IF_ERROR( - ASAPSchedulerWrapper::TightenBounds(bounds, f, pipeline_stages)); - } - // Every node is either fully constrained or explicitly put into a random - // cycle. - ScheduleCycleMap cycle_map; - cycle_map.reserve(all_nodes.size()); - for (auto* node : all_nodes) { - cycle_map[node] = bounds.lb(node); - } - return cycle_map; -} - -} // namespace xls diff --git a/xls/scheduling/random_scheduler.h b/xls/scheduling/random_scheduler.h deleted file mode 100644 index 3de791ee7b..0000000000 --- a/xls/scheduling/random_scheduler.h +++ /dev/null @@ -1,78 +0,0 @@ -// Copyright 2026 The XLS Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef XLS_SCHEDULING_RANDOM_SCHEDULER_H_ -#define XLS_SCHEDULING_RANDOM_SCHEDULER_H_ - -#include -#include -#include -#include - -#include "absl/random/random.h" -#include "absl/status/status.h" -#include "absl/status/statusor.h" -#include "absl/types/span.h" -#include "xls/estimators/delay_model/delay_estimator.h" -#include "xls/ir/node.h" -#include "xls/scheduling/asap_scheduler.h" -#include "xls/scheduling/schedule_graph.h" -#include "xls/scheduling/scheduler.h" -#include "xls/scheduling/scheduling_options.h" - -namespace xls { - -class RandomScheduler : public Scheduler { - public: - explicit RandomScheduler(const ScheduleGraph& graph, - DelayEstimator& delay_estimator, absl::BitGen bitgen) - : Scheduler("RandomScheduler"), - asap_(graph, delay_estimator), - bitgen_(std::move(bitgen)) {} - - absl::Status AddConstraints( - absl::Span constraints) override { - return asap_.AddConstraints(constraints); - } - - absl::StatusOr Schedule( - std::optional pipeline_stages, int64_t clock_period_ps, - SchedulingFailureBehavior failure_behavior, - std::optional worst_case_throughput = std::nullopt) override; - - protected: - // Helpers for testing (to allow one to suppress randomness) - virtual absl::StatusOr> ShuffleNodes(); - virtual absl::StatusOr GetRandomCycle(Node* node, int64_t low, - int64_t high); - - private: - // Helper to expose some internal ASAPScheduler methods for Random Scheduler - // use. - class ASAPSchedulerWrapper : public ASAPScheduler { - public: - ASAPSchedulerWrapper(const ScheduleGraph& graph, - DelayEstimator& delay_estimator) - : ASAPScheduler("ASAPSchedulerWrapper", graph, delay_estimator) {} - ~ASAPSchedulerWrapper() override = default; - using ASAPScheduler::ComputeBounds; - using ASAPScheduler::TightenBounds; - }; - ASAPSchedulerWrapper asap_; - absl::BitGen bitgen_; -}; - -} // namespace xls - -#endif // XLS_SCHEDULING_RANDOM_SCHEDULER_H_ diff --git a/xls/scheduling/random_scheduler_test.cc b/xls/scheduling/random_scheduler_test.cc deleted file mode 100644 index 6720b0f2e0..0000000000 --- a/xls/scheduling/random_scheduler_test.cc +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright 2026 The XLS Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "xls/scheduling/random_scheduler.h" - -#include -#include - -#include "gtest/gtest.h" -#include "absl/random/random.h" -#include "absl/status/statusor.h" -#include "xls/common/status/matchers.h" -#include "xls/estimators/delay_model/delay_estimator.h" -#include "xls/ir/bits.h" -#include "xls/ir/channel.h" -#include "xls/ir/channel_ops.h" -#include "xls/ir/function_builder.h" -#include "xls/ir/ir_test_base.h" -#include "xls/ir/value.h" -#include "xls/scheduling/schedule_graph.h" -#include "xls/scheduling/scheduling_options.h" - -namespace xls { -namespace { - -class RandomSchedulerTest : public IrTestBase {}; - -TEST_F(RandomSchedulerTest, SimpleFunction) { - auto p = CreatePackage(); - FunctionBuilder fb(TestName(), p.get()); - auto x = fb.Param("x", p->GetBitsType(32)); - auto y = fb.Param("y", p->GetBitsType(32)); - auto add = fb.Add(x, y); - XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.Build()); - - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleGraph graph, - ScheduleGraph::Create(f, /*dead_after_synthesis=*/{})); - TestDelayEstimator delay_estimator; - RandomScheduler scheduler(graph, delay_estimator, - absl::BitGen(std::seed_seq{42})); - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleCycleMap cycle_map, - scheduler.Schedule(/*pipeline_stages=*/1, - /*clock_period_ps=*/2, SchedulingFailureBehavior{})); - EXPECT_EQ(cycle_map.at(x.node()), 0); - EXPECT_EQ(cycle_map.at(y.node()), 0); - EXPECT_EQ(cycle_map.at(add.node()), 0); -} - -TEST_F(RandomSchedulerTest, SimpleProc) { - auto p = CreatePackage(); - ProcBuilder pb(TestName(), p.get()); - pb.StateElement("x", Value(UBits(0, 32))); - pb.Next(pb.GetStateParam(0), - pb.Add(pb.GetStateParam(0), pb.Literal(UBits(1, 32)))); - XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build()); - - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleGraph graph, - ScheduleGraph::Create(proc, /*dead_after_synthesis=*/{})); - TestDelayEstimator delay_estimator; - RandomScheduler scheduler(graph, delay_estimator, - absl::BitGen(std::seed_seq{42})); - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleCycleMap cycle_map, - scheduler.Schedule(/*pipeline_stages=*/1, - /*clock_period_ps=*/2, SchedulingFailureBehavior{})); - EXPECT_EQ(cycle_map.at(pb.GetStateParam(0).node()), 0); -} - -TEST_F(RandomSchedulerTest, WithIOConstraint) { - auto p = CreatePackage(); - XLS_ASSERT_OK_AND_ASSIGN( - Channel * ch_a, p->CreateStreamingChannel("a", ChannelOps::kReceiveOnly, - p->GetBitsType(32))); - XLS_ASSERT_OK_AND_ASSIGN(Channel * ch_b, - p->CreateStreamingChannel("b", ChannelOps::kSendOnly, - p->GetBitsType(32))); - - ProcBuilder pb(TestName(), p.get()); - BValue tkn = pb.Literal(Value::Token()); - BValue rcv = pb.Receive(ch_a, tkn); - BValue send = pb.Send(ch_b, pb.TupleIndex(rcv, 0), pb.TupleIndex(rcv, 1)); - XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build()); - - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleGraph graph, - ScheduleGraph::Create(proc, /*dead_after_synthesis=*/{})); - TestDelayEstimator delay_estimator; - RandomScheduler scheduler(graph, delay_estimator, - absl::BitGen(std::seed_seq{42})); - XLS_ASSERT_OK(scheduler.AddConstraints({IOConstraint( - "a", IODirection::kReceive, "b", IODirection::kSend, 1, 1)})); - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleCycleMap cycle_map, - scheduler.Schedule(/*pipeline_stages=*/4, - /*clock_period_ps=*/2, SchedulingFailureBehavior{})); - EXPECT_EQ(cycle_map.at(rcv.node()) + 1, cycle_map.at(send.node())); - RecordProperty("graph", proc->DumpIr(ScheduleCycleAnnotator(cycle_map))); -} - -} // namespace -} // namespace xls diff --git a/xls/scheduling/run_pipeline_schedule.cc b/xls/scheduling/run_pipeline_schedule.cc index 97d325a37d..717578715e 100644 --- a/xls/scheduling/run_pipeline_schedule.cc +++ b/xls/scheduling/run_pipeline_schedule.cc @@ -32,12 +32,11 @@ #include "absl/container/flat_hash_set.h" #include "absl/log/check.h" #include "absl/log/log.h" -#include "absl/random/random.h" +#include "absl/random/distributions.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" -#include "absl/strings/str_join.h" #include "absl/types/span.h" #include "xls/common/logging/log_lines.h" #include "xls/common/math_util.h" @@ -60,14 +59,11 @@ #include "xls/ir/proc.h" #include "xls/ir/proc_elaboration.h" #include "xls/ir/topo_sort.h" -#include "xls/scheduling/asap_scheduler.h" #include "xls/scheduling/min_cut_scheduler.h" #include "xls/scheduling/pipeline_schedule.h" -#include "xls/scheduling/random_scheduler.h" #include "xls/scheduling/schedule_bounds.h" #include "xls/scheduling/schedule_graph.h" #include "xls/scheduling/schedule_util.h" -#include "xls/scheduling/scheduler.h" #include "xls/scheduling/scheduling_options.h" #include "xls/scheduling/sdc_scheduler.h" @@ -75,6 +71,33 @@ namespace xls { namespace { +// Returns the nodes of `f` which must be scheduled in the first stage of a +// pipeline. For functions this is parameters. +std::vector FirstStageNodes(FunctionBase* f) { + if (Function* function = dynamic_cast(f)) { + return std::vector(function->params().begin(), + function->params().end()); + } + + return {}; +} + +// Returns the nodes of `f` which must be scheduled in the final stage of a +// pipeline. For functions this is the return value. +std::vector FinalStageNodes(FunctionBase* f) { + if (Function* function = dynamic_cast(f)) { + // If the return value is a parameter, then we do not force the return value + // to be scheduled in the final stage because, as a parameter, the node must + // be in the first stage. + if (function->return_value()->Is()) { + return {}; + } + return {function->return_value()}; + } + + return {}; +} + // Tighten `bounds` to the ASAP/ALAP bounds for each node. If `schedule_length` // is given, then the ALAP bounds are computed with the given length. Otherwise, // we use the minimum viable pipeline length, per the ASAP bounds. @@ -103,51 +126,51 @@ absl::Status TightenBounds(sched::ScheduleBounds& bounds, FunctionBase* f, // Returns the critical path through the given nodes (ordered topologically), // ignoring nodes that will be dead after synthesis. absl::StatusOr ComputeCriticalPath( - const ScheduleGraph& graph, const DelayEstimator& delay_estimator) { + absl::Span topo_sort, + const absl::flat_hash_set& dead_after_synthesis, + const DelayEstimator& delay_estimator) { int64_t function_cp = 0; absl::flat_hash_map node_cp; - for (const auto& sn : graph.nodes()) { - if (sn.is_dead_after_synthesis) { + for (Node* node : topo_sort) { + if (dead_after_synthesis.contains(node)) { continue; } int64_t node_start = 0; - for (Node* operand : sn.node->operands()) { + for (Node* operand : node->operands()) { node_start = std::max(node_start, node_cp[operand]); } XLS_ASSIGN_OR_RETURN(int64_t node_delay, - delay_estimator.GetOperationDelayInPs(sn.node)); - node_cp[sn.node] = node_start + node_delay; - function_cp = std::max(function_cp, node_cp[sn.node]); + delay_estimator.GetOperationDelayInPs(node)); + node_cp[node] = node_start + node_delay; + function_cp = std::max(function_cp, node_cp[node]); } return function_cp; } +absl::StatusOr ComputeCriticalPath( + FunctionBase* f, const DelayEstimator& delay_estimator) { + XLS_ASSIGN_OR_RETURN(absl::flat_hash_set dead_after_synthesis, + GetDeadAfterSynthesisNodes(f)); + XLS_ASSIGN_OR_RETURN(std::vector topo_sort_nodes, TopoSort(f)); + return ComputeCriticalPath(topo_sort_nodes, dead_after_synthesis, + delay_estimator); +} // Returns the minimum clock period in picoseconds for which it is feasible to // schedule the function into a pipeline with the given number of stages. If // `target_clock_period_ps` is specified, will not try to check lower clock // periods than this. absl::StatusOr FindMinimumClockPeriod( - const ScheduleGraph& graph, std::optional pipeline_stages, + FunctionBase* f, std::optional pipeline_stages, std::optional worst_case_throughput, - const DelayEstimator& delay_estimator, Scheduler& scheduler, + const DelayEstimator& delay_estimator, SDCScheduler& scheduler, SchedulingFailureBehavior failure_behavior, std::optional target_clock_period_ps = std::nullopt) { VLOG(4) << "FindMinimumClockPeriod()"; - VLOG(4) << " graph = " << graph.name(); - VLOG(4) << " scheduler = " << scheduler.name(); VLOG(4) << " pipeline stages = " << (pipeline_stages.has_value() ? absl::StrCat(*pipeline_stages) : "(unspecified)"); - VLOG(4) << " worst case throughput = " - << (worst_case_throughput.has_value() - ? absl::StrCat(*worst_case_throughput) - : "(unspecified)"); - VLOG(4) << " target clock period = " - << (target_clock_period_ps.has_value() - ? absl::StrCat(*target_clock_period_ps) - : "(unspecified)"); XLS_ASSIGN_OR_RETURN(int64_t function_cp_ps, - ComputeCriticalPath(graph, delay_estimator)); + ComputeCriticalPath(f, delay_estimator)); // The upper bound of the search is simply the critical path of the entire // function, and the lower bound is the critical path delay evenly distributed @@ -176,13 +199,13 @@ absl::StatusOr FindMinimumClockPeriod( // schedule this function at all; if not, return a useful error. XLS_RETURN_IF_ERROR(scheduler .Schedule(pipeline_stages, pessimistic_clk_period_ps, - failure_behavior, worst_case_throughput) + failure_behavior, + /*check_feasibility=*/true, + worst_case_throughput) .status()) .SetPrepend() - << absl::StrFormat( - "Impossible to schedule %s %s as specified with clock period %d", - (graph.IsSingleProc() ? "proc" : "function"), graph.name(), - pessimistic_clk_period_ps); + << absl::StrFormat("Impossible to schedule %s %s as specified; ", + (f->IsProc() ? "proc" : "function"), f->name()); // Don't waste time explaining infeasibility for the failing points in the // search. @@ -190,16 +213,10 @@ absl::StatusOr FindMinimumClockPeriod( int64_t min_clk_period_ps = BinarySearchMinTrue( optimistic_clk_period_ps, pessimistic_clk_period_ps, [&](int64_t clk_period_ps) { - auto sched = - scheduler.Schedule(pipeline_stages, clk_period_ps, failure_behavior, - worst_case_throughput); - VLOG(5) << "FindMinClockPeriod(" << graph.name() - << ", pipeline_stages=" << pipeline_stages.value_or(-1) - << ", worst_case_throughput=" - << worst_case_throughput.value_or(-1) - << ", clk_period_ps=" << clk_period_ps - << "): " << sched.status(); - return sched.ok(); + return scheduler + .Schedule(pipeline_stages, clk_period_ps, failure_behavior, + /*check_feasibility=*/true, worst_case_throughput) + .ok(); }, BinarySearchAssumptions::kEndKnownTrue); VLOG(4) << "minimum clock period = " << min_clk_period_ps; @@ -211,31 +228,38 @@ absl::StatusOr FindMinimumClockPeriod( // schedule the function into a pipeline with the given number of stages and // target clock period. absl::StatusOr FindMinimumWorstCaseThroughput( - Proc* proc, std::optional pipeline_stages, int64_t clock_period_ps, - Scheduler& scheduler, SchedulingFailureBehavior failure_behavior) { + FunctionBase* f, std::optional pipeline_stages, + int64_t clock_period_ps, SDCScheduler& scheduler, + SchedulingFailureBehavior failure_behavior) { VLOG(4) << "FindMinimumWorstCaseThroughput()"; VLOG(4) << " pipeline stages = " << (pipeline_stages.has_value() ? absl::StrCat(*pipeline_stages) : "(unspecified)") << ", clock period = " << clock_period_ps << " ps"; + Proc* proc = f->AsProcOrDie(); + // Check that it is in fact possible to schedule this function at all, with no // worst-case throughput bound; if not, return a useful error. XLS_ASSIGN_OR_RETURN( ScheduleCycleMap schedule_cycle_map, scheduler.Schedule(pipeline_stages, clock_period_ps, failure_behavior, - /*worst_case_throughput=*/std::nullopt), + /*check_feasibility=*/true, + /*worst_case_throughput=*/0), _.SetPrepend() << absl::StrFormat( - "Impossible to schedule proc %s as specified; proc ", proc->name())); + "Impossible to schedule %s %s as specified; ", + (f->IsProc() ? "proc" : "function"), f->name())); // Extract the worst-case throughput from this schedule as an upper bound. int64_t pessimistic_worst_case_throughput = 1; - for (Next* next : proc->next_values()) { - Node* state_read = next->state_read(); - const int64_t backedge_length = - schedule_cycle_map[next] - schedule_cycle_map[state_read]; - pessimistic_worst_case_throughput = - std::max(pessimistic_worst_case_throughput, backedge_length + 1); + if (f->IsProc()) { + for (Next* next : proc->next_values()) { + Node* state_read = next->state_read(); + const int64_t backedge_length = + schedule_cycle_map[next] - schedule_cycle_map[state_read]; + pessimistic_worst_case_throughput = + std::max(pessimistic_worst_case_throughput, backedge_length + 1); + } } VLOG(4) << absl::StreamFormat( "Schedules at worst-case throughput %d; now binary searching over " @@ -248,19 +272,11 @@ absl::StatusOr FindMinimumWorstCaseThroughput( int64_t min_worst_case_throughput = BinarySearchMinTrue( 1, pessimistic_worst_case_throughput, [&](int64_t worst_case_throughput) { - CHECK(!failure_behavior.explain_infeasibility); - auto sched = scheduler.Schedule( - pipeline_stages, clock_period_ps, failure_behavior, - /*worst_case_throughput=*/worst_case_throughput); - VLOG(5) << "FindMinimumWorstCaseThroughput(" - << absl::StreamFormat( - "pipeline_stages=%d, clock_period_ps=%d, " - "worst_case_throughput=%d", - pipeline_stages.value_or(-1), clock_period_ps, - worst_case_throughput) - << "): " << sched.status(); - sched.IgnoreError(); - return sched.ok(); + return scheduler + .Schedule(pipeline_stages, clock_period_ps, failure_behavior, + /*check_feasibility=*/true, + /*worst_case_throughput=*/worst_case_throughput) + .ok(); }, BinarySearchAssumptions::kEndKnownTrue); VLOG(4) << "minimum worst-case throughput = " << min_worst_case_throughput; @@ -319,215 +335,52 @@ absl::StatusOr ApplyClockMargin(const SchedulingOptions& options, return clock_period_ps; } -// Handles the case where scheduling fails, providing more informative error -// messages based on the failure type and options. -absl::Status HandleScheduleFailure( - absl::Status schedule_status, const ScheduleGraph& graph, - const SchedulingOptions& options, - std::optional worst_case_throughput, - const DelayEstimator& io_delay_added, - std::optional& min_clock_period_ps_for_tracing, - int64_t clock_period_ps, Scheduler& bounds_scheduler) { - if (absl::IsInvalidArgument(schedule_status)) { - // The scheduler was able to explain the failure; report it up without - // further analysis. - return schedule_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. - - if (options.minimize_clock_on_failure().value_or(true)) { - // Find the smallest clock period that would have worked. - LOG(LEVEL(options.recover_after_minimizing_clock().value_or(false) - ? absl::LogSeverity::kWarning - : absl::LogSeverity::kError)) - << "Unable to schedule with the specified clock period (" - << clock_period_ps - << " ps); finding the shortest feasible clock period..."; - int64_t target_clock_period_ps = clock_period_ps + 1; - absl::StatusOr min_clock_period_ps = FindMinimumClockPeriod( - graph, options.pipeline_stages(), worst_case_throughput, - io_delay_added, bounds_scheduler, options.failure_behavior(), - target_clock_period_ps); - VLOG(5) << "FindMinimumClockPeriod() returned " << min_clock_period_ps; - if (min_clock_period_ps.ok()) { - min_clock_period_ps_for_tracing = *min_clock_period_ps; - // 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)); - } - if (absl::IsInvalidArgument(min_clock_period_ps.status())) { - // We failed with an explained error at the longest possible clock - // period. Report this error up, adding that the clock period will - // also need to be increased - though we don't know by how much. - 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, "); - } - // We fail with an unexplained error even at the longest possible - // clock period. Report the original error. - return schedule_status; - } - - // Check if just increasing the clock period would have helped. - XLS_ASSIGN_OR_RETURN(int64_t pessimistic_clock_period_ps, - ComputeCriticalPath(graph, io_delay_added)); - // Make a copy of failure behavior with explain_feasibility true- we - // always want to produce an error message because this we are - // re-running the scheduler for its error message. - SchedulingFailureBehavior pessimistic_failure_behavior = - options.failure_behavior(); - pessimistic_failure_behavior.explain_infeasibility = true; - absl::Status pessimistic_status = - bounds_scheduler - .Schedule(options.pipeline_stages(), pessimistic_clock_period_ps, - pessimistic_failure_behavior) - .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`."); +absl::Status GenerateHelpfulAsapError(absl::Status&& orig_status, + FunctionBase* f, int64_t clock_period_ps, + const DelayEstimator& delay_estimator, + const SchedulingOptions& options) { + xabsl::StatusBuilder status(std::move(orig_status)); + // Try to figure out what the actual required stages are. + if (options.pipeline_stages().has_value()) { + XLS_ASSIGN_OR_RETURN(absl::flat_hash_set dead_after_synthesis, + GetDeadAfterSynthesisNodes(f)); + XLS_ASSIGN_OR_RETURN(ScheduleGraph graph, + ScheduleGraph::Create(f, dead_after_synthesis)); + XLS_ASSIGN_OR_RETURN( + auto bounds, + sched::ScheduleBounds::Create(graph, clock_period_ps, delay_estimator, + f->GetInitiationInterval().value_or(1), + options.constraints()), + std::move(status) + << "(Failed to create schedule bounds for error message creation)"); + // Add first and last stage constraints. + using LastStageConstraint = + sched::ScheduleBounds::NodeSchedulingConstraint::LastStageConstraint; + for (Node* n : FirstStageNodes(f)) { + bounds.AddConstraint(NodeInCycleConstraint{n, 0}); } - if (absl::IsInvalidArgument(pessimistic_status)) { - // We failed with an explained error at the pessimistic clock period. - // Report this error up, adding that the clock period will also need - // to be increased - though we don't know by how much. - return xabsl::StatusBuilder(std::move(pessimistic_status)).SetPrepend() - << absl::StrFormat( - "cannot achieve the specified clock period; try " - "increasing `--clock_period_ps`. Also, "); + for (Node* n : FinalStageNodes(f)) { + bounds.AddConstraint(LastStageConstraint{n}); } - return pessimistic_status; - } - return schedule_status; -} - -absl::StatusOr RunIterativeSDCSchedule( - FunctionBase* f, const SchedulingOptions& options, int64_t clock_period_ps, - const DelayEstimator& delay_estimator, - const synthesis::Synthesizer* synthesizer) { - if (!options.clock_period_ps().has_value()) { - return absl::UnimplementedError( - "Iterative SDC scheduling is only supported when a clock period is " - "specified."); - } - - IterativeSDCSchedulingOptions isdc_options; - isdc_options.synthesizer = synthesizer; - isdc_options.iteration_number = options.fdo_iteration_number(); - isdc_options.delay_driven_path_number = - options.fdo_delay_driven_path_number(); - isdc_options.fanout_driven_path_number = - options.fdo_fanout_driven_path_number(); - isdc_options.stochastic_ratio = options.fdo_refinement_stochastic_ratio(); - isdc_options.path_evaluate_strategy = options.fdo_path_evaluate_strategy(); - - XLS_ASSIGN_OR_RETURN(DelayManager delay_manager, - DelayManager::Create(f, delay_estimator)); - XLS_ASSIGN_OR_RETURN( - ScheduleCycleMap cycle_map, - ScheduleByIterativeSDC(f, options.pipeline_stages(), clock_period_ps, - delay_manager, options.constraints(), isdc_options, - options.failure_behavior())); - - // Use delay manager for scheduling timing verification. - XLS_ASSIGN_OR_RETURN( - PipelineSchedule schedule, - PipelineSchedule::Create(f, cycle_map, options.pipeline_stages())); - XLS_RETURN_IF_ERROR(schedule.Verify()); - XLS_RETURN_IF_ERROR(schedule.VerifyTiming(clock_period_ps, delay_manager)); - XLS_RETURN_IF_ERROR(schedule.VerifyConstraints(options.constraints(), - f->GetInitiationInterval())); - - XLS_VLOG_LINES(3, "Schedule\n" + schedule.ToString()); - return schedule; -} - -// NB For backwards compatibility reasons the flag options for worst case -// throughput/II are rather confusing. Specifically, a value of 0 means that -// the scheduler is free to choose any WCT it wants and a value of nullopt -// means the value must be exactly 1. -// -// To make everything after this clearer we change this interpretation to have -// std::nullopt mean "no throughput requirements" and `Some(X)` mean a -// throughput requirement of X. Therefore a value of 0 from now on is invalid. -std::optional GetWorstCaseThroughputSetting( - const SchedulingOptions& options, FunctionBase* f) { - if (!f->IsProc()) { - VLOG(5) << "No II is possible because this is not a proc."; - return std::nullopt; - } - std::optional worst_case_throughput = - options.worst_case_throughput(); - if (worst_case_throughput && *worst_case_throughput == 0) { - VLOG(5) << "Setting II to unconstrained from options."; - worst_case_throughput = std::nullopt; - } else if (!worst_case_throughput) { - // No throughput requirement from options, See if the proc has one. - // - // NB This is only reachable if the scheduler is being configured from - // something other than scheduling_options_flags.cc and has complicated - // backwards-compat behaviors. - // - // In the future we are likely to move towards this being the primary way of - // configuring things however and we may want to reconsider how it is - // interpreted if/when that happens. - // - // TODO(allight): It may be worthwhile to go through and figure out what - // tests/other tools depend on this behavior to rationalize it all. - if (f->IsProc() && f->GetInitiationInterval()) { - // Now convert this to the new interpretation. - worst_case_throughput = f->GetInitiationInterval(); - if (worst_case_throughput == 0) { - VLOG(5) << "Setting II to unconstrained from proc setting."; - worst_case_throughput = std::nullopt; - } else { - VLOG(5) << "Setting II to " << *worst_case_throughput - << " from proc setting."; - } - } else if (options.minimize_worst_case_throughput()) { - VLOG(5) << "Setting II to unconstrained since no options or proc setting " - "and MinimizeWorstCaseThroughput is true."; - worst_case_throughput = std::nullopt; + absl::Status no_length = + TightenBounds(bounds, f, /*schedule_length=*/std::nullopt); + if (no_length.ok()) { + return (status << absl::StrFormat("Function %s cannot be scheduled in %d " + "stages. Computed minimum stage count " + "is %d.", + f->name(), + options.pipeline_stages().value(), + bounds.max_lower_bound() + 1)) + .SetCode(absl::StatusCode::kResourceExhausted); } else { - VLOG(5) << "Setting II to 1 since no options or proc setting and " - "MinimizeWorstCaseThroughput is false."; - worst_case_throughput = 1; + return (status << "Function " << f->name() + << " cannot be scheduled in any number of stages via ASAP " + "bounds. Some constraints may be unsatisfiable or " + "require a full SDC schedule to resolve.") + .SetCode(absl::StatusCode::kResourceExhausted); } - } else { - VLOG(5) << "Setting II to " << *worst_case_throughput - << " from worst_case_throughput option."; - } - return worst_case_throughput; -} - -// Set the WCT (either calculated or from options) to the function. For -// compatibility we need this to be in the same strange format as the option -// setting. Specifically where nullopt means a WCT of 1, and a value of 0 means -// unconstrained and all other values are WCT of the value. -// -// TODO(allight): Go through and update all users to the internal format where -// std::nullopt means unconstrained and any value is an exact WCT. -void SetWorstCaseThroughput(std::optional worst_case_throughput, - FunctionBase* f) { - if (!f->IsProc()) { - return; - } - if (worst_case_throughput.has_value()) { - if (worst_case_throughput == 1) { - f->AsProcOrDie()->ClearInitiationInterval(); - } else { - f->AsProcOrDie()->SetInitiationInterval(*worst_case_throughput); - } - } else { - f->AsProcOrDie()->SetInitiationInterval(0); } + return status; } absl::StatusOr RunPipelineScheduleInternal( @@ -551,22 +404,6 @@ absl::StatusOr RunPipelineScheduleInternal( "elaboration."); } - // NB For backwards compatibility reasons the flag options for worst case - // throughput/II are rather confusing. Specifically, a value of 0 means that - // the scheduler is free to choose any WCT it wants and a value of nullopt - // means the value must be exactly 1. - // - // To make everything after this clearer we change this interpretation to have - // std::nullopt mean "no throughput requirements" and `Some(X)` mean a - // throughput requirement of X. Therefore a value of 0 from now on is invalid. - std::optional worst_case_throughput = - GetWorstCaseThroughputSetting(options, f); - - VLOG(4) << "Computed WCT is " - << (worst_case_throughput.has_value() - ? absl::StrCat(*worst_case_throughput) - : "unconstrained"); - int64_t input_delay = options.additional_input_delay_ps().value_or(0); int64_t output_delay = options.additional_output_delay_ps().value_or(0); // Sends and receives each have inputs and outputs from the flow control @@ -613,9 +450,8 @@ absl::StatusOr RunPipelineScheduleInternal( return base_delay; }); - if (worst_case_throughput == 0) { - VLOG(5) << "Worst case throughput explicitly set to 0 (unbounded)."; - worst_case_throughput = std::nullopt; + if (options.worst_case_throughput().has_value()) { + f->SetInitiationInterval(*options.worst_case_throughput()); } if (options.pipeline_stages() == 1 && @@ -641,74 +477,13 @@ absl::StatusOr RunPipelineScheduleInternal( return schedule; } - XLS_ASSIGN_OR_RETURN(absl::flat_hash_set dead_after_synthesis, - GetDeadAfterSynthesisNodes(f)); - XLS_ASSIGN_OR_RETURN(ScheduleGraph graph, - ScheduleGraph::Create(f, dead_after_synthesis)); - - // std::unique_ptr sdc_scheduler; - std::unique_ptr scheduler; - auto create_scheduler = - [&](SchedulingStrategy strategy, - std::optional dynamic_throughput_objective_weight, - bool check_feasibility) - -> absl::StatusOr> { - std::unique_ptr s; - switch (strategy) { - case SchedulingStrategy::ASAP: { - s = std::make_unique(graph, io_delay_added); - break; - } - case SchedulingStrategy::MIN_CUT: { - s = std::make_unique(graph, io_delay_added); - break; - } - case SchedulingStrategy::SDC: { - XLS_ASSIGN_OR_RETURN( - std::unique_ptr sdc_scheduler, - SDCScheduler::Create(graph, io_delay_added, options)); - sdc_scheduler->SetCheckFeasibility(check_feasibility); - sdc_scheduler->SetDynamicThroughputObjectiveWeight( - dynamic_throughput_objective_weight); - s = std::move(sdc_scheduler); - break; - } - case SchedulingStrategy::RANDOM: { - s = std::make_unique( - graph, io_delay_added, - absl::BitGen(std::seed_seq{options.seed().value_or(0)})); - break; - } - } - VLOG(5) << "Constraints are : [" - << absl::StrJoin(options.constraints(), ",") << "]"; - XLS_RETURN_IF_ERROR(s->AddConstraints(options.constraints())); - return std::move(s); - }; - auto initialize_scheduler = [&]() -> absl::Status { - if (scheduler == nullptr) { - XLS_ASSIGN_OR_RETURN( - scheduler, - create_scheduler(options.strategy(), - options.dynamic_throughput_objective_weight(), - /*check_feasibility=*/false)); - } - VLOG(5) << "Using scheduler " << scheduler->name() - << " for primary scheduling."; - return absl::OkStatus(); - }; - std::unique_ptr bounds_scheduler; - auto initialize_bounds_scheduler = [&]() -> absl::Status { - if (bounds_scheduler == nullptr) { - // Even if we are SDC we ignore the dynamic_throughput_objective_weight - // for bounds. - XLS_ASSIGN_OR_RETURN( - bounds_scheduler, - create_scheduler(options.find_bounds_strategy(), - /*dynamic_throughput_objective_weight=*/std::nullopt, - /*check_feasibility=*/true)); + std::unique_ptr sdc_scheduler; + auto initialize_sdc_scheduler = [&]() -> absl::Status { + if (sdc_scheduler == nullptr) { + XLS_ASSIGN_OR_RETURN(sdc_scheduler, + SDCScheduler::Create(f, io_delay_added, options)); + XLS_RETURN_IF_ERROR(sdc_scheduler->AddConstraints(options.constraints())); } - VLOG(5) << "Using scheduler " << bounds_scheduler->name() << " for bounds."; return absl::OkStatus(); }; @@ -724,20 +499,17 @@ absl::StatusOr RunPipelineScheduleInternal( // user-specified relaxation percent if needed), and use that if it's // smaller than the target clock period. // - // TODO(allight): Ideally we'd always use the ASAP scheduler but it is - // possible to create graphs that are not ASAP schedulable. For now just - // use the configured bounds strategy. - VLOG(5) << "Finding min clock due to " - << (!options.clock_period_ps().has_value() - ? "no target clock period" - : "minimize_clock_on_failure and recover_after_minimizing_" - "clock"); - XLS_RETURN_IF_ERROR(initialize_bounds_scheduler()); + // 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 + // schedule. + XLS_RETURN_IF_ERROR(initialize_sdc_scheduler()); XLS_ASSIGN_OR_RETURN( min_clock_period_ps_for_tracing, - FindMinimumClockPeriod(graph, options.pipeline_stages(), - worst_case_throughput, io_delay_added, - *bounds_scheduler, options.failure_behavior())); + FindMinimumClockPeriod( + f, options.pipeline_stages(), + /*worst_case_throughput=*/f->IsProc() ? f->GetInitiationInterval() + : std::nullopt, + io_delay_added, *sdc_scheduler, options.failure_behavior())); // Pad the minimum clock period to account for the clock margin. if (options.clock_margin_percent().has_value()) { @@ -778,34 +550,30 @@ absl::StatusOr RunPipelineScheduleInternal( << " ps; continuing with clock period = " << clock_period_ps << " ps."; } - VLOG(5) << "Got clock period of " << clock_period_ps << " from minimum."; } else { - VLOG(5) << "Got clock period of " << options.clock_period_ps().value() - << " from options."; clock_period_ps = options.clock_period_ps().value(); } XLS_ASSIGN_OR_RETURN(clock_period_ps, ApplyClockMargin(options, clock_period_ps)); - // Only actually attempt to minimize WCT if we are (1) told we can do so, - // (2) we don't have an explicitly chosen WCT already, and (3) we have a - // constraint that actually cares about WCT. - // TODO(allight): Does it actually make sense to ever not have a back-edge - // constraint with a WCT setting? - if (options.minimize_worst_case_throughput() && f->IsProc() && - !worst_case_throughput && + std::optional worst_case_throughput = std::nullopt; + if (options.minimize_worst_case_throughput().value_or(false) && f->IsProc() && + f->GetInitiationInterval().value_or(1) <= 0 && absl::c_any_of( options.constraints(), [](const SchedulingConstraint& constraint) { return std::holds_alternative(constraint); })) { - XLS_RETURN_IF_ERROR(initialize_bounds_scheduler()); + // NOTE: We currently use the SDC scheduler to minimize the worst-case + // throughput (if minimization is requested), even if we're not using + // it for the final schedule. + XLS_RETURN_IF_ERROR(initialize_sdc_scheduler()); absl::StatusOr wct = FindMinimumWorstCaseThroughput( - f->AsProcOrDie(), options.pipeline_stages(), clock_period_ps, - *bounds_scheduler, + f, options.pipeline_stages(), clock_period_ps, *sdc_scheduler, /*failure_behavior=*/{.explain_infeasibility = false}); if (wct.ok()) { worst_case_throughput = *wct; + f->AsProcOrDie()->SetInitiationInterval(*wct); LOG(INFO) << "Minimized worst-case throughput for proc '" << f->name() << "': " << *worst_case_throughput; } else { @@ -815,59 +583,222 @@ absl::StatusOr RunPipelineScheduleInternal( } } - // Worst case throughput is either known from the options, the proc itself, or - // minimization by now. Notate it in the proc itself. - // - // Using a helper since other parts of codegen/sched/other tools use the same - // nullopt interpretation as the scheduling_options flag. See comment on - // function for more information. - SetWorstCaseThroughput(worst_case_throughput, f); - - // TODO(allight): Rewrite FDO into the scheduler API. - if (options.use_fdo() && options.strategy() == SchedulingStrategy::SDC) { - if (f->IsProc()) { - XLS_RET_CHECK_EQ(f->AsProcOrDie()->GetInitiationInterval().has_value(), - worst_case_throughput.has_value()) - << " ii: " << f->AsProcOrDie()->GetInitiationInterval().value_or(-1) - << " wct: " << worst_case_throughput.value_or(-1); - if (worst_case_throughput) { - XLS_RET_CHECK_EQ(*f->AsProcOrDie()->GetInitiationInterval(), - *worst_case_throughput); + ScheduleCycleMap cycle_map; + if (options.strategy() == SchedulingStrategy::SDC) { + // Enable iterative SDC scheduling when use_fdo is true + if (options.use_fdo()) { + if (!options.clock_period_ps().has_value()) { + return absl::UnimplementedError( + "Iterative SDC scheduling is only supported when a clock period is " + "specified."); + } + + IterativeSDCSchedulingOptions isdc_options; + isdc_options.synthesizer = synthesizer; + isdc_options.iteration_number = options.fdo_iteration_number(); + isdc_options.delay_driven_path_number = + options.fdo_delay_driven_path_number(); + isdc_options.fanout_driven_path_number = + options.fdo_fanout_driven_path_number(); + isdc_options.stochastic_ratio = options.fdo_refinement_stochastic_ratio(); + isdc_options.path_evaluate_strategy = + options.fdo_path_evaluate_strategy(); + + XLS_ASSIGN_OR_RETURN(DelayManager delay_manager, + DelayManager::Create(f, delay_estimator)); + XLS_ASSIGN_OR_RETURN( + cycle_map, + ScheduleByIterativeSDC(f, options.pipeline_stages(), clock_period_ps, + delay_manager, options.constraints(), + isdc_options, options.failure_behavior())); + + // Use delay manager for scheduling timing verification. + XLS_ASSIGN_OR_RETURN( + PipelineSchedule schedule, + PipelineSchedule::Create(f, cycle_map, options.pipeline_stages())); + XLS_RETURN_IF_ERROR(schedule.Verify()); + XLS_RETURN_IF_ERROR( + schedule.VerifyTiming(clock_period_ps, delay_manager)); + XLS_RETURN_IF_ERROR(schedule.VerifyConstraints( + options.constraints(), f->GetInitiationInterval())); + + XLS_VLOG_LINES(3, "Schedule\n" + schedule.ToString()); + return schedule; + } + + XLS_RETURN_IF_ERROR(initialize_sdc_scheduler()); + absl::StatusOr schedule_cycle_map = + sdc_scheduler->Schedule(options.pipeline_stages(), clock_period_ps, + options.failure_behavior(), + /*check_feasibility=*/false, + worst_case_throughput, + options.dynamic_throughput_objective_weight()); + 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. + + if (options.minimize_clock_on_failure().value_or(true)) { + // Find the smallest clock period that would have worked. + LOG(LEVEL(options.recover_after_minimizing_clock().value_or(false) + ? absl::LogSeverity::kWarning + : absl::LogSeverity::kError)) + << "Unable to schedule with the specified clock period (" + << clock_period_ps + << " ps); finding the shortest feasible clock period..."; + int64_t target_clock_period_ps = clock_period_ps + 1; + XLS_RETURN_IF_ERROR(initialize_sdc_scheduler()); + absl::StatusOr min_clock_period_ps = FindMinimumClockPeriod( + f, options.pipeline_stages(), worst_case_throughput, + io_delay_added, *sdc_scheduler, options.failure_behavior(), + target_clock_period_ps); + if (min_clock_period_ps.ok()) { + min_clock_period_ps_for_tracing = *min_clock_period_ps; + // 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)); + } + if (absl::IsInvalidArgument(min_clock_period_ps.status())) { + // We failed with an explained error at the longest possible clock + // period. Report this error up, adding that the clock period will + // also need to be increased - though we don't know by how much. + 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, "); + } + // We fail with an unexplained error even at the longest possible + // clock period. Report the original error. + return std::move(schedule_cycle_map).status(); + } + + // Check if just increasing the clock period would have helped. + XLS_ASSIGN_OR_RETURN(int64_t pessimistic_clock_period_ps, + ComputeCriticalPath(f, io_delay_added)); + // Make a copy of failure behavior with explain_feasibility true- we + // always want to produce an error message because this we are + // re-running the scheduler for its error message. + SchedulingFailureBehavior pessimistic_failure_behavior = + options.failure_behavior(); + pessimistic_failure_behavior.explain_infeasibility = true; + XLS_RETURN_IF_ERROR(initialize_sdc_scheduler()); + absl::Status pessimistic_status = + sdc_scheduler + ->Schedule(options.pipeline_stages(), + pessimistic_clock_period_ps, + pessimistic_failure_behavior, + /*check_feasibility=*/true) + .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`."); + } + if (absl::IsInvalidArgument(pessimistic_status)) { + // We failed with an explained error at the pessimistic clock period. + // Report this error up, adding that the clock period will also need + // to be increased - though we don't know by how much. + return xabsl::StatusBuilder(std::move(pessimistic_status)) + .SetPrepend() + << absl::StrFormat( + "cannot achieve the specified clock period; try " + "increasing `--clock_period_ps`. Also, "); + } + return pessimistic_status; + } + return schedule_cycle_map.status(); + } + cycle_map = *std::move(schedule_cycle_map); + } else { + // Run an initial ASAP/ALAP scheduling pass, which we'll refine with the + // chosen scheduler. + // First get the basic bounds. + XLS_ASSIGN_OR_RETURN(absl::flat_hash_set dead_after_synthesis, + GetDeadAfterSynthesisNodes(f)); + XLS_ASSIGN_OR_RETURN(ScheduleGraph graph, + ScheduleGraph::Create(f, dead_after_synthesis)); + XLS_ASSIGN_OR_RETURN( + auto bounds, + sched::ScheduleBounds::Create(graph, clock_period_ps, io_delay_added, + f->GetInitiationInterval().value_or(1), + options.constraints())); + // Add first and last stage constraints. + using LastStageConstraint = + sched::ScheduleBounds::NodeSchedulingConstraint::LastStageConstraint; + for (Node* n : FirstStageNodes(f)) { + bounds.AddConstraint(NodeInCycleConstraint{n, 0}); + } + for (Node* n : FinalStageNodes(f)) { + bounds.AddConstraint(LastStageConstraint{n}); + } + absl::Status tighten_bounds_status = + TightenBounds(bounds, f, options.pipeline_stages()); + if (!tighten_bounds_status.ok()) { + return GenerateHelpfulAsapError(std::move(tighten_bounds_status), f, + clock_period_ps, io_delay_added, options); + } + + if (options.strategy() == SchedulingStrategy::MIN_CUT) { + XLS_ASSIGN_OR_RETURN( + cycle_map, + MinCutScheduler( + f, + options.pipeline_stages().value_or(bounds.max_lower_bound() + 1), + clock_period_ps, io_delay_added, &bounds, options.constraints())); + } else if (options.strategy() == SchedulingStrategy::RANDOM) { + std::mt19937_64 gen(options.seed().value_or(0)); + + cycle_map = ScheduleCycleMap(); + XLS_ASSIGN_OR_RETURN(std::vector topo_sort_nodes, TopoSort(f)); + for (Node* node : topo_sort_nodes) { + if (IsUntimed(node)) { + continue; + } + int64_t cycle = absl::Uniform( + absl::IntervalClosed, gen, bounds.lb(node), bounds.ub(node)); + if (bounds.lb(node) != bounds.ub(node)) { + XLS_RETURN_IF_ERROR(bounds.TightenNodeLb(node, cycle)); + XLS_RETURN_IF_ERROR(bounds.TightenNodeUb(node, cycle)); + // TODO(allight): We might want to give this more fuel. + XLS_RETURN_IF_ERROR(bounds.PropagateBounds()); + } + cycle_map[node] = cycle; + } + } else { + XLS_RET_CHECK(options.strategy() == SchedulingStrategy::ASAP); + XLS_RET_CHECK(!options.pipeline_stages().has_value() || + options.pipeline_stages() == 1 || + options.pipeline_stages() >= bounds.max_lower_bound() + 1) + << "Pipeline stages must be at least 1 or greater than or equal to " + "the number of stages in the function. pipeline_stages: " + << (options.pipeline_stages() ? *options.pipeline_stages() : -1) + << " max_lower_bound: " << bounds.max_lower_bound(); + // Just schedule everything as soon as possible. + for (Node* node : f->nodes()) { + cycle_map[node] = bounds.lb(node); } } - return RunIterativeSDCSchedule(f, options, clock_period_ps, delay_estimator, - synthesizer); - } else if (options.use_fdo()) { - return absl::InvalidArgumentError( - "FDO is only supported with SDC strategy."); - } - VLOG(5) << "Starting primary scheduling. Failure behavior is: " - << options.failure_behavior().ToProto().ShortDebugString(); - ScheduleCycleMap cycle_map; - XLS_RETURN_IF_ERROR(initialize_scheduler()); - absl::StatusOr schedule_cycle_map = - scheduler->Schedule(options.pipeline_stages(), clock_period_ps, - options.failure_behavior(), worst_case_throughput); - - if (!schedule_cycle_map.ok()) { - XLS_RETURN_IF_ERROR(initialize_bounds_scheduler()); - return HandleScheduleFailure(std::move(schedule_cycle_map).status(), graph, - options, worst_case_throughput, io_delay_added, - min_clock_period_ps_for_tracing, - clock_period_ps, *bounds_scheduler); } - cycle_map = *std::move(schedule_cycle_map); XLS_ASSIGN_OR_RETURN( PipelineSchedule schedule, PipelineSchedule::Create(f, cycle_map, options.pipeline_stages(), min_clock_period_ps_for_tracing)); - XLS_VLOG_LINES(3, "Schedule\n" + schedule.ToString()); XLS_RETURN_IF_ERROR(schedule.Verify()); XLS_RETURN_IF_ERROR(schedule.VerifyTiming(clock_period_ps, io_delay_added)); XLS_RETURN_IF_ERROR(schedule.VerifyConstraints(options.constraints(), f->GetInitiationInterval())); + XLS_VLOG_LINES(3, "Schedule\n" + schedule.ToString()); return schedule; } diff --git a/xls/scheduling/schedule_bounds.cc b/xls/scheduling/schedule_bounds.cc index 7826ebd128..84f1334cbf 100644 --- a/xls/scheduling/schedule_bounds.cc +++ b/xls/scheduling/schedule_bounds.cc @@ -832,11 +832,6 @@ absl::Status ScheduleBounds::CheckBasicBounds() { absl::Status ScheduleBounds::CheckConstraints(std::optional fuel, bool stabilized) { auto main_result = [&]() -> absl::Status { - if (bounds_.empty()) { - // The schedule has no nodes in it. Either everything is dead or this is a - // trivial function. - return absl::OkStatus(); - } int64_t before_max_last_stage_num = absl::c_min_element(bounds_, [](const auto& a, const auto& b) { return a.second.before_max < b.second.before_max; diff --git a/xls/scheduling/scheduler.h b/xls/scheduling/scheduler.h deleted file mode 100644 index e7313930ab..0000000000 --- a/xls/scheduling/scheduler.h +++ /dev/null @@ -1,73 +0,0 @@ -// Copyright 2026 The XLS Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef XLS_SCHEDULING_SCHEDULER_H_ -#define XLS_SCHEDULING_SCHEDULER_H_ - -#include -#include -#include -#include - -#include "absl/status/status.h" -#include "absl/status/statusor.h" -#include "absl/types/span.h" -#include "xls/scheduling/schedule_bounds.h" -#include "xls/scheduling/scheduling_options.h" - -namespace xls { - -// An abstract base class for a scheduler. -class Scheduler { - public: - virtual ~Scheduler() = default; - explicit Scheduler(std::string name) : name_(name) {} - - std::string_view name() const { return name_; } - - virtual absl::Status AddConstraints( - absl::Span constraints) = 0; - - // Schedule the graph using the given constraints and this schedulers - // strategy. Most schedulers try to either find or at least approximate the - // solution to an optimization problem (eg minimize registers etc) but this - // function doesn't actually require anything other than the schedule conforms - // to the constraints passed both to it and the graph itself. - // - // If `pipeline_stages` is not specified, the solver will use the smallest - // feasible value. - // - // If the problem is infeasible, `failure_behavior` configures what will be - // done. If configured to do so, the scheduler will reformulate the problem - // with slack variables and give actionable feedback on how to update the - // design to be feasible to schedule. - // - // TODO(allight): Currently the tests for the scheduler expect that error - // messages fit some pretty specific patterns which match what SDC does and - // are not documented anywhere. We probably want to rationalize that all at - // some point. For now we just implement some of the expected values for ASAP. - // As all of these are failure paths even if a scheduler does not match the - // messages everything will still mostly work. - virtual absl::StatusOr Schedule( - std::optional pipeline_stages, int64_t clock_period_ps, - SchedulingFailureBehavior failure_behavior, - std::optional worst_case_throughput = std::nullopt) = 0; - - private: - std::string name_; -}; - -} // namespace xls - -#endif // XLS_SCHEDULING_SCHEDULER_H_ diff --git a/xls/scheduling/scheduling_options.cc b/xls/scheduling/scheduling_options.cc index 74468623a3..b20b26bc92 100644 --- a/xls/scheduling/scheduling_options.cc +++ b/xls/scheduling/scheduling_options.cc @@ -16,7 +16,6 @@ #include #include -#include #include #include #include @@ -119,12 +118,6 @@ absl::StatusOr OptionsFromFlagProto( scheduling_options.strategy( FromProtoSchedulingStrategy(proto.scheduling_strategy())); } - if (proto.has_find_bounds_strategy() && - proto.find_bounds_strategy() != - ProtoSchedulingStrategy::SCHEDULER_TYPE_UNSPECIFIED) { - scheduling_options.find_bounds_strategy( - FromProtoSchedulingStrategy(proto.find_bounds_strategy())); - } if (proto.has_opt_level()) { scheduling_options.opt_level(proto.opt_level()); } @@ -385,11 +378,6 @@ absl::StatusOr OptionsFromFlagProto( } // namespace -std::ostream& operator<<(std::ostream& os, SchedulingStrategy strategy) { - return os << ProtoSchedulingStrategy_Name( - ToProtoSchedulingStrategy(strategy)); -} - absl::StatusOr SetUpDelayEstimator( const SchedulingOptionsFlagsProto& flags) { return GetDelayEstimator(flags.delay_model()); diff --git a/xls/scheduling/scheduling_options.h b/xls/scheduling/scheduling_options.h index 2c5b81f46c..b33236ff1f 100644 --- a/xls/scheduling/scheduling_options.h +++ b/xls/scheduling/scheduling_options.h @@ -34,7 +34,6 @@ #include "xls/common/visitor.h" #include "xls/estimators/delay_model/delay_estimator.h" #include "xls/ir/channel.h" -#include "xls/ir/ir_annotator.h" #include "xls/ir/node.h" #include "xls/ir/package.h" #include "xls/passes/optimization_pass.h" @@ -70,11 +69,6 @@ SchedulingStrategy FromProtoSchedulingStrategy( bool AbslParseFlag(std::string_view text, SchedulingStrategy* strategy, std::string* error); std::string AbslUnparseFlag(const SchedulingStrategy& strategy); -std::ostream& operator<<(std::ostream& os, SchedulingStrategy strategy); -template -void AbslStringify(Sink& sink, SchedulingStrategy strategy) { - absl::Format(&sink, "%s", AbslUnparseFlag(strategy)); -} enum class PathEvaluateStrategy : int8_t { PATH, @@ -364,8 +358,6 @@ struct SchedulingFailureBehavior { // If scheduling fails, re-run scheduling with extra slack variables in an // attempt to explain why scheduling failed. - // - // Only used for SDC scheduler. bool explain_infeasibility = true; // If specified, the specified value must be > 0. Setting this configures how @@ -378,8 +370,6 @@ struct SchedulingFailureBehavior { // value should give more specific information about how much slack each // failing backedge needs at the cost of less actionable and harder to // understand output. - // - // Only used for SDC scheduler. std::optional infeasible_per_state_backedge_slack_pool; }; @@ -389,10 +379,8 @@ struct SchedulingFailureBehavior { class SchedulingOptions { public: explicit SchedulingOptions( - SchedulingStrategy strategy = SchedulingStrategy::SDC, - std::optional find_bounds_strategy = std::nullopt) + SchedulingStrategy strategy = SchedulingStrategy::SDC) : strategy_(strategy), - find_bounds_strategy_(find_bounds_strategy.value_or(strategy_)), opt_level_(kMaxOptLevel), minimize_clock_on_failure_(true), recover_after_minimizing_clock_(false), @@ -422,15 +410,6 @@ class SchedulingOptions { return *this; } - // Returns the find_bounds_strategy. - SchedulingOptions& find_bounds_strategy(SchedulingStrategy strategy) { - find_bounds_strategy_ = strategy; - return *this; - } - SchedulingStrategy find_bounds_strategy() const { - return find_bounds_strategy_; - } - // Sets/gets the target delay model SchedulingOptions& opt_level(int64_t value) { opt_level_ = value; @@ -507,7 +486,7 @@ class SchedulingOptions { minimize_worst_case_throughput_ = value; return *this; } - bool minimize_worst_case_throughput() const { + std::optional minimize_worst_case_throughput() const { return minimize_worst_case_throughput_; } @@ -779,9 +758,6 @@ class SchedulingOptions { private: SchedulingStrategy strategy_; - // Strategy used to find minimum clock-period and WCT bounds. This should - // usually be ASAP but can be SDC if needed. - SchedulingStrategy find_bounds_strategy_; int64_t opt_level_; std::optional clock_period_ps_; std::optional delay_model_; @@ -823,17 +799,6 @@ class SchedulingOptions { // A map from node to cycle as a bare-bones representation of a schedule. using ScheduleCycleMap = absl::flat_hash_map; -class ScheduleCycleAnnotator final : public IrAnnotator { - public: - ScheduleCycleAnnotator(const ScheduleCycleMap& cycle_map) - : cycle_map_(cycle_map) {} - Annotation NodeAnnotation(Node* node) const override { - return Annotation{.suffix = absl::StrCat(" [", cycle_map_.at(node), "]")}; - } - - private: - const ScheduleCycleMap& cycle_map_; -}; absl::StatusOr SetUpSchedulingOptions( const SchedulingOptionsFlagsProto& flags, const Package* p); diff --git a/xls/scheduling/sdc_scheduler.cc b/xls/scheduling/sdc_scheduler.cc index 0d13033538..e580d2ed7f 100644 --- a/xls/scheduling/sdc_scheduler.cc +++ b/xls/scheduling/sdc_scheduler.cc @@ -53,7 +53,6 @@ #include "xls/scheduling/schedule_bounds.h" #include "xls/scheduling/schedule_graph.h" #include "xls/scheduling/schedule_util.h" -#include "xls/scheduling/scheduler.h" #include "xls/scheduling/scheduling_options.h" #include "ortools/math_opt/cpp/math_opt.h" @@ -205,9 +204,9 @@ ComputeCombinationalDelayConstraints( } // namespace SDCSchedulingModel::SDCSchedulingModel( - const ScheduleGraph& graph, const DelayMap& delay_map, + ScheduleGraph graph, const DelayMap& delay_map, std::optional initiation_interval, double sdc_solution_tolerance) - : graph_(graph), + : graph_(std::move(graph)), model_(absl::StrCat("sdc_model:", graph_.name())), sdc_solution_tolerance_(sdc_solution_tolerance), delay_map_(delay_map), @@ -406,13 +405,7 @@ absl::Status SDCSchedulingModel::AddThroughputConstraint(StateRead* state_read, // necessary while enforcing a target II. absl::Status SDCSchedulingModel::AddBackedgeConstraints( const BackedgeConstraint& constraint) { - if (!initiation_interval_.has_value()) { - // Distance of backedge is constrained by the II, but the worst-case - // throughput constraint is not set. - return absl::OkStatus(); - } - - const int64_t II = *initiation_interval_; + const int64_t II = initiation_interval_.value_or(1); for (const ScheduleBackedge& backedge : graph_.backedges()) { if (!backedge.distance.has_value() || @@ -421,6 +414,11 @@ absl::Status SDCSchedulingModel::AddBackedgeConstraints( return absl::UnimplementedError( "Unsupported backedge type in SDC schedule"); } + if (II <= 0) { + // Distance of backedge is constrained by the II, but the worst-case + // throughput constraint is not set. + continue; + } VLOG(2) << "Setting backedge constraint (II): " << absl::StrFormat("cycle[%s] - cycle[%s] < %d", @@ -896,16 +894,13 @@ void SDCSchedulingModel::SetClockPeriod(int64_t clock_period_ps) { } absl::Status SDCSchedulingModel::SetWorstCaseThroughput( - std::optional worst_case_throughput) { - XLS_RET_CHECK(worst_case_throughput != 0) - << "WCT Of zero should be represented as std::nullopt (no throughput " - "requirements)"; + int64_t worst_case_throughput) { if (!graph_.IsSingleProc()) { return absl::UnimplementedError( "SetWorstCaseThroughput only supports procs, since it controls state " "backedges"); } - if (initiation_interval_ == worst_case_throughput) { + if (initiation_interval_.value_or(1) == worst_case_throughput) { return absl::OkStatus(); } initiation_interval_ = worst_case_throughput; @@ -913,10 +908,7 @@ absl::Status SDCSchedulingModel::SetWorstCaseThroughput( model_.DeleteLinearConstraint(constraint); } backedge_constraint_.clear(); - if (initiation_interval_) { - return AddBackedgeConstraints(BackedgeConstraint()); - } - return absl::OkStatus(); + return AddBackedgeConstraints(BackedgeConstraint()); } void SDCSchedulingModel::SetPipelineLength( @@ -1245,27 +1237,56 @@ SDCSchedulingModel::AddLowerBoundSlack( } absl::StatusOr> SDCScheduler::Create( - const ScheduleGraph& graph, const DelayEstimator& delay_estimator, + FunctionBase* f, const DelayEstimator& delay_estimator, + const SchedulingOptions& options) { + XLS_ASSIGN_OR_RETURN(absl::flat_hash_set dead_after_synthesis, + GetDeadAfterSynthesisNodes(f)); + VLOG(4) << "dead_after_synthesis: (size: " << dead_after_synthesis.size() + << ") [" + << absl::StrJoin(dead_after_synthesis, ", ", + [](std::string* out, Node* node) { + absl::StrAppendFormat(out, "%s(%s)", + node->GetName(), + OpToString(node->op())); + }) + << "]"; + XLS_ASSIGN_OR_RETURN(ScheduleGraph graph, + ScheduleGraph::Create(f, dead_after_synthesis)); + XLS_ASSIGN_OR_RETURN(DelayMap delay_map, + ComputeNodeDelays(graph, delay_estimator)); + std::optional initiation_interval = + f->IsProc() ? std::optional( + f->AsProcOrDie()->GetInitiationInterval().value_or(1)) + : std::nullopt; + std::unique_ptr scheduler(new SDCScheduler( + std::move(graph), options.sdc_solution_tolerance(), options.solver_type(), + options.solve_parameters(), initiation_interval, std::move(delay_map))); + XLS_RETURN_IF_ERROR(scheduler->Initialize()); + return std::move(scheduler); +} + +absl::StatusOr> SDCScheduler::Create( + ScheduleGraph graph, const DelayEstimator& delay_estimator, const SchedulingOptions& options) { XLS_ASSIGN_OR_RETURN(DelayMap delay_map, ComputeNodeDelays(graph, delay_estimator)); std::unique_ptr scheduler(new SDCScheduler( - graph, options.sdc_solution_tolerance(), options.solver_type(), + std::move(graph), options.sdc_solution_tolerance(), options.solver_type(), options.solve_parameters(), std::nullopt, std::move(delay_map))); XLS_RETURN_IF_ERROR(scheduler->Initialize()); return std::move(scheduler); } SDCScheduler::SDCScheduler( - const ScheduleGraph& graph, double sdc_solution_tolerance, + ScheduleGraph graph, double sdc_solution_tolerance, ::operations_research::math_opt::SolverType solver_type, ::operations_research::math_opt::SolveParameters&& solve_parameters, std::optional initiation_interval, DelayMap&& delay_map) - : Scheduler("SDCScheduler"), - delay_map_(std::move(delay_map)), + : delay_map_(std::move(delay_map)), solver_type_(solver_type), solve_parameters_(std::move(solve_parameters)), - model_(graph, delay_map_, initiation_interval, sdc_solution_tolerance) {} + model_(std::move(graph), delay_map_, initiation_interval, + sdc_solution_tolerance) {} absl::Status SDCScheduler::Initialize() { XLS_ASSIGN_OR_RETURN(solver_, math_opt::NewIncrementalSolver( @@ -1316,30 +1337,19 @@ absl::Status SDCScheduler::BuildError( absl::StatusOr SDCScheduler::Schedule( std::optional pipeline_stages, int64_t clock_period_ps, - SchedulingFailureBehavior failure_behavior, - std::optional worst_case_throughput) { - VLOG(5) << "Running scheduling with " - << (pipeline_stages.has_value() ? absl::StrCat(*pipeline_stages) - : "unspecified") - << " pipeline stages, " - << (worst_case_throughput.has_value() - ? absl::StrCat(*worst_case_throughput) - : "unspecified") - << " wct, and " << clock_period_ps << " clock period."; - VLOG(5) << " Configured with: " - << (check_feasibility_ ? "check feasibility" - : "minimize dynamic throughput"); + SchedulingFailureBehavior failure_behavior, bool check_feasibility, + std::optional worst_case_throughput, + std::optional dynamic_throughput_objective_weight) { model_.SetClockPeriod(clock_period_ps); - // TODO(allight): Having the II be sort of held in the model is a footgun - // since it can be not clear what the II being targeted is. For now just force - // it to the current value every time we schedule. - if (model_.initiation_interval() != worst_case_throughput) { - XLS_RETURN_IF_ERROR(model_.SetWorstCaseThroughput(worst_case_throughput)) - << "Failed to set WCT of " << worst_case_throughput.value_or(-1); + if (worst_case_throughput.has_value()) { + if (model_.initiation_interval().value_or(1) != *worst_case_throughput) { + XLS_RETURN_IF_ERROR( + model_.SetWorstCaseThroughput(*worst_case_throughput)); + } } model_.SetPipelineLength(pipeline_stages); - if (!pipeline_stages.has_value() && !check_feasibility_) { + if (!pipeline_stages.has_value() && !check_feasibility) { // Find the minimum feasible pipeline length. model_.MinimizePipelineLength(); XLS_ASSIGN_OR_RETURN( @@ -1357,16 +1367,16 @@ absl::StatusOr SDCScheduler::Schedule( model_.SetPipelineLength(min_pipeline_length); } - if (check_feasibility_) { + if (check_feasibility) { model_.RemoveObjective(); } else { - model_.SetObjective(dynamic_throughput_objective_weight_); + model_.SetObjective(dynamic_throughput_objective_weight); } XLS_ASSIGN_OR_RETURN(math_opt::SolveResult result, solver_->Solve({.parameters = solve_parameters_})); if (result.termination.reason == math_opt::TerminationReason::kOptimal || - (check_feasibility_ && + (check_feasibility && result.termination.reason == math_opt::TerminationReason::kFeasible)) { return model_.ExtractResult(result.variable_values()); } diff --git a/xls/scheduling/sdc_scheduler.h b/xls/scheduling/sdc_scheduler.h index 8f667f7913..c3e06f105e 100644 --- a/xls/scheduling/sdc_scheduler.h +++ b/xls/scheduling/sdc_scheduler.h @@ -32,7 +32,6 @@ #include "xls/ir/node.h" #include "xls/ir/nodes.h" #include "xls/scheduling/schedule_graph.h" -#include "xls/scheduling/scheduler.h" #include "xls/scheduling/scheduling_options.h" #include "ortools/math_opt/cpp/math_opt.h" @@ -49,7 +48,7 @@ class SDCSchedulingModel { static constexpr double kMaxStages = (1 << 20); public: - SDCSchedulingModel(const ScheduleGraph& graph, const DelayMap& delay_map, + SDCSchedulingModel(ScheduleGraph graph, const DelayMap& delay_map, std::optional initiation_interval, double sdc_solution_tolerance); @@ -73,8 +72,7 @@ class SDCSchedulingModel { void SetClockPeriod(int64_t clock_period_ps); - absl::Status SetWorstCaseThroughput( - std::optional worst_case_throughput); + absl::Status SetWorstCaseThroughput(int64_t worst_case_throughput); std::optional initiation_interval() const { return initiation_interval_; } @@ -135,8 +133,6 @@ class SDCSchedulingModel { operations_research::math_opt::LinearConstraint DiffEqualsConstraint( Node* x, Node* y, int64_t diff, std::string_view name); - const ScheduleGraph& graph() const { return graph_; } - private: operations_research::math_opt::Variable AddUpperBoundSlack( operations_research::math_opt::LinearConstraint c, @@ -172,7 +168,7 @@ class SDCSchedulingModel { absl::Status ScheduleDeadAfterSynthesisNodes( ScheduleCycleMap& cycle_map) const; - const ScheduleGraph& graph_; + ScheduleGraph graph_; operations_research::math_opt::Model model_; double sdc_solution_tolerance_; @@ -239,16 +235,20 @@ class SDCSchedulingModel { absl::flat_hash_map io_slack_; }; -class SDCScheduler final : public Scheduler { +class SDCScheduler { using DelayMap = absl::flat_hash_map; public: static absl::StatusOr> Create( - const ScheduleGraph& graph, const DelayEstimator& delay_estimator, + FunctionBase* f, const DelayEstimator& delay_estimator, + const SchedulingOptions& options); + + static absl::StatusOr> Create( + ScheduleGraph graph, const DelayEstimator& delay_estimator, const SchedulingOptions& options); absl::Status AddConstraints( - absl::Span constraints) override; + absl::Span constraints); // Schedule to minimize the total pipeline registers using SDC scheduling // the constraint matrix is totally unimodular, this ILP problem can be solved @@ -262,6 +262,10 @@ class SDCScheduler final : public Scheduler { // with slack variables and give actionable feedback on how to update the // design to be feasible to schedule. // + // With `check_feasibility = true`, the objective function will be constant, + // and the LP solver will merely attempt to show that the generated set of + // constraints is feasible, rather than find an register-optimal schedule. + // // References: // - Cong, Jason, and Zhiru Zhang. "An efficient and versatile scheduling // algorithm based on SDC formulation." 2006 43rd ACM/IEEE Design Automation @@ -269,42 +273,21 @@ class SDCScheduler final : public Scheduler { // - Zhang, Zhiru, and Bin Liu. "SDC-based modulo scheduling for pipeline // synthesis." 2013 IEEE/ACM International Conference on Computer-Aided // Design (ICCAD). IEEE, 2013. - // - // TODO(allight): Calling this with failure_behavior.explain_infeasibility = - // true and getting an infeasible result will render the scheduler permanently - // unable to service any other schedule requests. This is because - // the process of explaining the infeasibility permanently alters the - // underlying SDC model in ways that prevent future schedulings from - // succeeding. absl::StatusOr Schedule( std::optional pipeline_stages, int64_t clock_period_ps, SchedulingFailureBehavior failure_behavior, - std::optional worst_case_throughput = std::nullopt) override; - - void SetDynamicThroughputObjectiveWeight(std::optional weight) { - dynamic_throughput_objective_weight_ = weight; - } - std::optional dynamic_throughput_objective_weight() const { - return dynamic_throughput_objective_weight_; - } - // If set to true then the objective will be ignored and a feasible solution - // will be returned. Defaults to false. - void SetCheckFeasibility(bool check_feasibility) { - check_feasibility_ = check_feasibility; - } - bool check_feasibility() const { return check_feasibility_; } + bool check_feasibility = false, + std::optional worst_case_throughput = std::nullopt, + std::optional dynamic_throughput_objective_weight = std::nullopt); private: SDCScheduler( - const ScheduleGraph& graph, double sdc_solution_tolerance, + ScheduleGraph graph, double sdc_solution_tolerance, ::operations_research::math_opt::SolverType solver_type, ::operations_research::math_opt::SolveParameters&& solve_parameters, std::optional initiation_interval, DelayMap&& delay_map); absl::Status Initialize(); - // TODO(allight): Calling this with failure_behavior.explain_infeasibility = - // true will render the scheduler permanently unable to service any other - // schedule requests. absl::Status BuildError( const operations_research::math_opt::SolveResult& result, SchedulingFailureBehavior failure_behavior); @@ -314,8 +297,6 @@ class SDCScheduler final : public Scheduler { ::operations_research::math_opt::SolveParameters solve_parameters_; SDCSchedulingModel model_; std::unique_ptr solver_; - std::optional dynamic_throughput_objective_weight_; - bool check_feasibility_ = false; }; } // namespace xls diff --git a/xls/scheduling/sdc_scheduler_test.cc b/xls/scheduling/sdc_scheduler_test.cc deleted file mode 100644 index b2bea20c64..0000000000 --- a/xls/scheduling/sdc_scheduler_test.cc +++ /dev/null @@ -1,151 +0,0 @@ -// Copyright 2026 The XLS Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "xls/scheduling/sdc_scheduler.h" - -#include -#include - -#include "gtest/gtest.h" -#include "absl/status/statusor.h" -#include "xls/common/status/matchers.h" -#include "xls/estimators/delay_model/delay_estimator.h" -#include "xls/ir/bits.h" -#include "xls/ir/channel.h" -#include "xls/ir/channel_ops.h" -#include "xls/ir/function_builder.h" -#include "xls/ir/ir_test_base.h" -#include "xls/ir/value.h" -#include "xls/scheduling/schedule_graph.h" -#include "xls/scheduling/scheduling_options.h" - -namespace xls { -namespace { - -class TestDelayEstimator : public DelayEstimator { - public: - TestDelayEstimator() : DelayEstimator("test") {} - absl::StatusOr GetOperationDelayInPs(Node* node) const override { - return 1; - } -}; - -class SDCSchedulerTest : public IrTestBase {}; - -TEST_F(SDCSchedulerTest, SimpleFunction) { - auto p = CreatePackage(); - FunctionBuilder fb(TestName(), p.get()); - auto x = fb.Param("x", p->GetBitsType(32)); - auto y = fb.Param("y", p->GetBitsType(32)); - auto add = fb.Add(x, y); - XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.Build()); - - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleGraph graph, - ScheduleGraph::Create(f, /*dead_after_synthesis=*/{})); - TestDelayEstimator delay_estimator; - SchedulingOptions options; - XLS_ASSERT_OK_AND_ASSIGN( - auto scheduler, SDCScheduler::Create(graph, delay_estimator, options)); - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleCycleMap cycle_map, - scheduler->Schedule(/*pipeline_stages=*/std::nullopt, - /*clock_period_ps=*/2, SchedulingFailureBehavior{})); - EXPECT_EQ(cycle_map.at(x.node()), 0); - EXPECT_EQ(cycle_map.at(y.node()), 0); - EXPECT_EQ(cycle_map.at(add.node()), 0); -} - -TEST_F(SDCSchedulerTest, SimpleProc) { - auto p = CreatePackage(); - ProcBuilder pb(TestName(), p.get()); - pb.StateElement("x", Value(UBits(0, 32))); - pb.Next(pb.GetStateParam(0), - pb.Add(pb.GetStateParam(0), pb.Literal(UBits(1, 32)))); - XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build()); - - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleGraph graph, - ScheduleGraph::Create(proc, /*dead_after_synthesis=*/{})); - TestDelayEstimator delay_estimator; - SchedulingOptions options; - XLS_ASSERT_OK_AND_ASSIGN( - auto scheduler, SDCScheduler::Create(graph, delay_estimator, options)); - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleCycleMap cycle_map, - scheduler->Schedule(/*pipeline_stages=*/std::nullopt, - /*clock_period_ps=*/2, SchedulingFailureBehavior{})); - EXPECT_EQ(cycle_map.at(pb.GetStateParam(0).node()), 0); -} - -TEST_F(SDCSchedulerTest, WithConstraint) { - auto p = CreatePackage(); - FunctionBuilder fb(TestName(), p.get()); - auto x = fb.Param("x", p->GetBitsType(32)); - auto y = fb.Param("y", p->GetBitsType(32)); - auto add = fb.Add(x, y); - XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.Build()); - - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleGraph graph, - ScheduleGraph::Create(f, /*dead_after_synthesis=*/{})); - TestDelayEstimator delay_estimator; - SchedulingOptions options; - XLS_ASSERT_OK_AND_ASSIGN( - auto scheduler, SDCScheduler::Create(graph, delay_estimator, options)); - XLS_ASSERT_OK( - scheduler->AddConstraints({NodeInCycleConstraint(add.node(), 2)})); - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleCycleMap cycle_map, - scheduler->Schedule(/*pipeline_stages=*/std::nullopt, - /*clock_period_ps=*/2, SchedulingFailureBehavior{})); - EXPECT_EQ(cycle_map.at(x.node()), 0); - EXPECT_EQ(cycle_map.at(y.node()), 0); - EXPECT_EQ(cycle_map.at(add.node()), 2); -} - -TEST_F(SDCSchedulerTest, WithIOConstraint) { - auto p = CreatePackage(); - XLS_ASSERT_OK_AND_ASSIGN( - Channel * ch_a, p->CreateStreamingChannel("a", ChannelOps::kReceiveOnly, - p->GetBitsType(32))); - XLS_ASSERT_OK_AND_ASSIGN(Channel * ch_b, - p->CreateStreamingChannel("b", ChannelOps::kSendOnly, - p->GetBitsType(32))); - - ProcBuilder pb(TestName(), p.get()); - BValue tkn = pb.Literal(Value::Token()); - BValue rcv = pb.Receive(ch_a, tkn); - BValue send = pb.Send(ch_b, pb.TupleIndex(rcv, 0), pb.TupleIndex(rcv, 1)); - XLS_ASSERT_OK_AND_ASSIGN(Proc * proc, pb.Build()); - - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleGraph graph, - ScheduleGraph::Create(proc, /*dead_after_synthesis=*/{})); - TestDelayEstimator delay_estimator; - SchedulingOptions options; - XLS_ASSERT_OK_AND_ASSIGN( - auto scheduler, SDCScheduler::Create(graph, delay_estimator, options)); - XLS_ASSERT_OK(scheduler->AddConstraints({IOConstraint( - "a", IODirection::kReceive, "b", IODirection::kSend, 1, 1)})); - XLS_ASSERT_OK_AND_ASSIGN( - ScheduleCycleMap cycle_map, - scheduler->Schedule(/*pipeline_stages=*/std::nullopt, - /*clock_period_ps=*/2, SchedulingFailureBehavior{})); - EXPECT_EQ(cycle_map.at(rcv.node()), 0); - EXPECT_EQ(cycle_map.at(send.node()), 1); -} - -} // namespace -} // namespace xls diff --git a/xls/tools/scheduling_options_flags.cc b/xls/tools/scheduling_options_flags.cc index f7f8148605..6b88cf07c1 100644 --- a/xls/tools/scheduling_options_flags.cc +++ b/xls/tools/scheduling_options_flags.cc @@ -99,28 +99,9 @@ ABSL_FLAG(xls::SchedulingStrategy, scheduling_strategy, "Good for development and rapid iteration.\n" "min_cut: Approximates the minimum number of registers using a " "min-cut based algorithm.\n" - " Does not respect all io_constraints.\n" + " Does not respect io_constraints.\n" "random: Randomly schedules nodes. For internal testing only.\n" " Does not respect io_constraints."); - -ABSL_FLAG(std::optional, find_bounds_strategy, - std::nullopt, - "Scheduler algorithm to use for finding minimum bounds.\n" - "\n" - "Options: [sdc, asap, min_cut, random]\n" - "\n" - "The options are the same as the `--scheduling_strategy` flag and " - "have the same restrictions and behaviors as described for that " - "flag. If unspecified, the default is to use the same strategy as " - "`--scheduling_strategy`.\n" - "\n" - "If possible, one should consider using the 'asap' strategy for " - "finding bounds, as it is much faster than the other strategies. It " - "is possible the asap schedule will be unable to find equivalent " - "bounds to the sdc one depending on set of io and other constraints " - "in the design.\n" - "\n" - "This flag is only used if one of the --minimize flags is set."); ABSL_FLAG(int64_t, clock_margin_percent, 0, "The percentage of clock period to set aside as a margin to ensure " "timing is met. Effectively, this lowers the clock period by this " @@ -307,13 +288,6 @@ static absl::StatusOr SetOptionsFromFlags( ToProtoSchedulingStrategy(absl::GetFlag(FLAGS_scheduling_strategy)); proto.set_scheduling_strategy(scheduling_strategy); } - { - any_flags_set |= FLAGS_find_bounds_strategy.IsSpecifiedOnCommandLine(); - ProtoSchedulingStrategy find_bounds_strategy = ToProtoSchedulingStrategy( - absl::GetFlag(FLAGS_find_bounds_strategy) - .value_or(absl::GetFlag(FLAGS_scheduling_strategy))); - proto.set_find_bounds_strategy(find_bounds_strategy); - } POPULATE_FLAG(clock_period_ps); POPULATE_FLAG(pipeline_stages); POPULATE_FLAG(delay_model); diff --git a/xls/tools/scheduling_options_flags.proto b/xls/tools/scheduling_options_flags.proto index 0a84b3797c..0ddc889445 100644 --- a/xls/tools/scheduling_options_flags.proto +++ b/xls/tools/scheduling_options_flags.proto @@ -103,6 +103,4 @@ message SchedulingOptionsFlagsProto { 37; // What sort of scheduler one should use. optional ProtoSchedulingStrategy scheduling_strategy = 38; - // What sort of scheduler to use when finding bounds. - optional ProtoSchedulingStrategy find_bounds_strategy = 39; }