Skip to content

Commit e211bbb

Browse files
richmckeevercopybara-github
authored andcommitted
Don't lower an unscheduled proc to a block in codegen 1.5 when pipelining is requested.
Doing so was causing xls/contrib/mlir/testdata:integration/procify.mlir.test to fail with codegen 1.5 enabled by default. This change makes the behavior match 1.0 in this case. PiperOrigin-RevId: 863232457
1 parent 76fb633 commit e211bbb

File tree

6 files changed

+176
-2
lines changed

6 files changed

+176
-2
lines changed

xls/codegen_v_1_5/BUILD

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,27 @@ cc_library(
142142
],
143143
)
144144

145+
cc_test(
146+
name = "register_cleanup_pass_test",
147+
srcs = ["register_cleanup_pass_test.cc"],
148+
deps = [
149+
":block_conversion_pass",
150+
":register_cleanup_pass",
151+
"//xls/common:xls_gunit_main",
152+
"//xls/common/status:matchers",
153+
"//xls/ir",
154+
"//xls/ir:bits",
155+
"//xls/ir:function_builder",
156+
"//xls/ir:ir_matcher",
157+
"//xls/ir:ir_test_base",
158+
"//xls/ir:register",
159+
"//xls/ir:source_location",
160+
"//xls/passes:pass_base",
161+
"@com_google_absl//absl/status:statusor",
162+
"@googletest//:gtest",
163+
],
164+
)
165+
145166
cc_library(
146167
name = "block_conversion_wrapper_pass",
147168
srcs = ["block_conversion_wrapper_pass.cc"],

xls/codegen_v_1_5/register_cleanup_pass.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,8 @@ absl::StatusOr<bool> RegisterCleanupPass::RemoveUnreadRegisters(
127127
if (user->Is<RegisterWrite>()) {
128128
can_receive_value_from[reg].insert(
129129
user->As<RegisterWrite>()->GetRegister());
130-
} else if (user->Is<OutputPort>()) {
130+
} else if (user->Is<OutputPort>() || user->Is<Assert>() ||
131+
user->Is<Cover>() || user->Is<Trace>()) {
131132
directly_read_registers.insert(reg);
132133
}
133134
}
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
// Copyright 2026 The XLS Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#include "xls/codegen_v_1_5/register_cleanup_pass.h"
16+
17+
#include <memory>
18+
#include <optional>
19+
20+
#include "gmock/gmock.h"
21+
#include "gtest/gtest.h"
22+
#include "absl/status/statusor.h"
23+
#include "xls/codegen_v_1_5/block_conversion_pass.h"
24+
#include "xls/common/status/matchers.h"
25+
#include "xls/ir/bits.h"
26+
#include "xls/ir/block.h"
27+
#include "xls/ir/function_builder.h"
28+
#include "xls/ir/ir_matcher.h"
29+
#include "xls/ir/ir_test_base.h"
30+
#include "xls/ir/node.h"
31+
#include "xls/ir/register.h"
32+
#include "xls/ir/scheduled_builder.h"
33+
#include "xls/ir/source_location.h"
34+
#include "xls/passes/pass_base.h"
35+
36+
namespace m = xls::op_matchers;
37+
38+
namespace xls::codegen {
39+
namespace {
40+
41+
class RegisterCleanupPassTest : public IrTestBase {
42+
protected:
43+
absl::StatusOr<bool> Run(Package* p,
44+
const BlockConversionPassOptions& options =
45+
BlockConversionPassOptions()) {
46+
PassResults results;
47+
return RegisterCleanupPass().Run(p, options, &results);
48+
}
49+
};
50+
51+
TEST_F(RegisterCleanupPassTest, AssertConditionFromPriorStage) {
52+
auto p = CreatePackage();
53+
ScheduledBlockBuilder sbb(TestName(), p.get());
54+
XLS_ASSERT_OK(sbb.block()->AddClockPort("clk"));
55+
BValue x = sbb.InputPort("x", p->GetBitsType(32));
56+
57+
XLS_ASSERT_OK_AND_ASSIGN(
58+
Register * reg, sbb.block()->AddRegister("x_reg", p->GetBitsType(1)));
59+
60+
sbb.StartStage(sbb.Literal(UBits(1, 1)), sbb.Literal(UBits(1, 1)));
61+
BValue condition = sbb.UGe(x, sbb.Literal(UBits(3, 32)));
62+
sbb.RegisterWrite(reg, condition);
63+
sbb.EndStage(sbb.Literal(UBits(1, 1)), sbb.Literal(UBits(1, 1)));
64+
65+
sbb.StartStage(sbb.Literal(UBits(1, 1)), sbb.Literal(UBits(1, 1)));
66+
sbb.Assert(sbb.AfterAll({}), sbb.RegisterRead(reg), "failure", std::nullopt,
67+
SourceInfo{}, "assert0");
68+
sbb.EndStage(sbb.Literal(UBits(1, 1)), sbb.Literal(UBits(1, 1)));
69+
70+
XLS_ASSERT_OK_AND_ASSIGN(ScheduledBlock * sb, sbb.Build());
71+
72+
XLS_ASSERT_OK(Run(p.get()).status());
73+
XLS_ASSERT_OK_AND_ASSIGN(Node * assert0, sb->GetNode("assert0"));
74+
EXPECT_THAT(assert0->operands()[1], Not(m::Literal()));
75+
}
76+
77+
} // namespace
78+
} // namespace xls::codegen

xls/codegen_v_1_5/scheduling_pass.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,10 @@ absl::StatusOr<bool> SchedulingPass::RunInternal(
115115
return_value, Op::kIdentity));
116116
XLS_RETURN_IF_ERROR(new_fn->set_return_value(staged_return_value));
117117
}
118-
} else if (old_fb->IsProc()) {
118+
} else if (old_fb->IsProc() &&
119+
(options.codegen_options.generate_combinational() ||
120+
options.package_schedule.schedules().contains(
121+
old_fb->name()))) {
119122
Proc* new_proc = package->AddProc(
120123
std::make_unique<ScheduledProc>(old_fb->name(), package));
121124
new_fb = new_proc;

xls/codegen_v_1_5/scheduling_pass_test.cc

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,5 +120,46 @@ top proc __test__P_0_next(__state: bits[32], init={0}) {
120120
ExpectEqualToGoldenFile(output);
121121
}
122122

123+
TEST_F(SchedulingPassTest, MultiProcWithOneProcScheduled) {
124+
XLS_ASSERT_OK_AND_ASSIGN(
125+
std::string output, RunPassAndRoundTripIrText(
126+
R"(
127+
package test
128+
129+
chan test__a(bits[32], id=0, kind=streaming, ops=receive_only, flow_control=ready_valid, strictness=proven_mutually_exclusive)
130+
chan test__b(bits[32], id=1, kind=streaming, ops=receive_only, flow_control=ready_valid, strictness=proven_mutually_exclusive)
131+
chan test__result(bits[32], id=2, kind=streaming, ops=send_only, flow_control=ready_valid, strictness=proven_mutually_exclusive)
132+
133+
proc __test2__P_0_next(__state: bits[32], init={0}) {
134+
__last_state: bits[32] = state_read(state_element=__state)
135+
next_value.100: () = next_value(param=__state, value=__last_state)
136+
}
137+
138+
top proc __test__P_0_next(__state: bits[32], init={0}) {
139+
after_all.4: token = after_all(id=4)
140+
literal.3: bits[1] = literal(value=1, id=3)
141+
receive.5: (token, bits[32]) = receive(after_all.4, predicate=literal.3, channel=test__a, id=5)
142+
tok: token = tuple_index(receive.5, index=0, id=7)
143+
receive.9: (token, bits[32]) = receive(tok, predicate=literal.3, channel=test__b, id=9)
144+
a_value: bits[32] = tuple_index(receive.5, index=1, id=8)
145+
b_value: bits[32] = tuple_index(receive.9, index=1, id=12)
146+
umul.13: bits[32] = umul(a_value, b_value, id=13)
147+
__state: bits[32] = state_read(state_element=__state, id=2)
148+
tok__1: token = tuple_index(receive.9, index=0, id=11)
149+
result_value: bits[32] = add(umul.13, __state, id=14)
150+
__token: token = literal(value=token, id=1)
151+
tuple_index.6: token = tuple_index(receive.5, index=0, id=6)
152+
tuple_index.10: token = tuple_index(receive.9, index=0, id=10)
153+
send.15: token = send(tok__1, result_value, predicate=literal.3, channel=test__result, id=15)
154+
next_value.16: () = next_value(param=__state, value=result_value, id=16)
155+
}
156+
157+
)",
158+
/*expect_change=*/true, verilog::CodegenOptions(),
159+
SchedulingOptions().pipeline_stages(2)));
160+
161+
ExpectEqualToGoldenFile(output);
162+
}
163+
123164
} // namespace
124165
} // namespace xls::codegen
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package test
2+
3+
chan test__a(bits[32], id=0, kind=streaming, ops=receive_only, flow_control=ready_valid, strictness=proven_mutually_exclusive)
4+
chan test__b(bits[32], id=1, kind=streaming, ops=receive_only, flow_control=ready_valid, strictness=proven_mutually_exclusive)
5+
chan test__result(bits[32], id=2, kind=streaming, ops=send_only, flow_control=ready_valid, strictness=proven_mutually_exclusive)
6+
7+
proc __test2__P_0_next(__state: bits[32], init={0}) {
8+
__state: bits[32] = state_read(state_element=__state, id=117)
9+
next_value.100: () = next_value(param=__state, value=__state, id=100)
10+
}
11+
12+
top scheduled_proc __test__P_0_next(__state: bits[32], init={0}) {
13+
literal.3: bits[1] = literal(value=1, id=3)
14+
stage {
15+
after_all.4: token = after_all(id=4)
16+
receive.5: (token, bits[32]) = receive(after_all.4, predicate=literal.3, channel=test__a, id=5)
17+
tok: token = tuple_index(receive.5, index=0, id=7)
18+
receive.9: (token, bits[32]) = receive(tok, predicate=literal.3, channel=test__b, id=9)
19+
a_value: bits[32] = tuple_index(receive.5, index=1, id=8)
20+
tok__1: token = tuple_index(receive.9, index=0, id=11)
21+
b_value: bits[32] = tuple_index(receive.9, index=1, id=12)
22+
umul.13: bits[32] = umul(a_value, b_value, id=13)
23+
}
24+
stage {
25+
__state: bits[32] = state_read(state_element=__state, id=2)
26+
result_value: bits[32] = add(umul.13, __state, id=14)
27+
send.15: token = send(tok__1, result_value, predicate=literal.3, channel=test__result, id=15)
28+
next_value.16: () = next_value(param=__state, value=result_value, id=16)
29+
}
30+
}

0 commit comments

Comments
 (0)