Skip to content

[codegen 1.5] Fix handling of Functions as Top in the presence of Procs #3696

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

Closed
copybara-service wants to merge 1 commit into from
Closed

[codegen 1.5] Fix handling of Functions as Top in the presence of Procs #3696

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
109 changes: 48 additions & 61 deletions xls/codegen_v_1_5/block_conversion_pass_pipeline_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#include <string>
#include <string_view>
#include <tuple>
#include <utility>
#include <vector>

#include "gmock/gmock.h"
Expand All @@ -46,7 +45,6 @@
#include "xls/codegen/block_conversion_test_fixture.h"
#include "xls/codegen/codegen_options.h"
#include "xls/codegen/codegen_pass.h"
#include "xls/codegen/codegen_result.h"
#include "xls/codegen/ram_configuration.h"
#include "xls/codegen_v_1_5/convert_to_block.h"
#include "xls/common/casts.h"
Expand All @@ -69,15 +67,13 @@
#include "xls/ir/nodes.h"
#include "xls/ir/op.h"
#include "xls/ir/proc.h"
#include "xls/ir/proc_elaboration.h"
#include "xls/ir/register.h"
#include "xls/ir/source_location.h"
#include "xls/ir/value.h"
#include "xls/ir/verifier.h"
#include "xls/scheduling/pipeline_schedule.h"
#include "xls/scheduling/run_pipeline_schedule.h"
#include "xls/scheduling/scheduling_options.h"
#include "xls/scheduling/scheduling_result.h"
#include "xls/tools/codegen.h"
#include "xls/tools/codegen_flags.pb.h"
#include "xls/tools/scheduling_options_flags.pb.h"
Expand Down Expand Up @@ -6799,27 +6795,25 @@ TEST_F(BlockConversionTest, TwoBitSelectorAllCasesPopulated) {
TEST_F(ProcConversionTestFixture, SimpleMultiProcConversion) {
XLS_ASSERT_OK_AND_ASSIGN(std::unique_ptr<Package> p,
CreateMultiProcPackage());
SchedulingOptionsFlagsProto scheduling_options;
scheduling_options.set_pipeline_stages(2);
scheduling_options.set_delay_model("unit");
scheduling_options.set_multi_proc(true);
SchedulingOptions scheduling_options;
scheduling_options.pipeline_stages(2);
scheduling_options.delay_model("unit");
scheduling_options.schedule_all_procs(true);

CodegenFlagsProto codegen_options;
codegen_options.set_flop_inputs(false);
codegen_options.set_flop_outputs(false);
codegen_options.set_reset("rst");
codegen_options.set_streaming_channel_data_suffix("_data");
codegen_options.set_streaming_channel_valid_suffix("_valid");
codegen_options.set_streaming_channel_ready_suffix("_ready");
codegen_options.set_module_name("p");
codegen_options.set_generator(GeneratorKind::GENERATOR_KIND_PIPELINE);
CodegenOptions codegen_options;
codegen_options.clock_name("clk");
codegen_options.flop_inputs(false);
codegen_options.flop_outputs(false);
codegen_options.reset("rst", false, false, false);
codegen_options.streaming_channel_data_suffix("_data");
codegen_options.streaming_channel_valid_suffix("_valid");
codegen_options.streaming_channel_ready_suffix("_ready");
codegen_options.module_name("p");
codegen_options.generate_combinational(false);

std::pair<SchedulingResult, verilog::CodegenResult> result;
XLS_ASSERT_OK_AND_ASSIGN(
result, ScheduleAndCodegen(p.get(), scheduling_options, codegen_options,
/*with_delay_model=*/true));

XLS_ASSERT_OK_AND_ASSIGN(Block * top_block, p->GetBlock("p"));
Block * top_block,
ConvertToBlock(p.get(), codegen_options, scheduling_options));

std::vector<absl::flat_hash_map<std::string, uint64_t>> inputs;
std::vector<absl::flat_hash_map<std::string, uint64_t>> outputs;
Expand Down Expand Up @@ -6897,27 +6891,25 @@ TEST_F(ProcConversionTestFixture,
SimpleMultiProcConversionWithFunctionsPresent) {
XLS_ASSERT_OK_AND_ASSIGN(std::unique_ptr<Package> p,
CreateMultiProcPackage(/*with_functions=*/true));
SchedulingOptionsFlagsProto scheduling_options;
scheduling_options.set_pipeline_stages(2);
scheduling_options.set_delay_model("unit");
scheduling_options.set_multi_proc(true);
SchedulingOptions scheduling_options;
scheduling_options.pipeline_stages(2);
scheduling_options.delay_model("unit");
scheduling_options.schedule_all_procs(true);

CodegenFlagsProto codegen_options;
codegen_options.set_flop_inputs(false);
codegen_options.set_flop_outputs(false);
codegen_options.set_reset("rst");
codegen_options.set_streaming_channel_data_suffix("_data");
codegen_options.set_streaming_channel_valid_suffix("_valid");
codegen_options.set_streaming_channel_ready_suffix("_ready");
codegen_options.set_module_name("p");
codegen_options.set_generator(GeneratorKind::GENERATOR_KIND_PIPELINE);
CodegenOptions codegen_options;
codegen_options.clock_name("clk");
codegen_options.flop_inputs(false);
codegen_options.flop_outputs(false);
codegen_options.reset("rst", false, false, false);
codegen_options.streaming_channel_data_suffix("_data");
codegen_options.streaming_channel_valid_suffix("_valid");
codegen_options.streaming_channel_ready_suffix("_ready");
codegen_options.module_name("p");
codegen_options.generate_combinational(false);

std::pair<SchedulingResult, verilog::CodegenResult> result;
XLS_ASSERT_OK_AND_ASSIGN(
result, ScheduleAndCodegen(p.get(), scheduling_options, codegen_options,
/*with_delay_model=*/true));

XLS_ASSERT_OK_AND_ASSIGN(Block * top_block, p->GetBlock("p"));
Block * top_block,
ConvertToBlock(p.get(), codegen_options, scheduling_options));

std::vector<absl::flat_hash_map<std::string, uint64_t>> inputs;
std::vector<absl::flat_hash_map<std::string, uint64_t>> outputs;
Expand Down Expand Up @@ -6997,27 +6989,25 @@ TEST_F(ProcConversionTestFixture, SimpleFunctionWithProcsPresent) {
XLS_ASSERT_OK_AND_ASSIGN(Function * f0, p->GetFunction("f0"));
XLS_ASSERT_OK(p->SetTop(f0));

SchedulingOptionsFlagsProto scheduling_options;
scheduling_options.set_pipeline_stages(1);
scheduling_options.set_delay_model("unit");
scheduling_options.set_multi_proc(true);
SchedulingOptions scheduling_options;
scheduling_options.pipeline_stages(1);
scheduling_options.delay_model("unit");
scheduling_options.schedule_all_procs(true);

CodegenFlagsProto codegen_options;
codegen_options.set_flop_inputs(false);
codegen_options.set_flop_outputs(false);
codegen_options.set_reset("rst");
codegen_options.set_streaming_channel_data_suffix("_data");
codegen_options.set_streaming_channel_valid_suffix("_valid");
codegen_options.set_streaming_channel_ready_suffix("_ready");
codegen_options.set_module_name("p");
codegen_options.set_generator(GeneratorKind::GENERATOR_KIND_PIPELINE);
CodegenOptions codegen_options;
codegen_options.clock_name("clk");
codegen_options.flop_inputs(false);
codegen_options.flop_outputs(false);
codegen_options.reset("rst", false, false, false);
codegen_options.streaming_channel_data_suffix("_data");
codegen_options.streaming_channel_valid_suffix("_valid");
codegen_options.streaming_channel_ready_suffix("_ready");
codegen_options.module_name("p");
codegen_options.generate_combinational(false);

std::pair<SchedulingResult, verilog::CodegenResult> result;
XLS_ASSERT_OK_AND_ASSIGN(
result, ScheduleAndCodegen(p.get(), scheduling_options, codegen_options,
/*with_delay_model=*/true));

XLS_ASSERT_OK_AND_ASSIGN(Block * top_block, p->GetBlock("p"));
Block * top_block,
ConvertToBlock(p.get(), codegen_options, scheduling_options));

EXPECT_EQ(top_block->name(), "p");
EXPECT_EQ(top_block->GetPorts().size(), 5);
Expand Down Expand Up @@ -7073,9 +7063,6 @@ TEST_F(ProcConversionTestFixture, TrivialProcHierarchyWithProcScopedChannels) {
XLS_ASSERT_OK_AND_ASSIGN(Proc * top, pb.Build({}));
XLS_ASSERT_OK(p->SetTop(top));

XLS_ASSERT_OK_AND_ASSIGN(ProcElaboration elab,
ProcElaboration::Elaborate(top));

XLS_ASSERT_OK(ConvertToBlock(
p.get(),
CodegenOptions().reset("rst", false, false, false).clock_name("clk"),
Expand Down
64 changes: 47 additions & 17 deletions xls/codegen_v_1_5/global_channel_block_stitching_pass.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,15 @@ absl::Status InstantiateBlocksInContainer(
if (block.get() == container) {
continue;
}

// Skip anything that has a function source; this doesn't need to talk over
// channels, so it doesn't need an instantiation.
if (block->IsScheduled() &&
down_cast<ScheduledBlock*>(block.get())->source() != nullptr &&
down_cast<ScheduledBlock*>(block.get())->source()->IsFunction()) {
continue;
}

blocks_to_instantiate.push_back(block.get());
}
absl::c_sort(blocks_to_instantiate, &FunctionBase::NameLessThan);
Expand Down Expand Up @@ -399,10 +408,9 @@ absl::Status StitchChannel(Block* container, Channel* channel,

// Stitch all blocks in the container block together, punching external
// sends/receives through the container.
absl::Status StitchBlocks(Package* package,
absl::Status StitchBlocks(Package* package, Block* stitching_block,
const verilog::CodegenOptions& options) {
XLS_ASSIGN_OR_RETURN(Block * top_block, package->GetTopAsBlock());
XLS_RETURN_IF_ERROR(InstantiateBlocksInContainer(top_block, options));
XLS_RETURN_IF_ERROR(InstantiateBlocksInContainer(stitching_block, options));

XLS_ASSIGN_OR_RETURN(GlobalChannelMap channel_map,
GlobalChannelMap::Create(package));
Expand All @@ -414,7 +422,7 @@ absl::Status StitchBlocks(Package* package,
absl::c_sort(channels_sorted_by_name, Channel::NameLessThan);

for (Channel* channel : channels_sorted_by_name) {
XLS_RETURN_IF_ERROR(StitchChannel(top_block, channel, channel_map));
XLS_RETURN_IF_ERROR(StitchChannel(stitching_block, channel, channel_map));
}

return absl::OkStatus();
Expand All @@ -423,8 +431,9 @@ absl::Status StitchBlocks(Package* package,
// Adds a block with the given name to the package, renaming an existing block
// that has the same name if one exists. Also, update the metadata for the
// existing block if it is renamed.
absl::StatusOr<Block*> AddBlockWithName(Package* package,
std::string_view name) {
absl::StatusOr<Block*> AddBlockWithName(
Package* package, std::string_view name,
bool rename_old_block_on_conflict = true) {
NameUniquer uniquer(/*separator=*/"__", /*reserved_names=*/{});
Block* block_with_name = nullptr;
std::vector<Block*> blocks_sorted_by_name;
Expand All @@ -446,8 +455,12 @@ absl::StatusOr<Block*> AddBlockWithName(Package* package,
if (block_with_name != nullptr) {
std::string new_name = uniquer.GetSanitizedUniqueName(name);
XLS_RET_CHECK_NE(block_with_name->name(), new_name);
block_with_name->SetName(new_name);
XLS_RET_CHECK(block_with_name->IsScheduled());
if (rename_old_block_on_conflict) {
block_with_name->SetName(new_name);
XLS_RET_CHECK(block_with_name->IsScheduled());
} else {
name = new_name;
}
}

auto new_block = std::make_unique<ScheduledBlock>(name, package);
Expand All @@ -474,18 +487,35 @@ absl::StatusOr<bool> GlobalChannelBlockStitchingPass::RunInternal(
}

XLS_ASSIGN_OR_RETURN(Block * original_top_block, package->GetTopAsBlock());
std::string top_block_name(options.codegen_options.module_name().value_or(
original_top_block->name()));

XLS_ASSIGN_OR_RETURN(Block * top_block,
AddBlockWithName(package, top_block_name));
std::string stitching_block_name(
options.codegen_options.module_name().value_or(
original_top_block->name()));
bool stitching_block_is_top = true;
if (original_top_block->IsScheduled() &&
down_cast<ScheduledBlock*>(original_top_block)->source() != nullptr &&
down_cast<ScheduledBlock*>(original_top_block)->source()->IsFunction()) {
// If the top block is a function, we need to leave it alone.
stitching_block_name = absl::StrCat(package->name(), "__stitching_block");
stitching_block_is_top = false;
}

XLS_ASSIGN_OR_RETURN(
Block * stitching_block,
AddBlockWithName(
package, stitching_block_name,
/*rename_old_block_on_conflict=*/stitching_block_is_top));

XLS_RETURN_IF_ERROR(package->SetTop(top_block));
XLS_RETURN_IF_ERROR(top_block->AddClockPort("clk"));
XLS_RETURN_IF_ERROR(MaybeAddResetPort(top_block, options.codegen_options));
if (stitching_block_is_top) {
XLS_RETURN_IF_ERROR(package->SetTop(stitching_block));
}
XLS_RETURN_IF_ERROR(stitching_block->AddClockPort("clk"));
XLS_RETURN_IF_ERROR(
MaybeAddResetPort(stitching_block, options.codegen_options));

VLOG(2) << "Stitching blocks for " << top_block->name();
XLS_RETURN_IF_ERROR(StitchBlocks(package, options.codegen_options));
VLOG(2) << "Stitching blocks for " << stitching_block->name();
XLS_RETURN_IF_ERROR(
StitchBlocks(package, stitching_block, options.codegen_options));

return true;
}
Expand Down
14 changes: 10 additions & 4 deletions xls/codegen_v_1_5/scheduled_block_conversion_pass.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,15 @@ absl::StatusOr<bool> ScheduledBlockConversionPass::RunInternal(
continue;
}

ScheduledBlock* block = down_cast<ScheduledBlock*>(package->AddBlock(
std::make_unique<ScheduledBlock>(old_fb->name(), package)));
std::string_view name = old_fb->name();
if (package->IsTop(old_fb) && old_fb->IsFunction() &&
options.codegen_options.module_name().has_value()) {
// Rename the top function to the module name; we don't do this for procs,
// and instead handle it later while managing proc connections.
name = *options.codegen_options.module_name();
}
ScheduledBlock* block = down_cast<ScheduledBlock*>(
package->AddBlock(std::make_unique<ScheduledBlock>(name, package)));

if (!options.codegen_options.generate_combinational()) {
XLS_RETURN_IF_ERROR(
Expand Down Expand Up @@ -160,8 +167,7 @@ absl::StatusOr<bool> ScheduledBlockConversionPass::RunInternal(
block->MoveLogicFrom(*old_fb);
}

std::optional<FunctionBase*> top = package->GetTop();
if (top.has_value() && top == old_fb) {
if (package->IsTop(old_fb)) {
XLS_RETURN_IF_ERROR(package->SetTop(block));
}

Expand Down
4 changes: 2 additions & 2 deletions xls/scheduling/scheduling_options.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,8 @@ class SchedulingOptions {
int64_t opt_level() const { return opt_level_; }

// Sets/gets the target delay model
SchedulingOptions& delay_model(std::string& value) {
delay_model_ = value;
SchedulingOptions& delay_model(std::string_view value) {
delay_model_ = std::string(value);
return *this;
}
std::optional<std::string> delay_model() const { return delay_model_; }
Expand Down