EXPERIMENTAL - [scheduling] Remove MutualExclusionPass

xls/codegen/BUILD

@@ -442,6 +442,7 @@ cc_library(
         "//xls/ir",
         "//xls/ir:bits",
         "//xls/ir:channel",
+        "//xls/ir:channel_ops",
         "//xls/ir:node_util",
         "//xls/ir:op",
         "//xls/ir:proc_elaboration",

xls/codegen/block_conversion.cc

@@ -299,34 +299,29 @@ absl::Status AddCombinationalFlowControl(
     std::vector<std::vector<StreamingOutput>>& streaming_outputs,
     std::vector<std::optional<Node*>>& stage_valid,
     const CodegenOptions& options, Proc* proc, Block* block) {
-  std::string_view valid_suffix = options.streaming_channel_valid_suffix();
-  std::string_view ready_suffix = options.streaming_channel_ready_suffix();
-  std::string_view data_suffix = options.streaming_channel_data_suffix();
-
   XLS_ASSIGN_OR_RETURN(
       std::vector<Node*> all_active_outputs_ready,
       MakeInputReadyPortsForOutputChannels(streaming_outputs, /*stage_count=*/1,
-                                           ready_suffix, block));
+                                           options, block));
 
   XLS_ASSIGN_OR_RETURN(
       std::vector<Node*> all_active_inputs_valid,
       MakeInputValidPortsForInputChannels(streaming_inputs, /*stage_count=*/1,
-                                          valid_suffix, block));
+                                          options, block));
 
   XLS_ASSIGN_OR_RETURN(Node * literal_1, block->MakeNode<xls::Literal>(
                                              SourceInfo(), Value(UBits(1, 1))));
   std::vector<Node*> pipelined_valids{literal_1};
   std::vector<Node*> next_stage_open{literal_1};
   XLS_RETURN_IF_ERROR(MakeOutputValidPortsForOutputChannels(
       all_active_inputs_valid, pipelined_valids, next_stage_open,
-      streaming_outputs, valid_suffix, proc, {}, block));
+      streaming_outputs, options, proc, {}, block));
   XLS_RETURN_IF_ERROR(MakeOutputDataPortsForOutputChannels(
       all_active_inputs_valid, pipelined_valids, next_stage_open,
-      streaming_outputs, data_suffix, block));
+      streaming_outputs, options, block));
 
   XLS_RETURN_IF_ERROR(MakeOutputReadyPortsForInputChannels(
-      all_active_outputs_ready, streaming_inputs, ready_suffix, proc, {},
-      block));
+      all_active_outputs_ready, streaming_inputs, options, proc, {}, block));
 
   XLS_RET_CHECK(stage_valid.empty());
   XLS_RET_CHECK_EQ(all_active_inputs_valid.size(), 1);

xls/codegen/conversion_utils.cc

@@ -40,6 +40,7 @@
 #include "xls/ir/bits.h"
 #include "xls/ir/block.h"
 #include "xls/ir/channel.h"
+#include "xls/ir/channel_ops.h"
 #include "xls/ir/instantiation.h"
 #include "xls/ir/node.h"
 #include "xls/ir/node_util.h"
@@ -61,32 +62,61 @@ namespace xls::verilog {
 
 namespace {
 
+bool HasMultiplexingCycle(const FifoConfig& fifo_config) {
+  const bool push_to_pop_combo_paths =
+      fifo_config.bypass() && !fifo_config.register_pop_outputs();
+  const bool pop_to_push_combo_paths = !fifo_config.register_push_outputs();
+  return push_to_pop_combo_paths && pop_to_push_combo_paths;
+}
+
 absl::Status CheckForMultiplexingCycle(Channel* channel,
+                                       const CodegenOptions& options,
                                        std::string_view proc_name) {
   if (channel->kind() != ChannelKind::kStreaming) {
     return absl::OkStatus();
   }
   StreamingChannel* streaming_channel = down_cast<StreamingChannel*>(channel);
+  if (streaming_channel->supported_ops() != ChannelOps::kSendReceive) {
+    // This is an external channel; any I/O flopping will break the cycle.
+    if (options.flop_inputs() || options.flop_outputs()) {
+      return absl::OkStatus();
+    }
+    // If we've been given FIFO config information, we can rely on that;
+    // otherwise, we should tell the user how to fix the issue.
+    if (streaming_channel->channel_config().fifo_config().has_value() &&
+        !HasMultiplexingCycle(
+            *streaming_channel->channel_config().fifo_config())) {
+      return absl::OkStatus();
+    }
+    return absl::InvalidArgumentError(absl::StrFormat(
+        "Cannot allow multiple operations from proc `%s` on external streaming "
+        "channel `%s` due to a potential combinational cycle. This can be "
+        "fixed by any of the following changes:\n"
+        "\t1. flop inputs (with the `--flop_inputs` flag),\n"
+        "\t2. flop outputs (with the `--flop_outputs` flag), or\n"
+        "if channel `%s` connects to an external FIFO guaranteed to have "
+        "no combinational path between valid and ready, you can specify the "
+        "reason for this in the channel's FIFO config; it should involve at "
+        "least one of registered push outputs, registered pop outputs, or "
+        "disabled combinational bypass.",
+        proc_name, ChannelRefName(channel), ChannelRefName(channel)));
+  }
+
   if (!streaming_channel->channel_config().fifo_config().has_value()) {
     return absl::InvalidArgumentError(
         absl::StrCat("Cannot allow multiple operations from proc `", proc_name,
                      "` on streaming channel `", ChannelRefName(channel),
                      "`, which has no known FIFO configuration."));
   }
-  const FifoConfig& fifo_config =
-      *streaming_channel->channel_config().fifo_config();
-  bool push_to_pop_combo_paths =
-      fifo_config.bypass() && !fifo_config.register_pop_outputs();
-  bool pop_to_push_combo_paths = !fifo_config.register_push_outputs();
-  if (push_to_pop_combo_paths && pop_to_push_combo_paths) {
+  if (HasMultiplexingCycle(
+          *streaming_channel->channel_config().fifo_config())) {
     return absl::InvalidArgumentError(absl::StrFormat(
         "Cannot allow multiple operations from proc `%s` on streaming channel "
         "`%s` due to a combinational cycle. This can be fixed by reconfiguring "
         "channel `%s` in any of the following ways:\n"
-        "\t1. use proven-mutually-exclusive channel strictness (if possible),\n"
-        "\t2. register push outputs,\n"
-        "\t3. register pop outputs, or\n"
-        "\t4. disable combinational bypass paths.",
+        "\t1. register push outputs,\n"
+        "\t2. register pop outputs, or\n"
+        "\t3. disable combinational bypass paths.",
         proc_name, ChannelRefName(channel), ChannelRefName(channel)));
   }
   return absl::OkStatus();
@@ -138,7 +168,7 @@ std::string PipelineSignalName(std::string_view root, int64_t stage) {
 
 absl::StatusOr<std::vector<Node*>> MakeInputReadyPortsForOutputChannels(
     std::vector<std::vector<StreamingOutput>>& streaming_outputs,
-    int64_t stage_count, std::string_view ready_suffix, Block* block) {
+    int64_t stage_count, const CodegenOptions& options, Block* block) {
   std::vector<Node*> result;
 
   // Add a ready input port for each streaming output. Gather the ready signals
@@ -200,7 +230,7 @@ absl::StatusOr<std::vector<Node*>> MakeInputReadyPortsForOutputChannels(
 // Upon success returns a Node* to the all_active_inputs_valid signal.
 absl::StatusOr<std::vector<Node*>> MakeInputValidPortsForInputChannels(
     std::vector<std::vector<StreamingInput>>& streaming_inputs,
-    int64_t stage_count, std::string_view valid_suffix, Block* block) {
+    int64_t stage_count, const CodegenOptions& options, Block* block) {
   std::vector<Node*> result;
 
   for (Stage stage = 0; stage < stage_count; ++stage) {
@@ -424,7 +454,7 @@ absl::Status MakeOutputValidPortsForOutputChannels(
     absl::Span<Node* const> pipelined_valids,
     absl::Span<Node* const> next_stage_open,
     std::vector<std::vector<StreamingOutput>>& streaming_outputs,
-    std::string_view valid_suffix, Proc* proc,
+    const CodegenOptions& options, Proc* proc,
     absl::Span<ProcInstance* const> instances, Block* block) {
   absl::btree_map<Node*,
                   absl::btree_map<Stage, std::vector<std::optional<Node*>>>,
@@ -447,13 +477,13 @@ absl::Status MakeOutputValidPortsForOutputChannels(
       if (std::holds_alternative<Channel*>(channel)) {
         CHECK(instances.empty());
         XLS_RETURN_IF_ERROR(CheckForMultiplexingCycle(
-            std::get<Channel*>(channel), proc->name()));
+            std::get<Channel*>(channel), options, proc->name()));
       } else {
         CHECK(std::holds_alternative<ChannelInterface*>(channel));
         CHECK(!instances.empty());
         for (ProcInstance* instance : instances) {
           XLS_RETURN_IF_ERROR(CheckForMultiplexingCycle(
-              instance->GetChannelBinding(channel).instance->channel,
+              instance->GetChannelBinding(channel).instance->channel, options,
               instance->GetName()));
         }
       }
@@ -553,7 +583,7 @@ absl::Status MakeOutputDataPortsForOutputChannels(
     absl::Span<Node* const> pipelined_valids,
     absl::Span<Node* const> next_stage_open,
     std::vector<std::vector<StreamingOutput>>& streaming_outputs,
-    std::string_view data_suffix, Block* block) {
+    const CodegenOptions& options, Block* block) {
   absl::btree_map<Node*, absl::btree_map<Stage, std::vector<PredicatedValue>>,
                   Node::NodeIdLessThan>
       data_values;
@@ -613,7 +643,7 @@ absl::Status MakeOutputDataPortsForOutputChannels(
 absl::Status MakeOutputReadyPortsForInputChannels(
     absl::Span<Node* const> all_active_outputs_ready,
     std::vector<std::vector<StreamingInput>>& streaming_inputs,
-    std::string_view ready_suffix, Proc* proc,
+    const CodegenOptions& options, Proc* proc,
     absl::Span<ProcInstance* const> instances, Block* block) {
   absl::btree_map<Node*,
                   absl::btree_map<Stage, std::vector<std::optional<Node*>>>,
@@ -636,13 +666,13 @@ absl::Status MakeOutputReadyPortsForInputChannels(
       if (std::holds_alternative<Channel*>(channel)) {
         CHECK(instances.empty());
         XLS_RETURN_IF_ERROR(CheckForMultiplexingCycle(
-            std::get<Channel*>(channel), proc->name()));
+            std::get<Channel*>(channel), options, proc->name()));
       } else {
         CHECK(std::holds_alternative<ChannelInterface*>(channel));
         CHECK(!instances.empty());
         for (ProcInstance* instance : instances) {
           XLS_RETURN_IF_ERROR(CheckForMultiplexingCycle(
-              instance->GetChannelBinding(channel).instance->channel,
+              instance->GetChannelBinding(channel).instance->channel, options,
               instance->GetName()));
         }
       }

xls/codegen/conversion_utils.h

@@ -59,11 +59,11 @@ std::string PipelineSignalName(std::string_view root, int64_t stage);
 // Upon success returns a Node* to the all_active_inputs_valid signal.
 absl::StatusOr<std::vector<Node*>> MakeInputReadyPortsForOutputChannels(
     std::vector<std::vector<StreamingOutput>>& streaming_outputs,
-    int64_t stage_count, std::string_view ready_suffix, Block* block);
+    int64_t stage_count, const CodegenOptions& options, Block* block);
 
 absl::StatusOr<std::vector<Node*>> MakeInputValidPortsForInputChannels(
     std::vector<std::vector<StreamingInput>>& streaming_inputs,
-    int64_t stage_count, std::string_view valid_suffix, Block* block);
+    int64_t stage_count, const CodegenOptions& options, Block* block);
 
 // Given a node returns a node that is OR'd with the reset signal.
 // if said reset signal exists.  That node can be thought of as
@@ -117,7 +117,7 @@ absl::Status MakeOutputValidPortsForOutputChannels(
     absl::Span<Node* const> pipelined_valids,
     absl::Span<Node* const> next_stage_open,
     std::vector<std::vector<StreamingOutput>>& streaming_outputs,
-    std::string_view valid_suffix, Proc* proc,
+    const CodegenOptions& options, Proc* proc,
     absl::Span<ProcInstance* const> instances, Block* block);
 
 // Make data ports (output) for each output channel.
@@ -130,7 +130,7 @@ absl::Status MakeOutputDataPortsForOutputChannels(
     absl::Span<Node* const> pipelined_valids,
     absl::Span<Node* const> next_stage_open,
     std::vector<std::vector<StreamingOutput>>& streaming_outputs,
-    std::string_view data_suffix, Block* block);
+    const CodegenOptions& options, Block* block);
 
 // Make ready ports (output) for each input channel.
 //
@@ -139,7 +139,7 @@ absl::Status MakeOutputDataPortsForOutputChannels(
 absl::Status MakeOutputReadyPortsForInputChannels(
     absl::Span<Node* const> all_active_outputs_ready,
     std::vector<std::vector<StreamingInput>>& streaming_inputs,
-    std::string_view ready_suffix, Proc* proc,
+    const CodegenOptions& options, Proc* proc,
     absl::Span<ProcInstance* const> instances, Block* block);
 }  // namespace xls::verilog
 

xls/codegen/proc_block_conversion.cc

@@ -437,7 +437,7 @@ static absl::StatusOr<Node*> UpdateSingleStagePipelineWithFlowControl(
 static absl::StatusOr<std::vector<Node*>> MakeValidNodesForInputStates(
     std::vector<std::vector<int64_t>>& input_states,
     absl::Span<std::optional<StateRegister> const> state_registers,
-    int64_t stage_count, std::string_view valid_suffix, Block* block) {
+    int64_t stage_count, Block* block) {
   std::vector<Node*> result;
   for (Stage stage = 0; stage < stage_count; ++stage) {
     // Add a valid signal for each state element with non-trivial backedge.
@@ -813,34 +813,28 @@ static absl::Status AddBubbleFlowControl(
     Proc* proc, absl::Span<ProcInstance* const> instances, Block* block,
     std::vector<Node*>& all_active_outputs_ready) {
   int64_t stage_count = streaming_io.pipeline_registers.size() + 1;
-  std::string_view data_suffix = options.streaming_channel_data_suffix();
-  std::string_view valid_suffix = options.streaming_channel_valid_suffix();
-  std::string_view ready_suffix = options.streaming_channel_ready_suffix();
 
   // Node in each stage that represents when all inputs channels (that are
   // predicated true) are valid.
-  XLS_ASSIGN_OR_RETURN(
-      std::vector<Node*> all_active_inputs_valid,
-      MakeInputValidPortsForInputChannels(streaming_io.inputs, stage_count,
-                                          valid_suffix, block));
+  XLS_ASSIGN_OR_RETURN(std::vector<Node*> all_active_inputs_valid,
+                       MakeInputValidPortsForInputChannels(
+                           streaming_io.inputs, stage_count, options, block));
   VLOG(3) << "After Inputs Valid";
   XLS_VLOG_LINES(3, block->DumpIr());
 
   // Node in each stage that represents when all output channels (that are
   // predicated true) are ready.
-  XLS_ASSIGN_OR_RETURN(
-      all_active_outputs_ready,
-      MakeInputReadyPortsForOutputChannels(streaming_io.outputs, stage_count,
-                                           ready_suffix, block));
+  XLS_ASSIGN_OR_RETURN(all_active_outputs_ready,
+                       MakeInputReadyPortsForOutputChannels(
+                           streaming_io.outputs, stage_count, options, block));
   VLOG(3) << "After Outputs Ready";
   XLS_VLOG_LINES(3, block->DumpIr());
 
   // Node in each stage that represents when all state values are valid.
-  XLS_ASSIGN_OR_RETURN(
-      std::vector<Node*> all_active_states_valid,
-      MakeValidNodesForInputStates(streaming_io.input_states,
-                                   streaming_io.state_registers, stage_count,
-                                   valid_suffix, block));
+  XLS_ASSIGN_OR_RETURN(std::vector<Node*> all_active_states_valid,
+                       MakeValidNodesForInputStates(
+                           streaming_io.input_states,
+                           streaming_io.state_registers, stage_count, block));
   VLOG(3) << "After States Valid";
   XLS_VLOG_LINES(3, block->DumpIr());
 
@@ -884,11 +878,11 @@ static absl::Status AddBubbleFlowControl(
     }
     XLS_RETURN_IF_ERROR(MakeOutputValidPortsForOutputChannels(
         all_active_inputs_valid, stage_valid_no_option,
-        bubble_flow_control.next_stage_open, streaming_io.outputs, valid_suffix,
+        bubble_flow_control.next_stage_open, streaming_io.outputs, options,
         proc, instances, block));
     XLS_RETURN_IF_ERROR(MakeOutputDataPortsForOutputChannels(
         all_active_inputs_valid, stage_valid_no_option,
-        bubble_flow_control.next_stage_open, streaming_io.outputs, data_suffix,
+        bubble_flow_control.next_stage_open, streaming_io.outputs, options,
         block));
   }
 
@@ -910,8 +904,8 @@ static absl::Status AddBubbleFlowControl(
   XLS_VLOG_LINES(3, block->DumpIr());
 
   XLS_RETURN_IF_ERROR(MakeOutputReadyPortsForInputChannels(
-      bubble_flow_control.data_load_enable, streaming_io.inputs, ready_suffix,
-      proc, instances, block));
+      bubble_flow_control.data_load_enable, streaming_io.inputs, options, proc,
+      instances, block));
 
   VLOG(3) << "After Ready";
   XLS_VLOG_LINES(3, block->DumpIr());

xls/contrib/xlscc/unit_tests/unit_test.cc

@@ -505,6 +505,7 @@ void XlsccTestBase::BlockTest(
   codegen_flags_proto.set_generator(xls::GENERATOR_KIND_PIPELINE);
   codegen_flags_proto.set_streaming_channel_ready_suffix(channel_ready_suffix);
   codegen_flags_proto.set_streaming_channel_valid_suffix(channel_valid_suffix);
+  codegen_flags_proto.set_flop_inputs(true);
 
   XLS_ASSERT_OK(xls::ScheduleAndCodegen(package_.get(),
                                         scheduling_options_flags_proto,

xls/scheduling/BUILD

@@ -418,7 +418,6 @@ cc_library(
     srcs = ["scheduling_pass_pipeline.cc"],
     hdrs = ["scheduling_pass_pipeline.h"],
     deps = [
-        ":mutual_exclusion_pass",
         ":pipeline_scheduling_pass",
         ":proc_state_legalization_pass",
         ":scheduling_checker",

xls/scheduling/scheduling_pass_pipeline.cc

@@ -24,7 +24,6 @@
 #include "xls/passes/literal_uncommoning_pass.h"
 #include "xls/passes/optimization_pass.h"
 #include "xls/passes/optimization_pass_pipeline.h"
-#include "xls/scheduling/mutual_exclusion_pass.h"
 #include "xls/scheduling/pipeline_scheduling_pass.h"
 #include "xls/scheduling/proc_state_legalization_pass.h"
 #include "xls/scheduling/scheduling_checker.h"
@@ -44,7 +43,6 @@ std::unique_ptr<SchedulingCompoundPass> CreateSchedulingPassPipeline(
   top->Add<ProcStateLegalizationPass>();
 
   bool eliminate_noop_next = false;
-  top->Add<MutualExclusionPass>();
   if (opt_level > 0) {
     top->Add<SchedulingWrapperPass>(
         std::make_unique<FixedPointSimplificationPass>(), context, opt_level,
@@ -55,10 +53,6 @@ std::unique_ptr<SchedulingCompoundPass> CreateSchedulingPassPipeline(
   top->Add<PipelineSchedulingPass>();
   top->Add<SchedulingWrapperPass>(std::make_unique<DeadCodeEliminationPass>(),
                                   context, opt_level, eliminate_noop_next);
-  top->Add<MutualExclusionPass>();
-  top->Add<SchedulingWrapperPass>(std::make_unique<DeadCodeEliminationPass>(),
-                                  context, opt_level, eliminate_noop_next);
-  top->Add<PipelineSchedulingPass>();
 
   return top;
 }