Skip to content

Commit ce2089f

Browse files
grebecopybara-github
authored andcommitted
Fix infinite loop between strength reduction and DCE.
0-bit params could lead to strength reduction replacing it with a literal and DCE removing it, which causes an infinite loop when run to fixed point. PiperOrigin-RevId: 681593175
1 parent e827137 commit ce2089f

File tree

2 files changed

+39
-2
lines changed

2 files changed

+39
-2
lines changed

xls/passes/strength_reduction_pass.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,11 @@ absl::StatusOr<bool> StrengthReduceNode(
149149
return false;
150150
}
151151

152-
if (NarrowingEnabled(opt_level) && !node->Is<Literal>() &&
152+
if (NarrowingEnabled(opt_level) &&
153+
// Don't replace unused nodes. We don't want to add nodes when they will
154+
// get DCEd later. This can lead to an infinite loop between strength
155+
// reduction and DCE.
156+
!node->users().empty() && !node->Is<Literal>() &&
153157
query_engine.IsFullyKnown(node)) {
154158
VLOG(2) << "Replacing node with its (entirely known) value: " << node
155159
<< " as " << query_engine.KnownValue(node)->ToString();

xls/passes/strength_reduction_pass_test.cc

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@ namespace xls {
3939
namespace {
4040

4141
using status_testing::IsOkAndHolds;
42+
4243
using ::testing::_;
44+
using ::testing::Each;
45+
using ::testing::UnorderedElementsAre;
4346
using ::testing::VariantWith;
4447

4548
class StrengthReductionPassTest : public IrTestBase {
@@ -416,7 +419,7 @@ TEST_F(StrengthReductionPassTest, ArithToSelect) {
416419
ASSERT_THAT(Run(f), IsOkAndHolds(true));
417420
// Actual verification of result is done by semantics test.
418421
EXPECT_THAT(f->return_value()->operands(),
419-
testing::Each(m::Select(m::Eq(), {m::Literal(), m::Literal()})));
422+
Each(m::Select(m::Eq(), {m::Literal(), m::Literal()})));
420423
}
421424

422425
TEST_F(StrengthReductionPassTest, ArithToSelectOnlyWithOneBit) {
@@ -476,5 +479,35 @@ TEST_F(StrengthReductionPassTest, DoNotPushDownMultipleSelects) {
476479
ASSERT_THAT(Run(f), IsOkAndHolds(false)) << f->DumpIr();
477480
}
478481

482+
TEST_F(StrengthReductionPassTest, ReplaceWidth0Param) {
483+
auto p = CreatePackage();
484+
FunctionBuilder fb(TestName(), p.get());
485+
BValue ret = fb.Or(fb.ZeroExtend(fb.Param("x", p->GetBitsType(0)), 1),
486+
fb.Param("y", p->GetBitsType(1)));
487+
XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.BuildWithReturnValue(ret));
488+
EXPECT_THAT(f->nodes(), UnorderedElementsAre(
489+
m::Param("x"), m::Param("y"), m::ZeroExt(),
490+
m::Or(m::ZeroExt(m::Param("x")), m::Param("y"))));
491+
ASSERT_THAT(Run(f), IsOkAndHolds(true));
492+
EXPECT_THAT(f->nodes(),
493+
UnorderedElementsAre(m::Param("x"), m::Param("y"), m::Literal(),
494+
m::Or(m::Literal(0), m::Param("y"))));
495+
}
496+
497+
TEST_F(StrengthReductionPassTest, DoNotReplaceUnusedWidth0Param) {
498+
auto p = CreatePackage();
499+
FunctionBuilder fb(TestName(), p.get());
500+
fb.Param("x", p->GetBitsType(0));
501+
XLS_ASSERT_OK_AND_ASSIGN(
502+
Function * f, fb.BuildWithReturnValue(fb.Param("y", p->GetBitsType(1))));
503+
EXPECT_THAT(f->nodes(), UnorderedElementsAre(m::Param("x"), m::Param("y")));
504+
// Normally, the empty param would be replaced with a literal, but since it
505+
// is unused, it doesn't get replaced.
506+
// Replacing unused params with literals can lead to an infinite loop with
507+
// strength reduction adding the literal and DCE removing it.
508+
ASSERT_THAT(Run(f), IsOkAndHolds(false));
509+
EXPECT_THAT(f->nodes(), UnorderedElementsAre(m::Param("x"), m::Param("y")));
510+
}
511+
479512
} // namespace
480513
} // namespace xls

0 commit comments

Comments
 (0)