Skip to content

Commit c06d44f

Browse files
ericastorcopybara-github
authored andcommitted
Add support for omitting values from the next (...) line in XLS IR procs
If using `next_value` operations, these values are now redundant; state parameters default to remaining unchanged for the next activation. This is an all-or-nothing choice; if you want to use the `next (...)` line for any non-token value, you must provide all values, and may use no `next_value` operations. NOTE: `next_value` operations are not yet supported in any backend, so users should not switch over yet! PiperOrigin-RevId: 597977645
1 parent 6bbb23f commit c06d44f

File tree

7 files changed

+89
-13
lines changed

7 files changed

+89
-13
lines changed

xls/ir/function_builder.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,14 +1129,16 @@ absl::StatusOr<Proc*> ProcBuilder::Build(BValue token,
11291129
absl::StrFormat("Recurrent token of proc must be token type, is: %s.",
11301130
GetType(token)->ToString()));
11311131
}
1132-
if (next_state.size() != state_params_.size()) {
1132+
1133+
// TODO: Remove this once fully transitioned over to `next_value` nodes.
1134+
if (!next_state.empty() && next_state.size() != state_params_.size()) {
11331135
return absl::InvalidArgumentError(
11341136
absl::StrFormat("Number of recurrent state elements given (%d) does "
11351137
"not equal the number of state elements in the proc "
11361138
"(%d)",
11371139
next_state.size(), state_params_.size()));
11381140
}
1139-
for (int64_t i = 0; i < state_params_.size(); ++i) {
1141+
for (int64_t i = 0; i < next_state.size(); ++i) {
11401142
if (GetType(next_state[i]) != GetType(GetStateParam(i))) {
11411143
return absl::InvalidArgumentError(
11421144
absl::StrFormat("Recurrent state type %s does not match proc "

xls/ir/function_builder.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -762,10 +762,11 @@ class ProcBuilder : public BuilderBase {
762762
BValue GetStateParam(int64_t index) const { return state_params_.at(index); }
763763

764764
// Build the proc using the given BValues as the recurrent token and next
765-
// state values respectively. The number of recurrent state elements in
766-
// `next_state` must match the number of state parameters.
765+
// state values respectively. If `next_state` is not empty, the number of
766+
// recurrent state elements in `next_state` must match the number of state
767+
// parameters.
767768
absl::StatusOr<Proc*> Build(BValue token,
768-
absl::Span<const BValue> next_state);
769+
absl::Span<const BValue> next_state = {});
769770

770771
// Adds a state element to the proc with the given initial value. Returns the
771772
// newly added state parameter.

xls/ir/ir_parser.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1687,6 +1687,8 @@ absl::StatusOr<Parser::BodyResult> Parser::ParseBody(
16871687
"Proc next token name @ %s was not previously defined: \"%s\"",
16881688
next_token_name.pos().ToHumanString(), next_token_name.value()));
16891689
}
1690+
1691+
// TODO: Remove this once fully transitioned over to `next_value` nodes.
16901692
std::vector<BValue> next_state;
16911693
while (scanner_.TryDropToken(LexicalTokenType::kComma)) {
16921694
XLS_ASSIGN_OR_RETURN(Token next_state_name,
@@ -1699,6 +1701,14 @@ absl::StatusOr<Parser::BodyResult> Parser::ParseBody(
16991701
}
17001702
next_state.push_back(name_to_value->at(next_state_name.value()));
17011703
}
1704+
if (Proc* proc = fb->function()->AsProcOrDie();
1705+
!next_state.empty() && !proc->next_values().empty()) {
1706+
return absl::InvalidArgumentError(absl::StrFormat(
1707+
"Proc includes both next_value nodes (e.g., %s) and next-state "
1708+
"values on its 'next' line; both cannot be used at the same time.",
1709+
proc->next_values().front()->GetName()));
1710+
}
1711+
17021712
XLS_RETURN_IF_ERROR(
17031713
scanner_.DropTokenOrError(LexicalTokenType::kParenClose));
17041714
result =

xls/ir/ir_parser_test.cc

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,7 +1278,6 @@ proc my_proc(my_token: token, my_state: bits[32], init={42}) {
12781278
}
12791279

12801280
TEST(IrParserTest, ParseProcWithExplicitNext) {
1281-
// TODO(epastor): Remove the next-value for `my_state` from the last line
12821281
const std::string input = R"(package test
12831282
12841283
chan ch(bits[32], id=0, kind=streaming, ops=send_receive, flow_control=none, strictness=proven_mutually_exclusive, metadata="""""")
@@ -1289,14 +1288,33 @@ proc my_proc(my_token: token, my_state: bits[32], init={42}) {
12891288
receive.3: (token, bits[32]) = receive(send.1, predicate=literal.2, channel=ch, id=3)
12901289
tuple_index.4: token = tuple_index(receive.3, index=0, id=4)
12911290
next_value.5: () = next_value(param=my_state, value=my_state, id=5)
1292-
next (tuple_index.4, my_state)
1291+
next (tuple_index.4)
12931292
}
12941293
)";
12951294
ParsePackageAndCheckDump(input);
12961295
}
12971296

1297+
TEST(IrParserTest, ParseProcWithMixedNextValueStyles) {
1298+
const std::string input = R"(package test
1299+
1300+
chan ch(bits[32], id=0, kind=streaming, ops=send_receive, flow_control=none, strictness=proven_mutually_exclusive, metadata="""""")
1301+
1302+
proc my_proc(my_token: token, my_state_1: bits[32], my_state_2: bits[32], init={42, 64}) {
1303+
send.1: token = send(my_token, my_state_1, channel=ch, id=1)
1304+
literal.2: bits[1] = literal(value=1, id=2)
1305+
receive.3: (token, bits[32]) = receive(send.1, predicate=literal.2, channel=ch, id=3)
1306+
tuple_index.4: token = tuple_index(receive.3, index=0, id=4)
1307+
next_value.5: () = next_value(param=my_state_2, value=my_state_2, id=5)
1308+
next (tuple_index.4, my_state_1, my_state_2)
1309+
}
1310+
)";
1311+
EXPECT_THAT(Parser::ParsePackage(input).status(),
1312+
StatusIs(absl::StatusCode::kInvalidArgument,
1313+
HasSubstr("Proc includes both next_value nodes (e.g., "
1314+
"next_value.5) and next-state values")));
1315+
}
1316+
12981317
TEST(IrParserTest, ParseProcWithBadNextParam) {
1299-
// TODO(epastor): Remove the next-value for `my_state` from the last line
13001318
const std::string input = R"(package test
13011319
13021320
chan ch(bits[32], id=0, kind=streaming, ops=send_receive, flow_control=none, strictness=proven_mutually_exclusive, metadata="""""")
@@ -1307,7 +1325,7 @@ proc my_proc(my_token: token, my_state: bits[32], init={42}) {
13071325
receive.3: (token, bits[32]) = receive(send.1, predicate=literal.2, channel=ch, id=3)
13081326
tuple_index.4: token = tuple_index(receive.3, index=0, id=4)
13091327
next_value.5: () = next_value(param=not_my_state, value=my_state, id=5)
1310-
next (tuple_index.4, my_state)
1328+
next (tuple_index.4)
13111329
}
13121330
)";
13131331
EXPECT_THAT(Parser::ParsePackage(input).status(),
@@ -1318,7 +1336,6 @@ proc my_proc(my_token: token, my_state: bits[32], init={42}) {
13181336
}
13191337

13201338
TEST(IrParserTest, ParseProcWithBadNextValueType) {
1321-
// TODO(epastor): Remove the next-value for `my_state` from the last line
13221339
const std::string input = R"(package test
13231340
13241341
chan ch(bits[32], id=0, kind=streaming, ops=send_receive, flow_control=none, strictness=proven_mutually_exclusive, metadata="""""")
@@ -1329,7 +1346,7 @@ proc my_proc(my_token: token, my_state: bits[32], init={42}) {
13291346
receive.3: (token, bits[32]) = receive(send.1, predicate=literal.2, channel=ch, id=3)
13301347
tuple_index.4: token = tuple_index(receive.3, index=0, id=4)
13311348
next_value.5: () = next_value(param=my_state, value=literal.2, id=5)
1332-
next (tuple_index.4, my_state)
1349+
next (tuple_index.4)
13331350
}
13341351
)";
13351352
EXPECT_THAT(Parser::ParsePackage(input).status(),

xls/ir/proc.cc

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,13 @@ std::string Proc::DumpIr() const {
8585
}
8686
absl::StrAppend(&res, " ", node->ToString(), "\n");
8787
}
88+
8889
absl::StrAppend(&res, " next (", NextToken()->GetName());
89-
for (Node* node : next_state_) {
90-
absl::StrAppend(&res, ", ", node->GetName());
90+
// TODO: Remove this once fully transitioned over to `next_value` nodes.
91+
if (next_values_.empty()) {
92+
for (Node* node : next_state_) {
93+
absl::StrAppend(&res, ", ", node->GetName());
94+
}
9195
}
9296
absl::StrAppend(&res, ")\n}\n");
9397
return res;
@@ -222,8 +226,11 @@ absl::StatusOr<Param*> Proc::ReplaceStateElement(
222226
"state param %s has uses",
223227
index, name(), old_param->GetName()));
224228
}
229+
230+
// TODO: Remove this once fully transitioned over to `next_value` nodes.
225231
next_state_indices_[next_state_[index]].erase(index);
226232
next_state_[index] = nullptr;
233+
227234
XLS_RETURN_IF_ERROR(RemoveNode(old_param));
228235

229236
// Construct the new param node, and update all trackers.
@@ -243,6 +250,8 @@ absl::StatusOr<Param*> Proc::ReplaceStateElement(
243250
next_state.value()->GetType()->ToString(), init_value.ToString()));
244251
}
245252
init_values_[index] = init_value;
253+
254+
// TODO: Remove this once fully transitioned over to `next_value` nodes.
246255
next_state_[index] = next_state.value_or(param);
247256
next_state_indices_[next_state_[index]].insert(index);
248257

@@ -251,6 +260,8 @@ absl::StatusOr<Param*> Proc::ReplaceStateElement(
251260

252261
absl::Status Proc::RemoveStateElement(int64_t index) {
253262
XLS_RET_CHECK_LT(index, GetStateElementCount());
263+
264+
// TODO: Remove this once fully transitioned over to `next_value` nodes.
254265
if (index < GetStateElementCount() - 1) {
255266
for (auto& [_, indices] : next_state_indices_) {
256267
// Relabel all indices > `index`.
@@ -264,6 +275,7 @@ absl::Status Proc::RemoveStateElement(int64_t index) {
264275
}
265276
next_state_indices_[next_state_[index]].erase(index);
266277
next_state_.erase(next_state_.begin() + index);
278+
267279
Param* old_param = GetStateParam(index);
268280
if (!old_param->users().empty()) {
269281
return absl::InvalidArgumentError(
@@ -294,6 +306,8 @@ absl::StatusOr<Param*> Proc::InsertStateElement(
294306
MakeNodeWithName<Param>(SourceInfo(), state_param_name,
295307
package()->GetTypeForValue(init_value)));
296308
XLS_RETURN_IF_ERROR(MoveParamToIndex(param, index + 1));
309+
310+
// TODO: Remove this once fully transitioned over to `next_value` nodes.
297311
if (next_state.has_value()) {
298312
if (!ValueConformsToType(init_value, next_state.value()->GetType())) {
299313
return absl::InvalidArgumentError(absl::StrFormat(
@@ -316,6 +330,7 @@ absl::StatusOr<Param*> Proc::InsertStateElement(
316330
}
317331
next_state_.insert(next_state_.begin() + index, next_state.value_or(param));
318332
next_state_indices_[next_state_[index]].insert(index);
333+
319334
init_values_.insert(init_values_.begin() + index, init_value);
320335
return param;
321336
}
@@ -324,10 +339,13 @@ bool Proc::HasImplicitUse(Node* node) const {
324339
if (node == NextToken()) {
325340
return true;
326341
}
342+
343+
// TODO: Remove this once fully transitioned over to `next_value` nodes.
327344
if (auto it = next_state_indices_.find(node);
328345
it != next_state_indices_.end() && !it->second.empty()) {
329346
return true;
330347
}
348+
331349
return false;
332350
}
333351

@@ -535,10 +553,12 @@ absl::StatusOr<Proc*> Proc::Clone(
535553
XLS_RETURN_IF_ERROR(
536554
cloned_proc->SetNextToken(original_to_clone.at(NextToken())));
537555

556+
// TODO: Remove this once fully transitioned over to `next_value` nodes.
538557
for (int64_t i = 0; i < GetStateElementCount(); ++i) {
539558
XLS_RETURN_IF_ERROR(cloned_proc->SetNextStateElement(
540559
i, original_to_clone.at(GetNextStateElement(i))));
541560
}
561+
542562
return cloned_proc;
543563
}
544564

xls/ir/proc.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,17 @@ class Proc : public FunctionBase {
9797
Node* NextToken() const { return next_token_; }
9898

9999
// Returns the nodes holding the next recurrent state value.
100+
//
101+
// TODO: Remove this once fully transitioned over to `next_value` nodes.
100102
absl::Span<Node* const> NextState() const { return next_state_; }
101103
Node* GetNextStateElement(int64_t index) const {
102104
return NextState().at(index);
103105
}
104106

105107
// Return the state element indices for which the given `node` is the next
106108
// recurrent state value for that element.
109+
//
110+
// TODO: Remove this once fully transitioned over to `next_value` nodes.
107111
absl::btree_set<int64_t> GetNextStateIndices(Node* node) const;
108112

109113
// Returns the type of the given state element.
@@ -120,6 +124,8 @@ class Proc : public FunctionBase {
120124

121125
// Sets the next recurrent state value for the state element of the given
122126
// index. Node type must match the type of the state element.
127+
//
128+
// TODO: Remove this once fully transitioned over to `next_value` nodes.
123129
absl::Status SetNextStateElement(int64_t index, Node* next);
124130

125131
// Replace all state elements with new state parameters and the given initial
@@ -133,6 +139,8 @@ class Proc : public FunctionBase {
133139
// than as a std::optional `next_state` argument because initializer lists do
134140
// not explicitly convert to std::optional<absl::Span> making callsites
135141
// verbose.
142+
//
143+
// TODO: Remove this once fully transitioned over to `next_value` nodes.
136144
absl::Status ReplaceState(absl::Span<const std::string> state_param_names,
137145
absl::Span<const Value> init_values,
138146
absl::Span<Node* const> next_state);
@@ -262,9 +270,13 @@ class Proc : public FunctionBase {
262270
// proc.
263271
Node* next_token_;
264272
bool is_new_style_proc_;
273+
274+
// TODO: Remove this once fully transitioned over to `next_value` nodes.
265275
std::vector<Node*> next_state_;
266276

267277
// A map from the `next_state_` nodes back to the indices they control.
278+
//
279+
// TODO: Remove this once fully transitioned over to `next_value` nodes.
268280
absl::flat_hash_map<Node*, absl::btree_set<int64_t>> next_state_indices_;
269281

270282
// All channel references in this proc. Channel references can be part of the

xls/ir/verifier.cc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2070,6 +2070,20 @@ absl::Status VerifyProc(Proc* proc, bool codegen) {
20702070
// Verify that the order of parameters matches the state element order.
20712071
XLS_RET_CHECK_EQ(proc->param(i + 1), proc->GetStateParam(i));
20722072

2073+
Param* param = proc->GetStateParam(i);
2074+
Node* next_state = proc->GetNextStateElement(i);
2075+
if (next_state == param) {
2076+
continue;
2077+
}
2078+
2079+
// Verify that this proc does not use `next_value` nodes.
2080+
if (!proc->next_values().empty()) {
2081+
return absl::InvalidArgumentError(absl::StrFormat(
2082+
"Proc %s includes both next_value nodes (e.g., %s) and next-state "
2083+
"values on its 'next' line; both cannot be used at the same time.",
2084+
proc->name(), proc->next_values().front()->GetName()));
2085+
}
2086+
20732087
// Verify type of state param matches type of the corresponding initial
20742088
// value and next state element.
20752089
XLS_RET_CHECK_EQ(proc->GetStateParam(i)->GetType(),

0 commit comments

Comments
 (0)