Skip to content

Commit

Permalink
Remove dead proto ModulePortProto from channel signature/metadata.
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 722709435
  • Loading branch information
@meheffernan @copybara-github
meheffernan authored and copybara-github committed Feb 3, 2025
1 parent b54dce5 commit a431cc7
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 99 deletions.
28 changes: 10 additions & 18 deletions xls/codegen/block_conversion_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -701,10 +701,8 @@ fn __implicit_token__main() -> () {
TEST_F(BlockConversionTest, SimpleProc) {
const std::string ir_text = R"(package test
chan in(bits[32], id=0, kind=single_value, ops=receive_only,
metadata="""module_port { flopped: false, port_order: 1 }""")
chan out(bits[32], id=1, kind=single_value, ops=send_only,
metadata="""module_port { flopped: false, port_order: 0 }""")
chan in(bits[32], id=0, kind=single_value, ops=receive_only, metadata="""""")
chan out(bits[32], id=1, kind=single_value, ops=send_only, metadata="""""")
proc my_proc(my_state: (), init={()}) {
my_token: token = literal(value=token, id=1)
Expand Down Expand Up @@ -1083,13 +1081,13 @@ TEST_F(BlockConversionTest, ProcWithMultipleInputChannels) {
const std::string ir_text = R"(package test
chan in0(bits[32], id=0, kind=single_value, ops=receive_only,
metadata="""module_port { flopped: false, port_order: 0 }""")
metadata="""""")
chan in1(bits[32], id=1, kind=single_value, ops=receive_only,
metadata="""module_port { flopped: false, port_order: 2 }""")
metadata="""""")
chan in2(bits[32], id=2, kind=single_value, ops=receive_only,
metadata="""module_port { flopped: false, port_order: 1 }""")
metadata="""""")
chan out(bits[32], id=3, kind=single_value, ops=send_only,
metadata="""module_port { flopped: false, port_order: 0 }""")
metadata="""""")
proc my_proc(my_state: (), init={()}) {
my_token: token = literal(value=token, id=1)
Expand Down Expand Up @@ -1188,11 +1186,8 @@ top proc my_proc() {

TEST_F(BlockConversionTest, OnlyFIFOInProcGateRecvsTrue) {
const std::string ir_text = R"(package test
chan in(bits[32], id=0, kind=streaming, ops=receive_only,
flow_control=ready_valid, metadata="""module_port { flopped: false
port_order: 0 }""")
chan out(bits[32], id=1, kind=single_value,
ops=send_only, metadata="""module_port { flopped: false port_order: 1 }""")
chan in(bits[32], id=0, kind=streaming, ops=receive_only, flow_control=ready_valid, metadata="""""")
chan out(bits[32], id=1, kind=single_value, ops=send_only, metadata="""""")
proc my_proc(st: (), init={()}) {
tkn: token = literal(value=token, id=1)
Expand Down Expand Up @@ -1225,11 +1220,8 @@ proc my_proc(st: (), init={()}) {

TEST_F(BlockConversionTest, OnlyFIFOInProcGateRecvsFalse) {
const std::string ir_text = R"(package test
chan in(bits[32], id=0, kind=streaming, ops=receive_only,
flow_control=ready_valid, metadata="""module_port { flopped: false
port_order: 0 }""")
chan out(bits[32], id=1, kind=single_value,
ops=send_only, metadata="""module_port { flopped: false port_order: 1 }""")
chan in(bits[32], id=0, kind=streaming, ops=receive_only, flow_control=ready_valid, metadata="""""")
chan out(bits[32], id=1, kind=single_value, ops=send_only, metadata="""""")
proc my_proc(st: (), init={()}) {
tkn: token = literal(value=token, id=1)
Expand Down
22 changes: 0 additions & 22 deletions xls/ir/channel.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,24 +55,6 @@ message ChannelConfigProto {
FlopKind flop_outputs = 3;
}

// Metadata for a channel which describes ports on a Verilog module.
message ModulePortProto {
// Whether the port is flopped on entry (exit) to (from) the module.
// TODO(meheff): 2021-04-23 This is effectively unused. Remove it.
optional bool flopped = 1;

// A number used to determine the ordering of the ports. Within a proc, the
// port_order of the channels must be numbered densely from zero. Channels
// with lower numbered port_order values will have corresponding ports earlier
// in the module declaration. The port_order value does not necessarily
// correspond to the position of the port in the Verilog module because a
// channel can correspond to multiple ports and the module may have other
// ports such as clock and reset.
// TODO(meheff): 2021-04-23 Remove this and replace uses with
// PortChannel::Position.
optional int64 port_order = 2;
}

// Metadata associating channels with specific ports in a block/module.
message BlockPortMappingProto {
optional string block_name = 1;
Expand All @@ -86,10 +68,6 @@ message BlockPortMappingProto {
// (e.g., data types) should be held directly in the xls::Channel data
// structure.
message ChannelMetadataProto {
oneof channel_oneof {
ModulePortProto module_port = 1;
}

// Different sides of a channel can be associated with different ports in
// different blocks.
repeated BlockPortMappingProto block_ports = 2;
Expand Down
48 changes: 16 additions & 32 deletions xls/ir/ir_parser_error_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1532,17 +1532,14 @@ TEST(IrParserErrorTest, ParseSendReceiveChannel) {
Parser::ParseChannel(
R"(chan foo(bits[32], id=42, kind=single_value,
ops=send_receive,
metadata="module_port { flopped: true }"))",
metadata=""))",
&p));
EXPECT_EQ(ch->name(), "foo");
EXPECT_EQ(ch->id(), 42);
EXPECT_EQ(ch->supported_ops(), ChannelOps::kSendReceive);
EXPECT_EQ(ch->kind(), ChannelKind::kSingleValue);
EXPECT_EQ(ch->type(), p.GetBitsType(32));
EXPECT_TRUE(ch->initial_values().empty());
EXPECT_EQ(ch->metadata().channel_oneof_case(),
ChannelMetadataProto::kModulePort);
EXPECT_TRUE(ch->metadata().module_port().flopped());
}

TEST(IrParserErrorTest, ParseSendReceiveChannelWithInitialValues) {
Expand All @@ -1552,7 +1549,7 @@ TEST(IrParserErrorTest, ParseSendReceiveChannelWithInitialValues) {
Parser::ParseChannel(
R"(chan foo(bits[32], initial_values={2, 4, 5}, id=42, kind=streaming,
flow_control=none, ops=send_receive,
metadata="module_port { flopped: true }"))",
metadata=""))",
&p));
EXPECT_EQ(ch->name(), "foo");
EXPECT_EQ(ch->id(), 42);
Expand All @@ -1562,9 +1559,6 @@ TEST(IrParserErrorTest, ParseSendReceiveChannelWithInitialValues) {
EXPECT_THAT(ch->initial_values(),
ElementsAre(Value(UBits(2, 32)), Value(UBits(4, 32)),
Value(UBits(5, 32))));
EXPECT_EQ(ch->metadata().channel_oneof_case(),
ChannelMetadataProto::kModulePort);
EXPECT_TRUE(ch->metadata().module_port().flopped());
}

TEST(IrParserErrorTest, ParseSendReceiveChannelWithTupleType) {
Expand All @@ -1574,7 +1568,7 @@ TEST(IrParserErrorTest, ParseSendReceiveChannelWithTupleType) {
initial_values={(123, 1), (42, 0)},
id=42, kind=streaming, flow_control=ready_valid,
ops=send_receive,
metadata="module_port { flopped: true }"))",
metadata=""))",
&p));
EXPECT_EQ(ch->name(), "foo");
EXPECT_THAT(
Expand All @@ -1588,31 +1582,25 @@ TEST(IrParserErrorTest, ParseSendOnlyChannel) {
XLS_ASSERT_OK_AND_ASSIGN(Channel * ch, Parser::ParseChannel(
R"(chan bar((bits[32], bits[1]),
id=7, kind=single_value, ops=send_only,
metadata="module_port { flopped: false }"))",
metadata=""))",
&p));
EXPECT_EQ(ch->name(), "bar");
EXPECT_EQ(ch->id(), 7);
EXPECT_EQ(ch->supported_ops(), ChannelOps::kSendOnly);
EXPECT_EQ(ch->type(), p.GetTupleType({p.GetBitsType(32), p.GetBitsType(1)}));
EXPECT_EQ(ch->metadata().channel_oneof_case(),
ChannelMetadataProto::kModulePort);
EXPECT_FALSE(ch->metadata().module_port().flopped());
}

TEST(IrParserErrorTest, ParseReceiveOnlyChannel) {
Package p("my_package");
XLS_ASSERT_OK_AND_ASSIGN(Channel * ch, Parser::ParseChannel(
R"(chan meh(bits[32][4], id=0,
kind=single_value, ops=receive_only,
metadata="module_port { flopped: true }"))",
metadata=""))",
&p));
EXPECT_EQ(ch->name(), "meh");
EXPECT_EQ(ch->id(), 0);
EXPECT_EQ(ch->supported_ops(), ChannelOps::kReceiveOnly);
EXPECT_EQ(ch->type(), p.GetArrayType(4, p.GetBitsType(32)));
EXPECT_EQ(ch->metadata().channel_oneof_case(),
ChannelMetadataProto::kModulePort);
EXPECT_TRUE(ch->metadata().module_port().flopped());
}

TEST(IrParserErrorTest, ParseStreamingChannelWithStrictness) {
Expand Down Expand Up @@ -1715,16 +1703,12 @@ TEST(IrParserErrorTest, ParseSingleValueChannelWithBlockPortMapping) {
XLS_ASSERT_OK_AND_ASSIGN(Channel * ch, Parser::ParseChannel(
R"(chan meh(bits[32][4], id=0,
kind=single_value, ops=receive_only,
metadata="""module_port { flopped: true },
block_ports { data_port_name : "data",
metadata="""block_ports { data_port_name : "data",
block_name : "blk"}"""))",
&p));
EXPECT_EQ(ch->name(), "meh");
EXPECT_EQ(ch->id(), 0);
EXPECT_EQ(ch->supported_ops(), ChannelOps::kReceiveOnly);
EXPECT_EQ(ch->metadata().channel_oneof_case(),
ChannelMetadataProto::kModulePort);
EXPECT_TRUE(ch->metadata().module_port().flopped());

EXPECT_THAT(
ch->metadata().block_ports(),
Expand Down Expand Up @@ -1761,7 +1745,7 @@ TEST(IrParserErrorTest, ChannelParsingErrors) {
EXPECT_THAT(Parser::ParseChannel(
R"(chan meh(bits[32][4], kind=single_value,
ops=receive_only,
metadata="module_port { flopped: true }"))",
metadata=""))",
&p)
.status(),
StatusIs(absl::StatusCode::kInvalidArgument,
Expand All @@ -1770,7 +1754,7 @@ TEST(IrParserErrorTest, ChannelParsingErrors) {
EXPECT_THAT(
Parser::ParseChannel(
R"(chan meh(bits[32][4], id=42, ops=receive_only,
metadata="module_port { flopped: true }"))",
metadata=""))",
&p)
.status(),
StatusIs(absl::StatusCode::kInvalidArgument,
Expand All @@ -1779,7 +1763,7 @@ TEST(IrParserErrorTest, ChannelParsingErrors) {
EXPECT_THAT(Parser::ParseChannel(
R"(chan meh(bits[32][4], id=42, kind=bogus,
ops=receive_only,
metadata="module_port { flopped: true }"))",
metadata=""))",
&p)
.status(),
StatusIs(absl::StatusCode::kInvalidArgument,
Expand All @@ -1788,7 +1772,7 @@ TEST(IrParserErrorTest, ChannelParsingErrors) {
EXPECT_THAT(
Parser::ParseChannel(
R"(chan meh(bits[32][4], id=7, kind=streaming,
metadata="module_port { flopped: true }"))",
metadata=""))",
&p)
.status(),
StatusIs(absl::StatusCode::kInvalidArgument,
Expand All @@ -1798,7 +1782,7 @@ TEST(IrParserErrorTest, ChannelParsingErrors) {
EXPECT_THAT(Parser::ParseChannel(
R"(chan meh(bits[4], initial_values={128}, kind=streaming,
ops=send_receive, id=7,
metadata="module_port { flopped: true }"))",
metadata=""))",
&p)
.status(),
StatusIs(absl::StatusCode::kInvalidArgument,
Expand All @@ -1808,7 +1792,7 @@ TEST(IrParserErrorTest, ChannelParsingErrors) {
EXPECT_THAT(Parser::ParseChannel(
R"(chan meh(bits[4], initial_values={(1, 2)}, kind=streaming,
ops=send_receive, id=7
metadata="module_port { flopped: true }"))",
metadata=""))",
&p)
.status(),
StatusIs(absl::StatusCode::kInvalidArgument,
Expand All @@ -1825,7 +1809,7 @@ TEST(IrParserErrorTest, ChannelParsingErrors) {

EXPECT_THAT(Parser::ParseChannel(
R"(chan meh(id=44, kind=streaming, ops=receive_only,
metadata="module_port { flopped: true }"))",
metadata=""))",
&p)
.status(),
StatusIs(absl::StatusCode::kInvalidArgument,
Expand All @@ -1834,7 +1818,7 @@ TEST(IrParserErrorTest, ChannelParsingErrors) {
EXPECT_THAT(Parser::ParseChannel(
R"(chan meh(bits[32], id=44, kind=streaming,
ops=receive_only, bogus="totally!",
metadata="module_port { flopped: true }"))",
metadata=""))",
&p)
.status(),
StatusIs(absl::StatusCode::kInvalidArgument,
Expand Down Expand Up @@ -1884,9 +1868,9 @@ TEST(IrParserErrorTest, PackageWithSingleDataElementChannels) {
package test
chan hbo(bits[32], id=0, kind=streaming, flow_control=none, ops=receive_only,
fifo_depth=42, metadata="module_port { flopped: true }")
fifo_depth=42, metadata="")
chan mtv(bits[32], id=1, kind=streaming, flow_control=none, ops=send_only,
metadata="module_port { flopped: true }")
metadata="")
proc my_proc(my_token: token, my_state: bits[32], init={token, 42}) {
receive.1: (token, bits[32]) = receive(my_token, channel=hbo)
Expand Down
32 changes: 8 additions & 24 deletions xls/ir/ir_parser_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -632,17 +632,14 @@ TEST(IrParserTest, ParseSendReceiveChannel) {
Parser::ParseChannel(
R"(chan foo(bits[32], id=42, kind=single_value,
ops=send_receive,
metadata="module_port { flopped: true }"))",
metadata=""))",
&p));
EXPECT_EQ(ch->name(), "foo");
EXPECT_EQ(ch->id(), 42);
EXPECT_EQ(ch->supported_ops(), ChannelOps::kSendReceive);
EXPECT_EQ(ch->kind(), ChannelKind::kSingleValue);
EXPECT_EQ(ch->type(), p.GetBitsType(32));
EXPECT_TRUE(ch->initial_values().empty());
EXPECT_EQ(ch->metadata().channel_oneof_case(),
ChannelMetadataProto::kModulePort);
EXPECT_TRUE(ch->metadata().module_port().flopped());

EXPECT_FALSE(p.ChannelsAreProcScoped());
}
Expand All @@ -654,7 +651,7 @@ TEST(IrParserTest, ParseSendReceiveChannelWithInitialValues) {
Parser::ParseChannel(
R"(chan foo(bits[32], initial_values={2, 4, 5}, id=42, kind=streaming,
flow_control=none, ops=send_receive,
metadata="module_port { flopped: true }"))",
metadata=""))",
&p));
EXPECT_EQ(ch->name(), "foo");
EXPECT_EQ(ch->id(), 42);
Expand All @@ -664,9 +661,6 @@ TEST(IrParserTest, ParseSendReceiveChannelWithInitialValues) {
EXPECT_THAT(ch->initial_values(),
ElementsAre(Value(UBits(2, 32)), Value(UBits(4, 32)),
Value(UBits(5, 32))));
EXPECT_EQ(ch->metadata().channel_oneof_case(),
ChannelMetadataProto::kModulePort);
EXPECT_TRUE(ch->metadata().module_port().flopped());
}

TEST(IrParserTest, ParseSendReceiveChannelWithTupleType) {
Expand All @@ -676,7 +670,7 @@ TEST(IrParserTest, ParseSendReceiveChannelWithTupleType) {
initial_values={(123, 1), (42, 0)},
id=42, kind=streaming, flow_control=ready_valid,
ops=send_receive,
metadata="module_port { flopped: true }"))",
metadata=""))",
&p));
EXPECT_EQ(ch->name(), "foo");
EXPECT_THAT(
Expand All @@ -690,31 +684,25 @@ TEST(IrParserTest, ParseSendOnlyChannel) {
XLS_ASSERT_OK_AND_ASSIGN(Channel * ch, Parser::ParseChannel(
R"(chan bar((bits[32], bits[1]),
id=7, kind=single_value, ops=send_only,
metadata="module_port { flopped: false }"))",
metadata=""))",
&p));
EXPECT_EQ(ch->name(), "bar");
EXPECT_EQ(ch->id(), 7);
EXPECT_EQ(ch->supported_ops(), ChannelOps::kSendOnly);
EXPECT_EQ(ch->type(), p.GetTupleType({p.GetBitsType(32), p.GetBitsType(1)}));
EXPECT_EQ(ch->metadata().channel_oneof_case(),
ChannelMetadataProto::kModulePort);
EXPECT_FALSE(ch->metadata().module_port().flopped());
}

TEST(IrParserTest, ParseReceiveOnlyChannel) {
Package p("my_package");
XLS_ASSERT_OK_AND_ASSIGN(Channel * ch, Parser::ParseChannel(
R"(chan meh(bits[32][4], id=0,
kind=single_value, ops=receive_only,
metadata="module_port { flopped: true }"))",
metadata=""))",
&p));
EXPECT_EQ(ch->name(), "meh");
EXPECT_EQ(ch->id(), 0);
EXPECT_EQ(ch->supported_ops(), ChannelOps::kReceiveOnly);
EXPECT_EQ(ch->type(), p.GetArrayType(4, p.GetBitsType(32)));
EXPECT_EQ(ch->metadata().channel_oneof_case(),
ChannelMetadataProto::kModulePort);
EXPECT_TRUE(ch->metadata().module_port().flopped());
}

TEST(IrParserTest, ParseStreamingChannelWithStrictness) {
Expand Down Expand Up @@ -849,16 +837,12 @@ TEST(IrParserTest, ParseSingleValueChannelWithBlockPortMapping) {
XLS_ASSERT_OK_AND_ASSIGN(Channel * ch, Parser::ParseChannel(
R"(chan meh(bits[32][4], id=0,
kind=single_value, ops=receive_only,
metadata="""module_port { flopped: true },
block_ports { data_port_name : "data",
metadata="""block_ports { data_port_name : "data",
block_name : "blk"}"""))",
&p));
EXPECT_EQ(ch->name(), "meh");
EXPECT_EQ(ch->id(), 0);
EXPECT_EQ(ch->supported_ops(), ChannelOps::kReceiveOnly);
EXPECT_EQ(ch->metadata().channel_oneof_case(),
ChannelMetadataProto::kModulePort);
EXPECT_TRUE(ch->metadata().module_port().flopped());

EXPECT_THAT(
ch->metadata().block_ports(),
Expand Down Expand Up @@ -895,9 +879,9 @@ TEST(IrParserTest, PackageWithSingleDataElementChannels) {
package test
chan hbo(bits[32], id=0, kind=streaming, flow_control=none, ops=receive_only,
fifo_depth=42, metadata="module_port { flopped: true }")
fifo_depth=42, metadata="")
chan mtv(bits[32], id=1, kind=streaming, flow_control=none, ops=send_only,
metadata="module_port { flopped: true }")
metadata="")
proc my_proc(my_token: token, my_state: bits[32], init={token, 42}) {
receive.1: (token, bits[32]) = receive(my_token, channel=hbo)
Expand Down
Loading

0 comments on commit a431cc7

Please sign in to comment.