[codegen 1.5] Enable the simplest proc test for new block conversion

This required small patches to channel & state lowering, as well as to how we transfer Nodes into ScheduledBlocks:

1. Channel lowering now handles single-value channnels without crashing in some circumstances. (Whoops.)
2. State lowering now cleans up after itself.
3. ScheduledBlock conversion now takes ownership of the node name as well as the node.

We also add dataflow simplification to the pipeline (matching codegen 1.0).

PiperOrigin-RevId: 849163225

Author: ericastor

xls/codegen_v_1_5/BUILD

@@ -41,15 +41,20 @@ cc_library(
     hdrs = ["block_conversion_pass_pipeline.h"],
     deps = [
         ":block_conversion_pass",
+        ":block_conversion_wrapper_pass",
         ":block_finalization_pass",
         ":channel_to_port_io_lowering_pass",
         ":flow_control_insertion_pass",
         ":function_io_lowering_pass",
         ":idle_insertion_pass",
         ":pipeline_register_insertion_pass",
+        ":register_cleanup_pass",
         ":scheduled_block_conversion_pass",
         ":scheduling_pass",
         ":state_to_register_io_lowering_pass",
+        "//xls/passes:dataflow_simplification_pass",
+        "//xls/passes:dce_pass",
+        "//xls/passes:optimization_pass",
     ],
 )
 
@@ -93,15 +98,127 @@ cc_test(
     ],
 )
 
+cc_library(
+    name = "register_cleanup_pass",
+    srcs = ["register_cleanup_pass.cc"],
+    hdrs = ["register_cleanup_pass.h"],
+    deps = [
+        ":block_conversion_pass",
+        "//xls/common/status:status_macros",
+        "//xls/data_structures:transitive_closure",
+        "//xls/ir",
+        "//xls/ir:register",
+        "//xls/ir:value",
+        "//xls/ir:value_utils",
+        "//xls/passes:node_dependency_analysis",
+        "//xls/passes:partial_info_query_engine",
+        "//xls/passes:pass_base",
+        "//xls/passes:query_engine",
+        "@com_google_absl//absl/algorithm:container",
+        "@com_google_absl//absl/container:flat_hash_map",
+        "@com_google_absl//absl/container:flat_hash_set",
+        "@com_google_absl//absl/status:statusor",
+        "@com_google_absl//absl/types:span",
+    ],
+)
+
+cc_library(
+    name = "block_conversion_wrapper_pass",
+    srcs = ["block_conversion_wrapper_pass.cc"],
+    hdrs = ["block_conversion_wrapper_pass.h"],
+    deps = [
+        ":block_conversion_pass",
+        "//xls/ir",
+        "//xls/passes:optimization_pass",
+        "//xls/passes:pass_base",
+        "@com_google_absl//absl/status:statusor",
+        "@com_google_absl//absl/strings:str_format",
+        "@com_google_absl//absl/types:span",
+    ],
+)
+
+cc_test(
+    name = "block_conversion_wrapper_pass_test",
+    srcs = ["block_conversion_wrapper_pass_test.cc"],
+    deps = [
+        ":block_conversion_pass",
+        ":block_conversion_wrapper_pass",
+        "//xls/common:xls_gunit_main",
+        "//xls/common/status:matchers",
+        "//xls/ir",
+        "//xls/ir:function_builder",
+        "//xls/ir:ir_matcher",
+        "//xls/ir:ir_test_base",
+        "//xls/passes:dce_pass",
+        "//xls/passes:optimization_pass",
+        "//xls/passes:pass_base",
+        "@com_google_absl//absl/status:statusor",
+        "@googletest//:gtest",
+    ],
+)
+
 cc_library(
     name = "channel_to_port_io_lowering_pass",
     srcs = ["channel_to_port_io_lowering_pass.cc"],
     hdrs = ["channel_to_port_io_lowering_pass.h"],
     deps = [
         ":block_conversion_pass",
+        "//xls/codegen:conversion_utils",
+        "//xls/codegen:ram_configuration",
+        "//xls/common:casts",
+        "//xls/common/status:ret_check",
+        "//xls/common/status:status_macros",
+        "//xls/ir",
+        "//xls/ir:bits",
+        "//xls/ir:channel",
+        "//xls/ir:node_util",
+        "//xls/ir:op",
+        "//xls/ir:register",
+        "//xls/ir:source_location",
+        "//xls/ir:value",
+        "//xls/ir:value_utils",
+        "//xls/passes:bdd_query_engine",
+        "//xls/passes:pass_base",
+        "//xls/public:function_builder",
+        "@com_google_absl//absl/algorithm:container",
+        "@com_google_absl//absl/base:core_headers",
+        "@com_google_absl//absl/container:btree",
+        "@com_google_absl//absl/container:flat_hash_map",
+        "@com_google_absl//absl/container:flat_hash_set",
+        "@com_google_absl//absl/container:inlined_vector",
+        "@com_google_absl//absl/log:check",
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
+        "@com_google_absl//absl/strings",
+        "@com_google_absl//absl/strings:str_format",
+        "@com_google_absl//absl/types:span",
+        "@cppitertools",
+    ],
+)
+
+cc_test(
+    name = "channel_to_port_io_lowering_pass_test",
+    srcs = ["channel_to_port_io_lowering_pass_test.cc"],
+    deps = [
+        ":block_conversion_pass",
+        ":channel_to_port_io_lowering_pass",
+        ":scheduled_block_conversion_pass",
+        "//xls/codegen:codegen_options",
+        "//xls/common:xls_gunit_main",
+        "//xls/common/status:matchers",
+        "//xls/common/status:status_macros",
         "//xls/ir",
+        "//xls/ir:bits",
+        "//xls/ir:channel",
+        "//xls/ir:channel_ops",
+        "//xls/ir:function_builder",
+        "//xls/ir:ir_matcher",
+        "//xls/ir:ir_test_base",
+        "//xls/ir:op",
+        "//xls/ir:value",
         "//xls/passes:pass_base",
         "@com_google_absl//absl/status:statusor",
+        "@googletest//:gtest",
     ],
 )
 
@@ -117,6 +234,7 @@ cc_library(
         "//xls/common/status:status_macros",
         "//xls/estimators/delay_model:delay_estimator",
         "//xls/ir",
+        "//xls/passes:optimization_pass",
         "//xls/passes:pass_base",
         "//xls/scheduling:pipeline_schedule_cc_proto",
         "//xls/scheduling:scheduling_options",

xls/codegen_v_1_5/block_conversion_pass_pipeline.cc

@@ -20,20 +20,25 @@
 #include <memory>
 
 #include "xls/codegen_v_1_5/block_conversion_pass.h"
+#include "xls/codegen_v_1_5/block_conversion_wrapper_pass.h"
 #include "xls/codegen_v_1_5/block_finalization_pass.h"
 #include "xls/codegen_v_1_5/channel_to_port_io_lowering_pass.h"
 #include "xls/codegen_v_1_5/flow_control_insertion_pass.h"
 #include "xls/codegen_v_1_5/function_io_lowering_pass.h"
 #include "xls/codegen_v_1_5/idle_insertion_pass.h"
 #include "xls/codegen_v_1_5/pipeline_register_insertion_pass.h"
+#include "xls/codegen_v_1_5/register_cleanup_pass.h"
 #include "xls/codegen_v_1_5/scheduled_block_conversion_pass.h"
 #include "xls/codegen_v_1_5/scheduling_pass.h"
 #include "xls/codegen_v_1_5/state_to_register_io_lowering_pass.h"
+#include "xls/passes/dataflow_simplification_pass.h"
+#include "xls/passes/dce_pass.h"
+#include "xls/passes/optimization_pass.h"
 
 namespace xls::codegen {
 
-std::unique_ptr<BlockConversionCompoundPass>
-CreateBlockConversionPassPipeline() {
+std::unique_ptr<BlockConversionCompoundPass> CreateBlockConversionPassPipeline(
+    OptimizationContext& opt_context) {
   auto top = std::make_unique<BlockConversionCompoundPass>(
       "block_conversion", "Top level codegen v1.5 block conversion pipeline");
 
@@ -64,6 +69,18 @@ CreateBlockConversionPassPipeline() {
   // Lower scheduled block to standard block, inlining each stage.
   top->Add<BlockFinalizationPass>();
 
+  // Clean up unused registers & load-enable bits (including flow-control
+  // registers).
+  top->Add<RegisterCleanupPass>();
+
+  // Clean up unnecessary array/tuple manipulation.
+  top->Add<BlockConversionWrapperPass>(
+      std::make_unique<DataflowSimplificationPass>(), opt_context);
+
+  // Remove anything we created & then left dead.
+  top->Add<BlockConversionWrapperPass>(
+      std::make_unique<DeadCodeEliminationPass>(), opt_context);
+
   return top;
 }
 

xls/codegen_v_1_5/block_conversion_pass_pipeline.h

@@ -18,13 +18,14 @@
 #include <memory>
 
 #include "xls/codegen_v_1_5/block_conversion_pass.h"
+#include "xls/passes/optimization_pass.h"
 
 namespace xls::codegen {
 
 // Returns a pipeline which converts an unscheduled IR package into a standard
 // block.
-std::unique_ptr<BlockConversionCompoundPass>
-CreateBlockConversionPassPipeline();
+std::unique_ptr<BlockConversionCompoundPass> CreateBlockConversionPassPipeline(
+    OptimizationContext& opt_context);
 
 }  // namespace xls::codegen
 

xls/codegen_v_1_5/block_conversion_pass_pipeline_test.cc

@@ -54,7 +54,6 @@
 #include "xls/common/status/matchers.h"
 #include "xls/common/status/ret_check.h"
 #include "xls/common/status/status_macros.h"
-#include "xls/estimators/delay_model/delay_estimator.h"
 #include "xls/interpreter/block_evaluator.h"
 #include "xls/interpreter/block_interpreter.h"
 #include "xls/ir/bits.h"
@@ -97,6 +96,7 @@ using ::testing::ElementsAre;
 using ::testing::Eq;
 using ::testing::Ge;
 using ::testing::HasSubstr;
+using ::testing::IsEmpty;
 using ::testing::Optional;
 using ::testing::Pair;
 using ::testing::SizeIs;
@@ -282,7 +282,7 @@ TEST_F(BlockConversionTest, SimpleFunction) {
       m::OutputPort("out", m::Add(m::InputPort("x"), m::InputPort("y"))));
 }
 
-TEST_F(BlockConversionTest, DISABLED_SimpleFunctionWithNamedOutput) {
+TEST_F(BlockConversionTest, SimpleFunctionWithNamedOutput) {
   auto p = CreatePackage();
   FunctionBuilder fb(TestName(), p.get());
   BValue x = fb.Param("x", p->GetBitsType(32));
@@ -331,7 +331,7 @@ TEST_F(BlockConversionTest, ZeroWidthInputsAndOutput) {
   EXPECT_EQ(block->GetPorts().size(), 4);
 }
 
-TEST_F(BlockConversionTest, DISABLED_SimplePipelinedFunction) {
+TEST_F(BlockConversionTest, SimplePipelinedFunction) {
   auto p = CreatePackage();
   FunctionBuilder fb(TestName(), p.get());
   BValue x = fb.Param("x", p->GetBitsType(32));
@@ -348,11 +348,13 @@ TEST_F(BlockConversionTest, DISABLED_SimplePipelinedFunction) {
           SchedulingOptions().pipeline_stages(3)));
 
   EXPECT_THAT(GetOutputPort(block),
-              m::OutputPort(m::Neg(m::Register(m::Not(m::Register(
-                  m::Add(m::InputPort("x"), m::InputPort("y"))))))));
+              m::OutputPort(m::Neg(m::Register(m::Not(
+                  m::Register(m::Add(m::InputPort("x"), m::InputPort("y"))))))))
+      << "\n\nIR:\n"
+      << block->DumpIr();
 }
 
-TEST_F(BlockConversionTest, DISABLED_TrivialPipelinedFunctionNoFlopping) {
+TEST_F(BlockConversionTest, TrivialPipelinedFunctionNoFlopping) {
   auto p = CreatePackage();
   FunctionBuilder fb(TestName(), p.get());
   BValue x = fb.Param("x", p->GetBitsType(32));
@@ -373,7 +375,7 @@ TEST_F(BlockConversionTest, DISABLED_TrivialPipelinedFunctionNoFlopping) {
                   m::Add(m::InputPort("x"), m::InputPort("y"))))))));
 }
 
-TEST_F(BlockConversionTest, DISABLED_TrivialPipelinedFunctionFloppedInputs) {
+TEST_F(BlockConversionTest, TrivialPipelinedFunctionFloppedInputs) {
   auto p = CreatePackage();
   FunctionBuilder fb(TestName(), p.get());
   BValue x = fb.Param("x", p->GetBitsType(32));
@@ -395,7 +397,7 @@ TEST_F(BlockConversionTest, DISABLED_TrivialPipelinedFunctionFloppedInputs) {
           m::Register(m::InputPort("x")), m::Register(m::InputPort("y")))))))));
 }
 
-TEST_F(BlockConversionTest, DISABLED_TrivialPipelinedFunctionFloppedOutputs) {
+TEST_F(BlockConversionTest, TrivialPipelinedFunctionFloppedOutputs) {
   auto p = CreatePackage();
   FunctionBuilder fb(TestName(), p.get());
   BValue x = fb.Param("x", p->GetBitsType(32));
@@ -416,7 +418,7 @@ TEST_F(BlockConversionTest, DISABLED_TrivialPipelinedFunctionFloppedOutputs) {
                   m::Add(m::InputPort("x"), m::InputPort("y")))))))));
 }
 
-TEST_F(BlockConversionTest, DISABLED_TrivialPipelinedFunctionFloppedIo) {
+TEST_F(BlockConversionTest, TrivialPipelinedFunctionFloppedIo) {
   auto p = CreatePackage();
   FunctionBuilder fb(TestName(), p.get());
   BValue x = fb.Param("x", p->GetBitsType(32));
@@ -438,7 +440,7 @@ TEST_F(BlockConversionTest, DISABLED_TrivialPipelinedFunctionFloppedIo) {
                                      m::Register(m::InputPort("y"))))))))));
 }
 
-TEST_F(BlockConversionTest, DISABLED_ZeroWidthPipeline) {
+TEST_F(BlockConversionTest, ZeroWidthPipeline) {
   auto p = CreatePackage();
   FunctionBuilder fb(TestName(), p.get());
   BValue x = fb.Param("x", p->GetTupleType({}));
@@ -454,7 +456,28 @@ TEST_F(BlockConversionTest, DISABLED_ZeroWidthPipeline) {
               "clk"),
           SchedulingOptions().pipeline_stages(3)));
 
-  EXPECT_EQ(block->GetRegisters().size(), 4);
+  EXPECT_THAT(block->GetRegisters(), IsEmpty());
+}
+
+TEST_F(BlockConversionTest, ZeroWidthPipelineWithValidControl) {
+  auto p = CreatePackage();
+  FunctionBuilder fb(TestName(), p.get());
+  BValue x = fb.Param("x", p->GetTupleType({}));
+  BValue y = fb.Param("y", p->GetBitsType(0));
+  XLS_ASSERT_OK_AND_ASSIGN(Function * f,
+                           fb.BuildWithReturnValue(fb.Tuple({x, y})));
+
+  XLS_ASSERT_OK_AND_ASSIGN(
+      Block * block,
+      ConvertToBlock(f,
+                     CodegenOptions()
+                         .flop_inputs(false)
+                         .flop_outputs(false)
+                         .clock_name("clk")
+                         .valid_control("inputs_valid", "output_valid"),
+                     SchedulingOptions().pipeline_stages(3)));
+
+  EXPECT_THAT(block->GetRegisters(), SizeIs(2));
 }
 
 // Verifies that an implicit token, as generated by the DSLX IR converter, is
@@ -473,11 +496,10 @@ fn __itok__implicit_token__main(__token: token, __activated: bits[1]) ->
 fn __implicit_token__main() -> () {
   after_all.9: token = after_all(id=9)
   literal.10: bits[1] = literal(value=1, id=10)
-  invoke.11: (token, ()) = invoke(after_all.9, literal.10,
-  to_apply=__itok__implicit_token__main, id=11) tuple_index.12: token =
-  tuple_index(invoke.11, index=0, id=12) invoke.13: (token, ()) =
-  invoke(tuple_index.12, literal.10, to_apply=__itok__implicit_token__main,
-  id=13) ret tuple_index.14: () = tuple_index(invoke.13, index=1, id=14)
+  invoke.11: (token, ()) = invoke(after_all.9, literal.10, to_apply=__itok__implicit_token__main, id=11)
+  tuple_index.12: token = tuple_index(invoke.11, index=0, id=12)
+  invoke.13: (token, ()) = invoke(tuple_index.12, literal.10, to_apply=__itok__implicit_token__main, id=13)
+  ret tuple_index.14: () = tuple_index(invoke.13, index=1, id=14)
 }
   )";
   XLS_ASSERT_OK_AND_ASSIGN(auto p, Parser::ParsePackage(kIrText));
@@ -489,7 +511,7 @@ fn __implicit_token__main() -> () {
   XLS_ASSERT_OK(VerifyBlock(block));
 }
 
-TEST_F(BlockConversionTest, DISABLED_SimpleProc) {
+TEST_F(BlockConversionTest, SimpleProc) {
   const std::string ir_text = R"(package test
 
 chan in(bits[32], id=0, kind=single_value, ops=receive_only)
@@ -513,7 +535,9 @@ proc my_proc(my_state: (), init={()}) {
       Block * block,
       ConvertToBlock(proc, codegen_options().generate_combinational(true)));
   EXPECT_THAT(FindNode("out", block),
-              m::OutputPort("out", m::Neg(m::InputPort("in"))));
+              m::OutputPort("out", m::Neg(m::InputPort("in"))))
+      << "\n\nIR:\n"
+      << block->DumpIr();
 }
 
 TEST_F(BlockConversionTest, DISABLED_StreamingChannelMetadataForSimpleProc) {
@@ -6394,7 +6418,7 @@ TEST_F(ProcConversionTestFixture,
   }
 }
 
-TEST_F(ProcConversionTestFixture, DISABLED_SimpleFunctionWithProcsPresent) {
+TEST_F(ProcConversionTestFixture, SimpleFunctionWithProcsPresent) {
   XLS_ASSERT_OK_AND_ASSIGN(std::unique_ptr<Package> p,
                            CreateMultiProcPackage(/*with_functions=*/true));
   XLS_ASSERT_OK_AND_ASSIGN(Function * f0, p->GetFunction("f0"));