Skip to content

Commit

Permalink
Enable channel arrays to be used at the IR conversion boundary.
Browse files Browse the repository at this point in the history 
Channels that begin their existence via the `chan` constructor somewhere in the converted DSLX code were working before this change, and continue working the same way. They are encountered as `ChannelDecl` objects, which trigger the definition of a `ChannelOrArray` in the `ChannelScope`, where subsequent `Index` operations can resolve them.

However, channels that are at the boundary have only `Param` objects and no `ChannelDecl` representing them in the AST. They were previously instantiated based on the `Param` in ir_converter.cc using older, array-unaware code. This change moves their instantiation into `ChannelScope` so that arrays will work.

Context: google#1657

Fixes google#1657

PiperOrigin-RevId: 684617481
  • Loading branch information
@richmckeever @copybara-github
richmckeever authored and copybara-github committed Oct 10, 2024
1 parent 83d5b9e commit 6456292
Show file tree
Hide file tree
Showing 9 changed files with 277 additions and 78 deletions.
8 changes: 4 additions & 4 deletions xls/dslx/ir_convert/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,14 @@ cc_library(
"//xls/common:visitor",
"//xls/common/status:ret_check",
"//xls/common/status:status_macros",
"//xls/dslx:channel_direction",
"//xls/dslx:constexpr_evaluator",
"//xls/dslx:import_data",
"//xls/dslx:interp_value",
"//xls/dslx:interp_value_utils",
"//xls/dslx/frontend:ast",
"//xls/dslx/type_system:parametric_env",
"//xls/dslx/type_system:type",
"//xls/dslx/type_system:type_info",
"//xls/ir",
"//xls/ir:channel",
Expand All @@ -68,8 +70,10 @@ cc_test(
deps = [
":channel_scope",
":conversion_info",
"//xls/common:proto_test_utils",
"//xls/common:xls_gunit_main",
"//xls/common/status:matchers",
"//xls/dslx:channel_direction",
"//xls/dslx:create_import_data",
"//xls/dslx:import_data",
"//xls/dslx/frontend:ast",
Expand Down Expand Up @@ -322,12 +326,10 @@ cc_library(
":convert_options",
":extract_conversion_order",
":function_converter",
":ir_conversion_utils",
":proc_config_ir_converter",
"//xls/common/file:filesystem",
"//xls/common/status:ret_check",
"//xls/common/status:status_macros",
"//xls/dslx:channel_direction",
"//xls/dslx:command_line_utils",
"//xls/dslx:constexpr_evaluator",
"//xls/dslx:create_import_data",
Expand All @@ -346,8 +348,6 @@ cc_library(
"//xls/dslx/type_system:type_info",
"//xls/dslx/type_system:typecheck_module",
"//xls/ir",
"//xls/ir:channel",
"//xls/ir:channel_ops",
"//xls/ir:function_builder",
"//xls/ir:ir_scanner",
"//xls/ir:value",
Expand Down
123 changes: 97 additions & 26 deletions xls/dslx/ir_convert/channel_scope.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,16 @@
#include "xls/common/status/ret_check.h"
#include "xls/common/status/status_macros.h"
#include "xls/common/visitor.h"
#include "xls/dslx/channel_direction.h"
#include "xls/dslx/constexpr_evaluator.h"
#include "xls/dslx/frontend/ast.h"
#include "xls/dslx/import_data.h"
#include "xls/dslx/interp_value.h"
#include "xls/dslx/interp_value_utils.h"
#include "xls/dslx/ir_convert/conversion_info.h"
#include "xls/dslx/ir_convert/ir_conversion_utils.h"
#include "xls/dslx/type_system/parametric_env.h"
#include "xls/dslx/type_system/type.h"
#include "xls/dslx/type_system/type_info.h"
#include "xls/ir/channel.h"
#include "xls/ir/channel_ops.h"
Expand All @@ -64,33 +67,107 @@ ChannelScope::ChannelScope(PackageConversionData* conversion_info,

absl::StatusOr<ChannelOrArray> ChannelScope::DefineChannelOrArray(
const ChannelDecl* decl) {
VLOG(4) << "ChannelScope::HandleChannelDecl: " << decl->ToString();
VLOG(4) << "ChannelScope::DefineChannelOrArray: " << decl->ToString();
CHECK(function_context_.has_value());
XLS_ASSIGN_OR_RETURN(std::string base_channel_name,
CreateBaseChannelName(decl));
XLS_ASSIGN_OR_RETURN(
InterpValue name_interp_value,
ConstexprEvaluator::EvaluateToValue(
import_data_, function_context_->type_info,
/*warning_collector=*/nullptr, function_context_->bindings,
&decl->channel_name_expr()));
XLS_ASSIGN_OR_RETURN(std::string short_name,
InterpValueAsString(name_interp_value));
XLS_ASSIGN_OR_RETURN(xls::Type * type, GetChannelType(decl));
XLS_ASSIGN_OR_RETURN(std::optional<FifoConfig> fifo_config,
CreateFifoConfig(decl));
XLS_ASSIGN_OR_RETURN(
ChannelOrArray channel_or_array,
DefineChannelOrArrayInternal(short_name, ChannelOps::kSendReceive, type,
std::move(fifo_config), decl->dims()));
decl_to_channel_or_array_[decl] = channel_or_array;
return channel_or_array;
}

absl::StatusOr<ChannelOrArray> ChannelScope::DefineChannelOrArrayInternal(
std::string_view short_name, ChannelOps ops, xls::Type* type,
std::optional<FifoConfig> fifo_config,
const std::optional<std::vector<Expr*>>& dims) {
XLS_ASSIGN_OR_RETURN(std::string base_channel_name,
CreateBaseChannelName(short_name));
std::vector<std::string> channel_names;
if (!decl->dims().has_value()) {
XLS_ASSIGN_OR_RETURN(Channel * channel,
CreateChannel(base_channel_name, type, fifo_config));
decl_to_channel_or_array_[decl] = channel;
if (!dims.has_value()) {
XLS_ASSIGN_OR_RETURN(
Channel * channel,
CreateChannel(base_channel_name, ops, type, fifo_config));
return channel;
}
ChannelArray* array = &arrays_.emplace_back(ChannelArray(base_channel_name));
XLS_ASSIGN_OR_RETURN(std::vector<std::string> suffixes,
CreateAllArrayElementSuffixes(*decl->dims()));
CreateAllArrayElementSuffixes(*dims));
for (const std::string& suffix : suffixes) {
std::string channel_name = absl::StrCat(base_channel_name, "__", suffix);
XLS_ASSIGN_OR_RETURN(Channel * channel,
CreateChannel(channel_name, type, fifo_config));
CreateChannel(channel_name, ops, type, fifo_config));
array->AddChannel(channel_name, channel);
}
decl_to_channel_or_array_[decl] = array;
return array;
}

absl::StatusOr<ChannelOrArray> ChannelScope::DefineBoundaryChannelOrArray(
const Param* param, TypeInfo* type_info) {
VLOG(4) << "ChannelScope::DefineBoundaryChannelOrArray: "
<< param->ToString();
auto* type_annot =
dynamic_cast<ChannelTypeAnnotation*>(param->type_annotation());
XLS_RET_CHECK(type_annot != nullptr);
std::optional<Type*> type = type_info->GetItem(type_annot->payload());
XLS_RET_CHECK(type.has_value());
XLS_ASSIGN_OR_RETURN(
xls::Type * ir_type,
TypeToIr(conversion_info_->package.get(), **type, ParametricEnv()));
ChannelOps op = type_annot->direction() == ChannelDirection::kIn
? ChannelOps::kReceiveOnly
: ChannelOps::kSendOnly;
XLS_ASSIGN_OR_RETURN(ChannelOrArray channel_or_array,
DefineChannelOrArrayInternal(
param->identifier(), op, ir_type,
/*fifo_config=*/std::nullopt, type_annot->dims()));
XLS_RETURN_IF_ERROR(DefineProtoChannelOrArray(channel_or_array, type_annot,
ir_type, type_info));
return channel_or_array;
}

absl::Status ChannelScope::DefineProtoChannelOrArray(
ChannelOrArray channel_or_array, dslx::ChannelTypeAnnotation* type_annot,
xls::Type* ir_type, TypeInfo* type_info) {
if (std::holds_alternative<ChannelArray*>(channel_or_array)) {
auto* array = std::get<ChannelArray*>(channel_or_array);
for (const std::string& name : array->flattened_names_in_order()) {
std::optional<Channel*> channel = array->FindChannel(name);
XLS_RET_CHECK(channel.has_value());
XLS_RETURN_IF_ERROR(
DefineProtoChannelOrArray(*channel, type_annot, ir_type, type_info));
}
return absl::OkStatus();
}
Channel* channel = std::get<Channel*>(channel_or_array);
PackageInterfaceProto::Channel* proto_chan =
conversion_info_->interface.add_channels();
*proto_chan->mutable_name() = channel->name();
*proto_chan->mutable_type() = ir_type->ToProto();
// Channels at the boundary only have one direction, with the other direction
// being used externally to the DSLX code.
proto_chan->set_direction(type_annot->direction() == ChannelDirection::kIn
? PackageInterfaceProto::Channel::IN
: PackageInterfaceProto::Channel::OUT);
XLS_ASSIGN_OR_RETURN(std::optional<std::string> first_sv_type,
type_info->FindSvType(type_annot->payload()));
if (first_sv_type) {
*proto_chan->mutable_sv_type() = *first_sv_type;
}
return absl::OkStatus();
}

absl::StatusOr<ChannelOrArray>
ChannelScope::AssociateWithExistingChannelOrArray(const NameDef* name_def,
const ChannelDecl* decl) {
Expand All @@ -110,7 +187,8 @@ absl::Status ChannelScope::AssociateWithExistingChannelOrArray(
const NameDef* name_def, ChannelOrArray channel_or_array) {
VLOG(4) << "ChannelScope::AssociateWithExistingChannelOrArray : "
<< name_def->ToString() << " -> "
<< GetBaseNameForChannelOrArray(channel_or_array);
<< GetBaseNameForChannelOrArray(channel_or_array) << " (array: "
<< std::holds_alternative<ChannelArray*>(channel_or_array) << ")";
name_def_to_channel_or_array_[name_def] = channel_or_array;
return absl::OkStatus();
}
Expand Down Expand Up @@ -200,24 +278,17 @@ ChannelScope::CreateAllArrayElementSuffixes(const std::vector<Expr*>& dims) {
}

absl::StatusOr<std::string> ChannelScope::CreateBaseChannelName(
const ChannelDecl* decl) {
XLS_ASSIGN_OR_RETURN(
InterpValue name_interp_value,
ConstexprEvaluator::EvaluateToValue(
import_data_, function_context_->type_info,
/*warning_collector=*/nullptr, function_context_->bindings,
&decl->channel_name_expr()));
XLS_ASSIGN_OR_RETURN(std::string base_channel_name,
InterpValueAsString(name_interp_value));
std::string_view short_name) {
return channel_name_uniquer_.GetSanitizedUniqueName(
absl::StrCat(conversion_info_->package->name(), "__", base_channel_name));
absl::StrCat(conversion_info_->package->name(), "__", short_name));
}

absl::StatusOr<xls::Type*> ChannelScope::GetChannelType(
const ChannelDecl* decl) const {
auto maybe_type = function_context_->type_info->GetItem(decl->type());
XLS_RET_CHECK(maybe_type.has_value());
return TypeToIr(conversion_info_->package.get(), *maybe_type.value(),
std::optional<Type*> type =
function_context_->type_info->GetItem(decl->type());
XLS_RET_CHECK(type.has_value());
return TypeToIr(conversion_info_->package.get(), **type,
function_context_->bindings);
}

Expand Down Expand Up @@ -259,10 +330,10 @@ absl::StatusOr<std::optional<FifoConfig>> ChannelScope::CreateFifoConfig(
}

absl::StatusOr<Channel*> ChannelScope::CreateChannel(
std::string_view name, xls::Type* type,
std::string_view name, ChannelOps ops, xls::Type* type,
std::optional<FifoConfig> fifo_config) {
return conversion_info_->package->CreateStreamingChannel(
name, ChannelOps::kSendReceive, type,
name, ops, type,
/*initial_values=*/{},
/*fifo_config=*/fifo_config);
}
Expand Down
33 changes: 31 additions & 2 deletions xls/dslx/ir_convert/channel_scope.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@
#include "absl/container/flat_hash_map.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/types/span.h"
#include "xls/dslx/frontend/ast.h"
#include "xls/dslx/import_data.h"
#include "xls/dslx/ir_convert/conversion_info.h"
#include "xls/dslx/type_system/parametric_env.h"
#include "xls/dslx/type_system/type_info.h"
#include "xls/ir/channel.h"
#include "xls/ir/channel_ops.h"
#include "xls/ir/name_uniquer.h"
#include "xls/ir/package.h"
#include "xls/ir/type.h"
Expand All @@ -52,7 +54,12 @@ class ChannelArray {

std::string_view base_channel_name() const { return base_channel_name_; }

absl::Span<const std::string> flattened_names_in_order() const {
return flattened_names_in_order_;
}

void AddChannel(std::string_view flattened_name, Channel* channel) {
flattened_names_in_order_.push_back(std::string(flattened_name));
flattened_name_to_channel_.emplace(flattened_name, channel);
}

Expand All @@ -68,6 +75,11 @@ class ChannelArray {
// in the DSLX source code.
const std::string base_channel_name_;

// The flattened names in order of addition. The scope adds channels in
// ascending index order, and in some situations wants to enumerate them in
// that order.
std::vector<std::string> flattened_names_in_order_;

// Each element in this map represents one element of the array. The flattened
// channel name for an element is `base_channel_name_` plus a suffix produced
// by `ChannelScope::CreateAllArrayElementSuffixes()`, with the base name and
Expand Down Expand Up @@ -95,6 +107,12 @@ class ChannelScope {
// given `decl`, and returns it.
absl::StatusOr<ChannelOrArray> DefineChannelOrArray(const ChannelDecl* decl);

// Creates the channel object, or array of channel objects, indicated by the
// given parameter at the overall DSLX->IR conversion boundary. Channels at
// the boundary have only a send or receive side.
absl::StatusOr<ChannelOrArray> DefineBoundaryChannelOrArray(
const Param* param, TypeInfo* type_info);

// Associates `name_def` as an alias of the channel or array defined by the
// given `decl` that was previously passed to `DefineChannelOrArray`. This
// should be used, for example, when a channel is passed into `spawn` and the
Expand Down Expand Up @@ -123,6 +141,15 @@ class ChannelScope {
absl::StatusOr<Channel*> GetChannelForArrayIndex(const Index* index);

private:
absl::StatusOr<ChannelOrArray> DefineChannelOrArrayInternal(
std::string_view short_name, ChannelOps ops, xls::Type* type,
std::optional<FifoConfig> fifo_config,
const std::optional<std::vector<Expr*>>& dims);

absl::Status DefineProtoChannelOrArray(
ChannelOrArray array, dslx::ChannelTypeAnnotation* type_annot,
xls::Type* ir_type, TypeInfo* type_info);

absl::StatusOr<std::string_view> GetBaseNameForNameDef(
const NameDef* name_def);

Expand All @@ -132,14 +159,16 @@ class ChannelScope {
absl::StatusOr<std::vector<std::string>> CreateAllArrayElementSuffixes(
const std::vector<Expr*>& dims);

absl::StatusOr<std::string> CreateBaseChannelName(const ChannelDecl* decl);
absl::StatusOr<std::string> CreateBaseChannelName(
std::string_view short_name);

absl::StatusOr<xls::Type*> GetChannelType(const ChannelDecl* decl) const;

absl::StatusOr<std::optional<FifoConfig>> CreateFifoConfig(
const ChannelDecl* decl) const;

absl::StatusOr<Channel*> CreateChannel(std::string_view name, xls::Type* type,
absl::StatusOr<Channel*> CreateChannel(std::string_view name, ChannelOps ops,
xls::Type* type,
std::optional<FifoConfig> fifo_config);

absl::StatusOr<Channel*> GetChannelArrayElement(
Expand Down
Loading

0 comments on commit 6456292

Please sign in to comment.