Skip to content

[codegen 1.5] Enable all passing block conversion pipeline tests #3631

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
copybara-service wants to merge 1 commit into
base: main
Choose a base branch
from
+306 −134
Draft

[codegen 1.5] Enable all passing block conversion pipeline tests #3631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions xls/codegen_v_1_5/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,9 +321,9 @@ cc_library(
"//xls/common/status:ret_check",
"//xls/common/status:status_macros",
"//xls/ir",
"//xls/ir:node_util",
"//xls/ir:op",
"//xls/ir:register",
"//xls/ir:value",
"//xls/ir:value_utils",
"//xls/passes:pass_base",
"@com_google_absl//absl/algorithm:container",
Expand Down Expand Up @@ -630,7 +630,6 @@ cc_test(
"//xls/common/status:matchers",
"//xls/common/status:ret_check",
"//xls/common/status:status_macros",
"//xls/estimators/delay_model:delay_estimator",
"//xls/interpreter:block_evaluator",
"//xls/interpreter:ir_interpreter",
"//xls/ir",
Expand Down
228 changes: 159 additions & 69 deletions xls/codegen_v_1_5/block_conversion_pass_pipeline_test.cc
View file
Open in desktop

Large diffs are not rendered by default.

62 changes: 43 additions & 19 deletions xls/codegen_v_1_5/channel_to_port_io_lowering_pass.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,9 +318,14 @@ absl::StatusOr<Connector> AddPortsForSend(
.ready = ready};

XLS_RETURN_IF_ERROR(block->AddChannelPortMetadata(
channel, ChannelDirection::kSend, connector.data->GetName(),
GetOptionalNodeName(connector.valid),
GetOptionalNodeName(connector.ready)));
ChannelPortMetadata{.channel_name = std::string(ChannelRefName(channel)),
.type = ChannelRefType(channel),
.direction = ChannelDirection::kSend,
.channel_kind = ChannelRefKind(channel),
.flop_kind = FlopKind::kNone,
.data_port = data->GetName(),
.valid_port = GetOptionalNodeName(valid),
.ready_port = GetOptionalNodeName(ready)}));

return connector;
}
Expand Down Expand Up @@ -374,9 +379,14 @@ absl::StatusOr<Connector> AddPortsForReceive(
.ready = ready};

XLS_RETURN_IF_ERROR(block->AddChannelPortMetadata(
channel, ChannelDirection::kReceive, connector.data->GetName(),
GetOptionalNodeName(connector.valid),
GetOptionalNodeName(connector.ready)));
ChannelPortMetadata{.channel_name = std::string(ChannelRefName(channel)),
.type = ChannelRefType(channel),
.direction = ChannelDirection::kReceive,
.channel_kind = ChannelRefKind(channel),
.flop_kind = FlopKind::kNone,
.data_port = data->GetName(),
.valid_port = GetOptionalNodeName(valid),
.ready_port = GetOptionalNodeName(ready)}));

return connector;
}
Expand Down Expand Up @@ -697,9 +707,11 @@ absl::Status ConnectReceivesToConnector(
absl::MakeConstSpan({*connector.valid, recv_inactive}),
Op::kOr));
}
XLS_RETURN_IF_ERROR(
ReplaceWithAnd(stage.active_inputs_valid(), recv_valid_or_inactive)
.status());

XLS_RETURN_IF_ERROR(ReplaceWithAnd(stage.active_inputs_valid(),
recv_valid_or_inactive,
/*combine_literals=*/false)
.status());
}

Node* data = connector.data;
Expand Down Expand Up @@ -753,13 +765,10 @@ absl::Status ConnectReceivesToConnector(
XLS_RETURN_IF_ERROR(block->RemoveNode(receive));
}
if (connector.ready.has_value()) {
if (ready_signals.size() == 1) {
XLS_RETURN_IF_ERROR(connector.ReplaceReadySignal(ready_signals.front()));
} else {
XLS_ASSIGN_OR_RETURN(Node * ready_signal,
JoinWithOr(block, ready_signals));
XLS_RETURN_IF_ERROR(connector.ReplaceReadySignal(ready_signal));
}
XLS_ASSIGN_OR_RETURN(
Node * ready_signal,
JoinWithOr(block, ready_signals, /*combine_literals=*/false));
XLS_RETURN_IF_ERROR(connector.ReplaceReadySignal(ready_signal));
}

return absl::OkStatus();
Expand Down Expand Up @@ -840,9 +849,17 @@ absl::Status ConnectSendsToConnector(
absl::MakeConstSpan({*connector.ready, send_inactive}),
Op::kOr));
}
XLS_RETURN_IF_ERROR(
ReplaceWithAnd(stage.outputs_valid(), send_done_or_inactive)
.status());

if (stage.outputs_valid()->Is<Literal>() &&
stage.outputs_valid()->As<Literal>()->value().IsAllOnes()) {
XLS_RETURN_IF_ERROR(
stage.outputs_valid()->ReplaceUsesWith(send_done_or_inactive));
} else {
XLS_RETURN_IF_ERROR(ReplaceWithAnd(stage.outputs_valid(),
send_done_or_inactive,
/*combine_literals=*/false)
.status());
}
}

if (connector.valid.has_value()) {
Expand Down Expand Up @@ -1607,6 +1624,13 @@ absl::StatusOr<bool> LowerIoToPorts(
needs_one_shot_logic = !outputs_mutually_exclusive;
}
if (needs_one_shot_logic) {
if (options.codegen_options.generate_combinational()) {
return absl::UnimplementedError(absl::StrFormat(
"Proc combinational generator only supports streaming output "
"channels which can be determined to be mutually exclusive, got %d "
"output channels which were not proven to be mutually exclusive",
outgoing_channel_count));
}
for (const auto& [directed_channel, _] : io_ops) {
if (directed_channel.second != ChannelDirection::kSend) {
continue;
Expand Down
23 changes: 4 additions & 19 deletions xls/codegen_v_1_5/function_io_lowering_pass.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,32 +35,17 @@
#include "xls/ir/block.h"
#include "xls/ir/function_base.h"
#include "xls/ir/node.h"
#include "xls/ir/node_util.h"
#include "xls/ir/nodes.h"
#include "xls/ir/op.h"
#include "xls/ir/package.h"
#include "xls/ir/register.h"
#include "xls/ir/value.h"
#include "xls/ir/value_utils.h"
#include "xls/passes/pass_base.h"

namespace xls::codegen {
namespace {

absl::StatusOr<Node*> ReplaceWithAnd(Node* original, Node* new_operand) {
if (original->Is<Literal>() && original->As<Literal>()->value().IsAllOnes()) {
XLS_RETURN_IF_ERROR(original->ReplaceUsesWith(new_operand));
return new_operand;
}
if (original->Is<NaryOp>() && original->As<NaryOp>()->op() == Op::kAnd) {
std::vector<Node*> operands(original->operands().begin(),
original->operands().end());
operands.push_back(new_operand);
return original->ReplaceUsesWithNew<NaryOp>(operands, Op::kAnd);
}
return original->ReplaceUsesWithNew<NaryOp>(
absl::MakeConstSpan({original, new_operand}), Op::kAnd);
}

absl::StatusOr<Node*> FlopNode(
ScheduledBlock* block, std::string_view name, Node* input,
int64_t stage_index, std::optional<Node*> load_enable = std::nullopt) {
Expand Down Expand Up @@ -119,9 +104,9 @@ absl::StatusOr<bool> FunctionIOLoweringPass::LowerParams(
// which will stop the pipeline from running new activations when the input
// is invalid. This also automatically propagates validity to the output in
// case an output-valid signal was requested.
XLS_RETURN_IF_ERROR(
ReplaceWithAnd(block->stages()[0].active_inputs_valid(), *input_valid)
.status());
XLS_RETURN_IF_ERROR(ReplaceWithAnd(block->stages()[0].active_inputs_valid(),
*input_valid, /*combine_literals=*/false)
.status());
}

std::vector<Param*> params_to_remove;
Expand Down
78 changes: 72 additions & 6 deletions xls/ir/node_util.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ absl::StatusOr<Node*> NaryNorIfNeeded(FunctionBase* f,

absl::StatusOr<Node*> JoinWithAnd(FunctionBase* f,
absl::Span<Node* const> operands,
std::string_view name,
bool combine_literals, std::string_view name,
std::optional<SourceInfo> loc) {
XLS_RET_CHECK(!operands.empty());
for (Node* operand : operands) {
Expand All @@ -536,9 +536,40 @@ absl::StatusOr<Node*> JoinWithAnd(FunctionBase* f,
}
}

std::optional<Bits> literal_value;
if (combine_literals) {
for (Node* operand : new_operands) {
if (operand->Is<Literal>()) {
Value operand_value = operand->As<Literal>()->value();
CHECK(operand_value.IsBits());
if (!literal_value.has_value()) {
literal_value = operand_value.bits();
} else {
literal_value = bits_ops::And(*literal_value, operand_value.bits());
}
}
}
if (literal_value.has_value() && literal_value->IsZero()) {
return f->MakeNodeWithName<Literal>(loc.value_or(SourceInfo()),
Value(*literal_value), name);
}
if (literal_value.has_value() && literal_value->IsAllOnes()) {
literal_value = std::nullopt;
}
}

std::vector<Node*> unique_operands = RemoveRedundantNodes(
new_operands,
/*drop_literal=*/[](const Value& v) { return v.IsAllOnes(); });
/*drop_literal=*/combine_literals
? std::make_optional([](const Value&) { return true; })
: std::nullopt);
if (literal_value.has_value()) {
XLS_ASSIGN_OR_RETURN(Node * literal,
f->MakeNode<Literal>(loc.value_or(SourceInfo()),
Value(*literal_value)));
unique_operands.push_back(literal);
}

if (unique_operands.empty()) {
return f->MakeNodeWithName<Literal>(
loc.value_or(SourceInfo()), AllOnesOfType(operands.front()->GetType()),
Expand All @@ -561,6 +592,7 @@ absl::StatusOr<Node*> JoinWithAnd(FunctionBase* f,

absl::StatusOr<Node*> ReplaceWithAnd(Node* old_node,
absl::Span<Node* const> new_nodes,
bool combine_literals,
std::string_view name,
std::optional<SourceInfo> loc) {
FunctionBase* f = old_node->function_base();
Expand All @@ -569,7 +601,8 @@ absl::StatusOr<Node*> ReplaceWithAnd(Node* old_node,
operands.push_back(old_node);
absl::c_copy(new_nodes, std::back_inserter(operands));

XLS_ASSIGN_OR_RETURN(Node * replacement, JoinWithAnd(f, operands, name, loc));
XLS_ASSIGN_OR_RETURN(Node * replacement,
JoinWithAnd(f, operands, combine_literals, name, loc));
if (f->IsStaged(old_node) && !f->IsStaged(replacement)) {
XLS_ASSIGN_OR_RETURN(int64_t stage_index,
old_node->function_base()->GetStageIndex(old_node));
Expand All @@ -593,7 +626,7 @@ absl::StatusOr<Node*> ReplaceWithAnd(Node* old_node,

absl::StatusOr<Node*> JoinWithOr(FunctionBase* f,
absl::Span<Node* const> operands,
std::string_view name,
bool combine_literals, std::string_view name,
std::optional<SourceInfo> loc) {
XLS_RET_CHECK(!operands.empty());
for (Node* operand : operands) {
Expand All @@ -610,9 +643,40 @@ absl::StatusOr<Node*> JoinWithOr(FunctionBase* f,
}
}

std::optional<Bits> literal_value;
if (combine_literals) {
for (Node* operand : new_operands) {
if (operand->Is<Literal>()) {
Value operand_value = operand->As<Literal>()->value();
CHECK(operand_value.IsBits());
if (!literal_value.has_value()) {
literal_value = operand_value.bits();
} else {
literal_value = bits_ops::Or(*literal_value, operand_value.bits());
}
}
}
if (literal_value.has_value() && literal_value->IsAllOnes()) {
return f->MakeNodeWithName<Literal>(loc.value_or(SourceInfo()),
Value(*literal_value), name);
}
if (literal_value.has_value() && literal_value->IsZero()) {
literal_value = std::nullopt;
}
}

std::vector<Node*> unique_operands = RemoveRedundantNodes(
new_operands,
/*drop_literal=*/[](const Value& v) { return v.IsAllZeros(); });
/*drop_literal=*/combine_literals
? std::make_optional([](const Value&) { return true; })
: std::nullopt);
if (literal_value.has_value()) {
XLS_ASSIGN_OR_RETURN(Node * literal,
f->MakeNode<Literal>(loc.value_or(SourceInfo()),
Value(*literal_value)));
unique_operands.push_back(literal);
}

if (unique_operands.empty()) {
return f->MakeNodeWithName<Literal>(
loc.value_or(SourceInfo()),
Expand All @@ -635,6 +699,7 @@ absl::StatusOr<Node*> JoinWithOr(FunctionBase* f,

absl::StatusOr<Node*> ReplaceWithOr(Node* old_node,
absl::Span<Node* const> new_nodes,
bool combine_literals,
std::string_view name,
std::optional<SourceInfo> loc) {
FunctionBase* f = old_node->function_base();
Expand All @@ -643,7 +708,8 @@ absl::StatusOr<Node*> ReplaceWithOr(Node* old_node,
operands.push_back(old_node);
absl::c_copy(new_nodes, std::back_inserter(operands));

XLS_ASSIGN_OR_RETURN(Node * replacement, JoinWithOr(f, operands, name, loc));
XLS_ASSIGN_OR_RETURN(Node * replacement,
JoinWithOr(f, operands, combine_literals, name, loc));
if (f->IsStaged(old_node) && !f->IsStaged(replacement)) {
XLS_ASSIGN_OR_RETURN(int64_t stage_index,
old_node->function_base()->GetStageIndex(old_node));
Expand Down
42 changes: 24 additions & 18 deletions xls/ir/node_util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,33 +282,35 @@ absl::StatusOr<Node*> NaryNorIfNeeded(FunctionBase* f,

// Build a node representing `AND(operands...)`, expressing it as (locally)
// efficiently as possible. This will merge & deduplicate the nodes' operands if
// any node is an AND, drop any operands that are literal all-ones, and
// construct the AND of the remaining operands.
// any node is an AND, combine all literals into at most one literal (unless
// requested not to), and construct the AND of the remaining operands.
//
// If `source_info` is provided, the result will have that source info.
// Otherwise, the result will have the merged source infos of the operands.
absl::StatusOr<Node*> JoinWithAnd(FunctionBase* f,
absl::Span<Node* const> operands,
bool combine_literals = true,
std::string_view name = "",
std::optional<SourceInfo> loc = std::nullopt);

// Build a node representing `OR(operands...)`, expressing it as (locally)
// efficiently as possible. This will merge & deduplicate the nodes' operands if
// any node is an OR, drop any operands that are literal zeroes, and construct
// the OR of the remaining operands.
// any node is an OR, combine all literals into at most one literal (unless
// requested not to), and construct the OR of the remaining operands.
//
// If `source_info` is provided, the result will have that source info.
// Otherwise, the result will have the merged source infos of the operands.
absl::StatusOr<Node*> JoinWithOr(FunctionBase* f,
absl::Span<Node* const> operands,
bool combine_literals = true,
std::string_view name = "",
std::optional<SourceInfo> loc = std::nullopt);

// Replaces all uses of `old_node` with `old_node AND new_nodes...`, expressing
// it as efficiently as possible. This will merge & deduplicate the nodes'
// operands if either node is an AND, drop any operands that are literal
// all-ones, and construct the AND of the remaining operands, replacing
// `old_node` with this new result.
// operands if either node is an AND, combine all literals into at most one
// literal (unless requested not to), and construct the AND of the remaining
// operands, replacing `old_node` with this new result.
//
// - If a name is provided, the result will have that name.
// - If no name is provided, the result will take `old_node`'s name if it has
Expand All @@ -320,18 +322,20 @@ absl::StatusOr<Node*> JoinWithOr(FunctionBase* f,
// Also, if `old_node` is staged, the result will be placed in the same stage.
absl::StatusOr<Node*> ReplaceWithAnd(
Node* old_node, absl::Span<Node* const> new_nodes,
std::string_view name = "", std::optional<SourceInfo> loc = std::nullopt);
bool combine_literals = true, std::string_view name = "",
std::optional<SourceInfo> loc = std::nullopt);
inline absl::StatusOr<Node*> ReplaceWithAnd(
Node* old_node, Node* new_node, std::string_view name = "",
std::optional<SourceInfo> loc = std::nullopt) {
return ReplaceWithAnd(old_node, absl::MakeConstSpan({new_node}), name, loc);
Node* old_node, Node* new_node, bool combine_literals = true,
std::string_view name = "", std::optional<SourceInfo> loc = std::nullopt) {
return ReplaceWithAnd(old_node, absl::MakeConstSpan({new_node}),
combine_literals, name, loc);
}

// Replaces all uses of `old_node` with `old_node OR new_nodes...`, expressing
// it as efficiently as possible. This will merge & deduplicate the nodes'
// operands if either node is an OR, drop any operands that are literal zeroes,
// and construct the OR of the remaining operands, replacing `old_node` with
// this new result.
// operands if either node is an OR, combine all literals into at most one
// literal (unless requested not to), and construct the OR of the remaining
// operands, replacing `old_node` with this new result.
//
// - If a name is provided, the result will have that name.
// - If no name is provided, the result will take `old_node`'s name if it has
Expand All @@ -343,11 +347,13 @@ inline absl::StatusOr<Node*> ReplaceWithAnd(
// Also, if `old_node` is staged, the result will be placed in the same stage.
absl::StatusOr<Node*> ReplaceWithOr(
Node* old_node, absl::Span<Node* const> new_nodes,
std::string_view name = "", std::optional<SourceInfo> loc = std::nullopt);
bool combine_literals = true, std::string_view name = "",
std::optional<SourceInfo> loc = std::nullopt);
inline absl::StatusOr<Node*> ReplaceWithOr(
Node* old_node, Node* new_node, std::string_view name = "",
std::optional<SourceInfo> loc = std::nullopt) {
return ReplaceWithOr(old_node, absl::MakeConstSpan({new_node}), name, loc);
Node* old_node, Node* new_node, bool combine_literals = true,
std::string_view name = "", std::optional<SourceInfo> loc = std::nullopt) {
return ReplaceWithOr(old_node, absl::MakeConstSpan({new_node}),
combine_literals, name, loc);
}

// Returns whether the given node is a signed/unsigned comparison operation (for
Expand Down
Loading