From 1e83aa1bee4cf199a5f269a7d69b1adeb1871b08 Mon Sep 17 00:00:00 2001 From: Paul Rigge Date: Thu, 26 Dec 2024 17:11:50 -0800 Subject: [PATCH] Make Nary*IfNeeded variants keep the original order of the operands when uniquifying. This produces more stable output orders. PiperOrigin-RevId: 709910975 --- ...rator_test_LoopbackFifoInstantiation.svtxt | 6 +-- ...erator_test_LoopbackFifoInstantiation.vtxt | 6 +-- xls/ir/BUILD | 1 - xls/ir/node_util.cc | 54 +++++++++++-------- xls/ir/node_util_test.cc | 4 +- ..._main_test__test_add_idle_output_proc.vtxt | 2 +- 6 files changed, 41 insertions(+), 32 deletions(-) diff --git a/xls/codegen/testdata/block_generator_test_LoopbackFifoInstantiation.svtxt b/xls/codegen/testdata/block_generator_test_LoopbackFifoInstantiation.svtxt index 554f048ba7..545f188dd3 100644 --- a/xls/codegen/testdata/block_generator_test_LoopbackFifoInstantiation.svtxt +++ b/xls/codegen/testdata/block_generator_test_LoopbackFifoInstantiation.svtxt @@ -21,8 +21,8 @@ module running_sum( reg __out_data_has_been_sent_reg; reg __fifo_loopback_push_data_has_been_sent_reg; wire p2_all_active_states_valid; - wire __fifo_loopback_push_data_has_sent_or_is_ready; wire __out_data_has_sent_or_is_ready; + wire __fifo_loopback_push_data_has_sent_or_is_ready; wire p2_stage_valid; wire p2_all_active_inputs_valid; wire p2_all_active_outputs_ready; @@ -64,11 +64,11 @@ module running_sum( wire __fifo_loopback_push_data_has_been_sent_reg_load_en; assign p2_all_active_states_valid = 1'h1; - assign __fifo_loopback_push_data_has_sent_or_is_ready = instantiation_output_1061 | __fifo_loopback_push_data_has_been_sent_reg; assign __out_data_has_sent_or_is_ready = out_ready | __out_data_has_been_sent_reg; + assign __fifo_loopback_push_data_has_sent_or_is_ready = instantiation_output_1061 | __fifo_loopback_push_data_has_been_sent_reg; assign p2_stage_valid = p2_all_active_states_valid & p1_valid; assign p2_all_active_inputs_valid = 1'h1; - assign p2_all_active_outputs_ready = __fifo_loopback_push_data_has_sent_or_is_ready & __out_data_has_sent_or_is_ready; + assign p2_all_active_outputs_ready = __out_data_has_sent_or_is_ready & __fifo_loopback_push_data_has_sent_or_is_ready; assign p1_all_active_states_valid = 1'h1; assign loopback_active_valid = ~p0_not_first_cycle | instantiation_output_1045; assign p2_stage_done = p2_stage_valid & p2_all_active_inputs_valid & p2_all_active_outputs_ready; diff --git a/xls/codegen/testdata/block_generator_test_LoopbackFifoInstantiation.vtxt b/xls/codegen/testdata/block_generator_test_LoopbackFifoInstantiation.vtxt index bbf71594e9..89b944e112 100644 --- a/xls/codegen/testdata/block_generator_test_LoopbackFifoInstantiation.vtxt +++ b/xls/codegen/testdata/block_generator_test_LoopbackFifoInstantiation.vtxt @@ -21,8 +21,8 @@ module running_sum( reg __out_data_has_been_sent_reg; reg __fifo_loopback_push_data_has_been_sent_reg; wire p2_all_active_states_valid; - wire __fifo_loopback_push_data_has_sent_or_is_ready; wire __out_data_has_sent_or_is_ready; + wire __fifo_loopback_push_data_has_sent_or_is_ready; wire p2_stage_valid; wire p2_all_active_inputs_valid; wire p2_all_active_outputs_ready; @@ -64,11 +64,11 @@ module running_sum( wire __fifo_loopback_push_data_has_been_sent_reg_load_en; assign p2_all_active_states_valid = 1'h1; - assign __fifo_loopback_push_data_has_sent_or_is_ready = instantiation_output_1061 | __fifo_loopback_push_data_has_been_sent_reg; assign __out_data_has_sent_or_is_ready = out_ready | __out_data_has_been_sent_reg; + assign __fifo_loopback_push_data_has_sent_or_is_ready = instantiation_output_1061 | __fifo_loopback_push_data_has_been_sent_reg; assign p2_stage_valid = p2_all_active_states_valid & p1_valid; assign p2_all_active_inputs_valid = 1'h1; - assign p2_all_active_outputs_ready = __fifo_loopback_push_data_has_sent_or_is_ready & __out_data_has_sent_or_is_ready; + assign p2_all_active_outputs_ready = __out_data_has_sent_or_is_ready & __fifo_loopback_push_data_has_sent_or_is_ready; assign p1_all_active_states_valid = 1'h1; assign loopback_active_valid = ~p0_not_first_cycle | instantiation_output_1045; assign p2_stage_done = p2_stage_valid & p2_all_active_inputs_valid & p2_all_active_outputs_ready; diff --git a/xls/ir/BUILD b/xls/ir/BUILD index 2a79a1fd0b..c442dd35a8 100644 --- a/xls/ir/BUILD +++ b/xls/ir/BUILD @@ -1317,7 +1317,6 @@ cc_library( "//xls/data_structures:inline_bitmap", "//xls/data_structures:leaf_type_tree", "@com_google_absl//absl/algorithm:container", - "@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/log:check", diff --git a/xls/ir/node_util.cc b/xls/ir/node_util.cc index 4109b896c1..bc6305f7e0 100644 --- a/xls/ir/node_util.cc +++ b/xls/ir/node_util.cc @@ -26,7 +26,6 @@ #include #include "absl/algorithm/container.h" -#include "absl/container/btree_set.h" #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" #include "absl/log/check.h" @@ -54,6 +53,24 @@ #include "xls/ir/value_utils.h" namespace xls { +namespace { + +// Produces a vector of nodes from the given span removing duplicates. The +// first instance of each node is kept and subsequent duplicates are dropped, +// e.g. RemoveDuplicateNodes({a, b, a, c, a, d}) -> {a, b, c, d}. +std::vector RemoveDuplicateNodes(absl::Span values) { + absl::flat_hash_set unique_values_set(values.begin(), values.end()); + std::vector unique_values_vector; + unique_values_vector.reserve(unique_values_set.size()); + for (Node* value : values) { + if (auto extracted = unique_values_set.extract(value)) { + unique_values_vector.push_back(extracted.value()); + } + } + return unique_values_vector; +} + +} // namespace bool IsLiteralWithRunOfSetBits(Node* node, int64_t* leading_zero_count, int64_t* set_bit_count, @@ -347,15 +364,12 @@ absl::StatusOr NaryAndIfNeeded(FunctionBase* f, return f->MakeNodeWithName(source_info, Value(UBits(1, 1)), name); } - absl::btree_set unique_operands(operands.begin(), - operands.end()); + std::vector unique_operands = RemoveDuplicateNodes(operands); if (unique_operands.size() == 1) { - return operands[0]; + return unique_operands[0]; } - return f->MakeNodeWithName( - source_info, - std::vector(unique_operands.begin(), unique_operands.end()), - Op::kAnd, name); + return f->MakeNodeWithName(source_info, unique_operands, Op::kAnd, + name); } absl::StatusOr NaryOrIfNeeded(FunctionBase* f, @@ -366,15 +380,12 @@ absl::StatusOr NaryOrIfNeeded(FunctionBase* f, return f->MakeNodeWithName(source_info, Value(UBits(0, 1)), name); } - absl::btree_set unique_operands(operands.begin(), - operands.end()); + std::vector unique_operands = RemoveDuplicateNodes(operands); if (unique_operands.size() == 1) { - return operands[0]; + return unique_operands[0]; } - return f->MakeNodeWithName( - source_info, - std::vector(unique_operands.begin(), unique_operands.end()), - Op::kOr, name); + return f->MakeNodeWithName(source_info, unique_operands, Op::kOr, + name); } absl::StatusOr NaryNorIfNeeded(FunctionBase* f, @@ -382,15 +393,14 @@ absl::StatusOr NaryNorIfNeeded(FunctionBase* f, std::string_view name, const SourceInfo& source_info) { XLS_RET_CHECK(!operands.empty()); - absl::btree_set unique_operands(operands.begin(), - operands.end()); + + std::vector unique_operands = RemoveDuplicateNodes(operands); if (unique_operands.size() == 1) { - return f->MakeNodeWithName(source_info, operands[0], Op::kNot, name); + return f->MakeNodeWithName(source_info, unique_operands[0], Op::kNot, + name); } - return f->MakeNodeWithName( - source_info, - std::vector(unique_operands.begin(), unique_operands.end()), - Op::kNor, name); + return f->MakeNodeWithName(source_info, unique_operands, Op::kNor, + name); } bool IsUnsignedCompare(Node* node) { diff --git a/xls/ir/node_util_test.cc b/xls/ir/node_util_test.cc index 2c3d54c1f6..1be3edb8b5 100644 --- a/xls/ir/node_util_test.cc +++ b/xls/ir/node_util_test.cc @@ -481,8 +481,8 @@ TEST_F(NodeUtilTest, NaryNorWithMultipleInputs) { NaryNorIfNeeded( f, std::vector{a0.node(), a3.node(), a2.node(), a1.node(), a4.node(), a1.node(), a3.node(), a1.node()}), - IsOkAndHolds(m::Nor(m::Param("a0"), m::Param("a1"), m::Param("a2"), - m::Param("a3"), m::Param("a4")))); + IsOkAndHolds(m::Nor(m::Param("a0"), m::Param("a3"), m::Param("a2"), + m::Param("a1"), m::Param("a4")))); } TEST_F(NodeUtilTest, ChannelUsers) { diff --git a/xls/tools/testdata/codegen_main_test__test_add_idle_output_proc.vtxt b/xls/tools/testdata/codegen_main_test__test_add_idle_output_proc.vtxt index 08e7829335..9d3f7e46eb 100644 --- a/xls/tools/testdata/codegen_main_test__test_add_idle_output_proc.vtxt +++ b/xls/tools/testdata/codegen_main_test__test_add_idle_output_proc.vtxt @@ -45,5 +45,5 @@ module neg_proc( assign out = __out_reg; assign out_vld = __out_valid_reg; assign in_rdy = in_load_en; - assign idle = ~(in_vld | __in_valid_reg | __out_valid_reg); + assign idle = ~(__in_valid_reg | __out_valid_reg | in_vld); endmodule