From cac9fe887280fa04c5e73be7fd16e17516d70066 Mon Sep 17 00:00:00 2001 From: Eric Astor Date: Thu, 21 May 2026 12:58:30 -0700 Subject: [PATCH] Start adding `valid_data` as a Channel flow-control type For now, we put placeholder errors in every place where this could possibly affect how we handle channels. The goal is that this will be used for channels that don't support backpressure - probably with a codegen option to add an assert if data is ever received by a proc that isn't actually ready to receive the incoming data. PiperOrigin-RevId: 919208854 --- xls/codegen/BUILD | 3 ++- xls/codegen/block_stitching_pass.cc | 5 +++++ xls/codegen/clone_nodes_into_block_handler.cc | 16 +++++++++++++++ xls/codegen/conversion_utils.cc | 4 ++++ xls/codegen/mark_channel_fifos_pass.cc | 6 ++++++ xls/codegen/module_signature.cc | 4 ++++ xls/codegen/module_signature.proto | 1 + .../passes_ng/stage_to_block_conversion.cc | 9 +++++++++ xls/codegen/signature_generator.cc | 6 ++++++ xls/codegen_v_1_5/BUILD | 1 + .../channel_to_port_io_lowering_pass.cc | 20 +++++++++++++++++++ .../global_channel_block_stitching_pass.cc | 20 +++++++++++++++++++ .../proc_lowering_block_eval_psc_test.cc | 18 +++++++++++++++++ .../signature_generation_pass.cc | 7 +++++++ xls/ir/channel.cc | 7 ++++++- xls/ir/channel.h | 9 +++++++++ 16 files changed, 134 insertions(+), 2 deletions(-) diff --git a/xls/codegen/BUILD b/xls/codegen/BUILD index febba2b6e0..dd16c2c5c7 100644 --- a/xls/codegen/BUILD +++ b/xls/codegen/BUILD @@ -734,6 +734,7 @@ cc_library( "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/log", "@com_google_absl//absl/log:check", + "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings:str_format", ], @@ -953,7 +954,6 @@ proto_library( name = "module_signature_proto", srcs = ["module_signature.proto"], deps = [ - ":xls_metrics_proto", "//xls/ir:channel_proto", "//xls/ir:foreign_function_data_proto", "//xls/ir:xls_type_proto", @@ -2011,6 +2011,7 @@ cc_library( "//xls/ir:channel", "//xls/passes:pass_base", "@com_google_absl//absl/base", + "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", ], ) diff --git a/xls/codegen/block_stitching_pass.cc b/xls/codegen/block_stitching_pass.cc index d03c3b9e98..d4c6a2e2fa 100644 --- a/xls/codegen/block_stitching_pass.cc +++ b/xls/codegen/block_stitching_pass.cc @@ -490,6 +490,11 @@ absl::Status StitchSingleValueChannel( absl::Status StitchStreamingChannel( Block* container, StreamingChannel* channel, const ChannelMap& channel_map, const absl::flat_hash_map& instantiations) { + if (channel->flow_control() == FlowControl::kValidData) { + return absl::UnimplementedError( + "Channels with valid_data flow control are not supported in codegen " + "v1.0."); + } auto input_iter = channel_map.channel_to_streaming_input().find(channel); auto output_iter = channel_map.channel_to_streaming_output().find(channel); bool has_input = input_iter != channel_map.channel_to_streaming_input().end(); diff --git a/xls/codegen/clone_nodes_into_block_handler.cc b/xls/codegen/clone_nodes_into_block_handler.cc index 48ec0c26bc..72d8d23ec8 100644 --- a/xls/codegen/clone_nodes_into_block_handler.cc +++ b/xls/codegen/clone_nodes_into_block_handler.cc @@ -242,6 +242,10 @@ AddPortsForReceive(ChannelRef channel, Block* block, std::optional valid; std::optional ready; if (ChannelRefKind(channel) == ChannelKind::kStreaming) { + if (ChannelRefFlowControl(channel) == FlowControl::kValidData) { + return absl::UnimplementedError( + "valid_data flow control not supported in codegen v1.0"); + } XLS_ASSIGN_OR_RETURN( valid, block->AddInputPort( absl::StrCat(ChannelRefName(channel), @@ -290,6 +294,10 @@ absl::StatusOr AddPortsForSend( std::optional valid; std::optional ready; if (ChannelRefKind(channel) == ChannelKind::kStreaming) { + if (ChannelRefFlowControl(channel) == FlowControl::kValidData) { + return absl::UnimplementedError( + "valid_data flow control not supported in codegen v1.0"); + } XLS_ASSIGN_OR_RETURN(Node * one, block->MakeNode( SourceInfo(), Value(UBits(1, 1)))); XLS_ASSIGN_OR_RETURN( @@ -875,6 +883,10 @@ absl::StatusOr CloneNodesIntoBlockHandler::HandleReceiveNode( } XLS_RET_CHECK_EQ(ChannelRefKind(connection.channel), ChannelKind::kStreaming); + if (ChannelRefFlowControl(connection.channel) == FlowControl::kValidData) { + return absl::UnimplementedError( + "valid_data flow control not supported in codegen v1.0"); + } XLS_RET_CHECK_EQ(ChannelRefFlowControl(connection.channel), FlowControl::kReadyValid) << " channel " << ChannelRefToString(connection.channel); @@ -999,6 +1011,10 @@ absl::StatusOr CloneNodesIntoBlockHandler::HandleSendNode( } XLS_RET_CHECK_EQ(ChannelRefKind(connection.channel), ChannelKind::kStreaming); + if (ChannelRefFlowControl(connection.channel) == FlowControl::kValidData) { + return absl::UnimplementedError( + "valid_data flow control not supported in codegen v1.0"); + } XLS_RET_CHECK_EQ(ChannelRefFlowControl(connection.channel), FlowControl::kReadyValid); diff --git a/xls/codegen/conversion_utils.cc b/xls/codegen/conversion_utils.cc index dafd41e64d..f607c558be 100644 --- a/xls/codegen/conversion_utils.cc +++ b/xls/codegen/conversion_utils.cc @@ -69,6 +69,10 @@ absl::Status CheckForMultiplexingCycle(Channel* channel, } StreamingChannel* streaming_channel = absl::down_cast(channel); + if (streaming_channel->flow_control() == FlowControl::kValidData) { + return absl::UnimplementedError( + "Channels with flow control valid_data are not yet supported."); + } if (!streaming_channel->channel_config().fifo_config().has_value()) { return absl::InvalidArgumentError( absl::StrCat("Cannot allow multiple operations from proc `", proc_name, diff --git a/xls/codegen/mark_channel_fifos_pass.cc b/xls/codegen/mark_channel_fifos_pass.cc index f3196a15ae..4bed0f598a 100644 --- a/xls/codegen/mark_channel_fifos_pass.cc +++ b/xls/codegen/mark_channel_fifos_pass.cc @@ -15,6 +15,7 @@ #include "xls/codegen/mark_channel_fifos_pass.h" #include "absl/base/casts.h" +#include "absl/status/status.h" #include "absl/status/statusor.h" #include "xls/codegen/codegen_options.h" #include "xls/codegen/codegen_pass.h" @@ -45,6 +46,11 @@ absl::StatusOr MarkChannelFifosPass::RunInternal( continue; } StreamingChannel* schan = absl::down_cast(chan); + if (schan->flow_control() == FlowControl::kValidData) { + return absl::UnimplementedError( + "Channels with flow control valid_data are not supported in codegen " + "v1.0."); + } if (!schan->channel_config().input_flop_kind()) { schan->SetChannelConfig(schan->channel_config().WithInputFlopKind( GetRealFlopKind(options.codegen_options.flop_inputs(), diff --git a/xls/codegen/module_signature.cc b/xls/codegen/module_signature.cc index c14f1c47fa..45d91dcf9e 100644 --- a/xls/codegen/module_signature.cc +++ b/xls/codegen/module_signature.cc @@ -171,6 +171,8 @@ ModuleSignatureBuilder& ModuleSignatureBuilder::AddStreamingChannel( ChannelProto* channel = proto_.add_channels(); channel->set_name(name); channel->set_kind(CHANNEL_KIND_STREAMING); + CHECK_NE(flow_control, FlowControl::kValidData) + << "valid_data flow control not supported in codegen v1.0"; if (flow_control == FlowControl::kReadyValid) { channel->set_flow_control(CHANNEL_FLOW_CONTROL_READY_VALID); } else { @@ -212,6 +214,8 @@ ModuleSignatureBuilder& ModuleSignatureBuilder::AddStreamingChannelInterface( interface->set_direction(direction); *interface->mutable_type() = type->ToProto(); interface->set_kind(CHANNEL_KIND_STREAMING); + CHECK_NE(flow_control, FlowControl::kValidData) + << "valid_data flow control not supported in codegen v1.0"; interface->mutable_streaming()->set_flow_control( flow_control == FlowControl::kReadyValid ? CHANNEL_FLOW_CONTROL_READY_VALID diff --git a/xls/codegen/module_signature.proto b/xls/codegen/module_signature.proto index e4aee0a4f8..fbb0b4c3fe 100644 --- a/xls/codegen/module_signature.proto +++ b/xls/codegen/module_signature.proto @@ -42,6 +42,7 @@ enum ChannelKindProto { enum ChannelFlowControlProto { CHANNEL_FLOW_CONTROL_NONE = 0; CHANNEL_FLOW_CONTROL_READY_VALID = 1; + CHANNEL_FLOW_CONTROL_VALID_DATA = 2; } // Data structure describing a port on a module. diff --git a/xls/codegen/passes_ng/stage_to_block_conversion.cc b/xls/codegen/passes_ng/stage_to_block_conversion.cc index 131741bbe1..9fe6b22ce5 100644 --- a/xls/codegen/passes_ng/stage_to_block_conversion.cc +++ b/xls/codegen/passes_ng/stage_to_block_conversion.cc @@ -73,6 +73,8 @@ RDVNodeGroupName CreatePortNamesForChannel( ? options.streaming_channel_data_suffix() : ""; std::string_view valid_suffix = options.streaming_channel_valid_suffix(); + CHECK_NE(chan_interface->flow_control(), FlowControl::kValidData) + << "valid_data flow control not supported in codegen v2.0"; // Construct names for the ports. std::string port_ready_name = @@ -530,6 +532,13 @@ class TopProcToBlockCloner : public ProcToBlockClonerBase { absl::Status CreateWiresForChannel(const Channel* chan, Block* block) { Proc* proc = proc_metadata_.proc(); + if (chan->kind() == ChannelKind::kStreaming && + absl::down_cast(chan)->flow_control() == + FlowControl::kValidData) { + return absl::UnimplementedError( + "valid_data flow control not supported in codegen v2.0"); + } + // Retrieve the suffix for the ports associated with the channel. std::string_view data_suffix = (chan->kind() == ChannelKind::kStreaming) diff --git a/xls/codegen/signature_generator.cc b/xls/codegen/signature_generator.cc index b669894f40..3c1586b7a5 100644 --- a/xls/codegen/signature_generator.cc +++ b/xls/codegen/signature_generator.cc @@ -26,6 +26,7 @@ #include "absl/container/flat_hash_map.h" #include "absl/log/check.h" #include "absl/log/log.h" +#include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_format.h" #include "xls/codegen/codegen_options.h" @@ -150,6 +151,11 @@ absl::StatusOr GenerateSignature( : CHANNEL_DIRECTION_RECEIVE; switch (metadata.channel_kind) { case ChannelKind::kStreaming: { + if (metadata.ready_port.has_value() != + metadata.valid_port.has_value()) { + return absl::UnimplementedError( + "Codegen v1.0 only supports ready_valid or no flow control."); + } FlowControl flow_control = (metadata.ready_port.has_value() && metadata.valid_port.has_value()) ? FlowControl::kReadyValid diff --git a/xls/codegen_v_1_5/BUILD b/xls/codegen_v_1_5/BUILD index 548fa4aabb..a81a3be1f3 100644 --- a/xls/codegen_v_1_5/BUILD +++ b/xls/codegen_v_1_5/BUILD @@ -1060,6 +1060,7 @@ cc_library( "@com_google_absl//absl/base", "@com_google_absl//absl/log", "@com_google_absl//absl/log:check", + "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings:str_format", ], diff --git a/xls/codegen_v_1_5/channel_to_port_io_lowering_pass.cc b/xls/codegen_v_1_5/channel_to_port_io_lowering_pass.cc index 6c73fe86f5..4108e35ddb 100644 --- a/xls/codegen_v_1_5/channel_to_port_io_lowering_pass.cc +++ b/xls/codegen_v_1_5/channel_to_port_io_lowering_pass.cc @@ -352,6 +352,10 @@ absl::StatusOr AddPortsForSend( std::optional valid; std::optional ready; if (ChannelRefKind(channel) == ChannelKind::kStreaming) { + if (ChannelRefFlowControl(channel) == FlowControl::kValidData) { + return absl::UnimplementedError( + "valid_data flow control is not yet supported."); + } XLS_ASSIGN_OR_RETURN( Node * placeholder_valid, block->MakeNode(SourceInfo(), Value(UBits(1, 1)))); @@ -416,6 +420,10 @@ absl::StatusOr AddPortsForReceive( std::optional valid; std::optional ready; if (ChannelRefKind(channel) == ChannelKind::kStreaming) { + if (ChannelRefFlowControl(channel) == FlowControl::kValidData) { + return absl::UnimplementedError( + "valid_data flow control is not yet supported."); + } XLS_ASSIGN_OR_RETURN( valid, block->AddInputPort( @@ -1076,6 +1084,10 @@ absl::Status AddOneShotLogic(Connector& connector, ScheduledBlock* block, const BlockConversionPassOptions& options, std::string_view channel_name = "") { XLS_RET_CHECK_EQ(connector.direction, ChannelDirection::kSend); + if (connector.valid.has_value() && !connector.ready.has_value()) { + return absl::UnimplementedError( + "valid_data flow control is not yet supported."); + } XLS_RET_CHECK(connector.ready.has_value()); XLS_RET_CHECK(connector.valid.has_value()); @@ -1505,6 +1517,10 @@ absl::Status AddIOFlopsForReceive(Connector& connector, FlopKind flop_kind, return absl::OkStatus(); } CHECK_EQ(ChannelRefKind(channel), ChannelKind::kStreaming); + if (ChannelRefFlowControl(channel) == FlowControl::kValidData) { + return absl::UnimplementedError( + "Valid-data flow control is not yet supported."); + } if (flop_kind == FlopKind::kNone) { return absl::OkStatus(); @@ -1558,6 +1574,10 @@ absl::Status AddIOFlopsForSend(Connector& connector, FlopKind flop_kind, .status(); } CHECK_EQ(ChannelRefKind(channel), ChannelKind::kStreaming); + if (ChannelRefFlowControl(channel) == FlowControl::kValidData) { + return absl::UnimplementedError( + "Valid-data flow control is not yet supported."); + } CHECK(connector.valid.has_value()); CHECK(connector.ready.has_value()); diff --git a/xls/codegen_v_1_5/global_channel_block_stitching_pass.cc b/xls/codegen_v_1_5/global_channel_block_stitching_pass.cc index 19efd1efd7..39a10c6c10 100644 --- a/xls/codegen_v_1_5/global_channel_block_stitching_pass.cc +++ b/xls/codegen_v_1_5/global_channel_block_stitching_pass.cc @@ -146,6 +146,11 @@ absl::Status StitchStreamingOutputToFifo( SourceInfo(), block_valid, fifo, FifoInstantiation::kPushValidPortName) .status()); + if (!output.ready_port.has_value()) { + return absl::UnimplementedError( + "Channels with flow control other than ready_valid are not yet " + "supported."); + } XLS_RET_CHECK(output.ready_port.has_value()); XLS_RETURN_IF_ERROR(caller ->MakeNode( @@ -168,6 +173,11 @@ absl::Status StitchStreamingInputToFifo( xls::Node * fifo_valid, caller->MakeNode( SourceInfo(), fifo, FifoInstantiation::kPopValidPortName)); + if (!input.ready_port.has_value()) { + return absl::UnimplementedError( + "Channels with flow control other than ready_valid are not yet " + "supported."); + } XLS_RET_CHECK(input.ready_port.has_value()); XLS_ASSIGN_OR_RETURN( @@ -205,6 +215,11 @@ absl::Status ExposeStreamingOutput( VLOG(5) << "Exposing output port valid: " << name_or_none(output.valid_port); VLOG(5) << "Exposing output port ready: " << name_or_none(output.ready_port); + if (!output.ready_port.has_value()) { + return absl::UnimplementedError( + "Channels with flow control other than ready_valid are not yet " + "supported."); + } XLS_RET_CHECK(output.data_port.has_value()); XLS_RET_CHECK(output.ready_port.has_value()); XLS_RET_CHECK(output.valid_port.has_value()); @@ -246,6 +261,11 @@ absl::Status ExposeStreamingOutput( absl::Status ExposeStreamingInput( StreamingChannel* channel, const ChannelPortMetadata& input, Block* block, ::xls::BlockInstantiation* block_instantiation) { + if (!input.ready_port.has_value()) { + return absl::UnimplementedError( + "Channels with flow control other than ready_valid are not yet " + "supported."); + } XLS_RET_CHECK(input.data_port.has_value()); XLS_RET_CHECK(input.ready_port.has_value()); XLS_RET_CHECK(input.valid_port.has_value()); diff --git a/xls/codegen_v_1_5/proc_lowering_block_eval_psc_test.cc b/xls/codegen_v_1_5/proc_lowering_block_eval_psc_test.cc index f39632c42a..244a3cf60e 100644 --- a/xls/codegen_v_1_5/proc_lowering_block_eval_psc_test.cc +++ b/xls/codegen_v_1_5/proc_lowering_block_eval_psc_test.cc @@ -798,6 +798,12 @@ absl::StatusOr EvalBlock( // Only process streaming channels in this block continue; } + if (!metadata.data_port.has_value() || !metadata.valid_port.has_value() || + !metadata.ready_port.has_value()) { + return absl::UnimplementedError( + "Channels with flow control other than ready_valid are not yet " + "supported."); + } // If this channel isn't in the input, skip it. if (!inputs.contains(channel_name)) { continue; @@ -845,6 +851,12 @@ absl::StatusOr EvalBlock( // Only process streaming channels in this block continue; } + if (!metadata.data_port.has_value() || !metadata.valid_port.has_value() || + !metadata.ready_port.has_value()) { + return absl::UnimplementedError( + "Channels with flow control other than ready_valid are not yet " + "supported."); + } sinks.push_back(ChannelSink( *metadata.data_port, *metadata.valid_port, *metadata.ready_port, 0.5, block, ChannelSink::BehaviorDuringReset::kIgnoreValid)); @@ -880,6 +892,12 @@ absl::StatusOr EvalBlock( // Only process streaming channels in this block continue; } + if (!metadata.data_port.has_value() || !metadata.valid_port.has_value() || + !metadata.ready_port.has_value()) { + return absl::UnimplementedError( + "Channels with flow control other than ready_valid are not yet " + "supported."); + } if (!actual_outputs.contains(*metadata.data_port)) { continue; } diff --git a/xls/codegen_v_1_5/signature_generation_pass.cc b/xls/codegen_v_1_5/signature_generation_pass.cc index 239b06124f..e9a0d9552a 100644 --- a/xls/codegen_v_1_5/signature_generation_pass.cc +++ b/xls/codegen_v_1_5/signature_generation_pass.cc @@ -25,6 +25,7 @@ #include "absl/base/casts.h" #include "absl/log/check.h" #include "absl/log/log.h" +#include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_format.h" #include "xls/codegen/codegen_options.h" @@ -151,6 +152,12 @@ absl::StatusOr GenerateSignature( : verilog::ChannelDirectionProto::CHANNEL_DIRECTION_RECEIVE; switch (metadata.channel_kind) { case ChannelKind::kStreaming: { + if (metadata.ready_port.has_value() != + metadata.valid_port.has_value()) { + return absl::UnimplementedError( + "Channels with flow control other than ready_valid or none are " + "not yet supported."); + } FlowControl flow_control = (metadata.ready_port.has_value() && metadata.valid_port.has_value()) ? FlowControl::kReadyValid diff --git a/xls/ir/channel.cc b/xls/ir/channel.cc index 273408980a..9409b24139 100644 --- a/xls/ir/channel.cc +++ b/xls/ir/channel.cc @@ -262,6 +262,8 @@ std::string FlowControlToString(FlowControl fc) { return "none"; case FlowControl::kReadyValid: return "ready_valid"; + case FlowControl::kValidData: + return "valid_data"; } LOG(FATAL) << "Invalid flow control value: " << static_cast(fc); } @@ -273,8 +275,11 @@ absl::StatusOr StringToFlowControl(std::string_view str) { if (str == "ready_valid") { return FlowControl::kReadyValid; } + if (str == "valid_data") { + return FlowControl::kValidData; + } return absl::InvalidArgumentError( - absl::StrFormat("Invalid channel kind '%s'", str)); + absl::StrFormat("Invalid flow control '%s'", str)); } std::ostream& operator<<(std::ostream& os, FlowControl fc) { diff --git a/xls/ir/channel.h b/xls/ir/channel.h index c1d2dc25e5..385451bb5e 100644 --- a/xls/ir/channel.h +++ b/xls/ir/channel.h @@ -301,6 +301,15 @@ enum class FlowControl : uint8_t { // signal is valid. When both ready and valid are asserted a transaction // occurs. kReadyValid, + + // The channel uses a valid-data handshake. A valid signal indicates that the + // data is valid, and the receiver is presumed to always be ready to accept + // data. A transaction occurs whenever valid is asserted. + // + // This is used when the channel doesn't want to support backpressure. Safe + // use requires ensuring that the receiver can always accept data when the + // sender is sending, and can optionally be checked with an assert. + kValidData, }; std::string FlowControlToString(FlowControl fc);