diff --git a/xls/passes/visibility_expr_builder.cc b/xls/passes/visibility_expr_builder.cc index 807d542f16..70efa784b0 100644 --- a/xls/passes/visibility_expr_builder.cc +++ b/xls/passes/visibility_expr_builder.cc @@ -411,13 +411,6 @@ absl::StatusOr VisibilityBuilder::BuildVisibilityIRExpr( XLS_ASSIGN_OR_RETURN( Literal * always_visible, func->MakeNode(SourceInfo(), Value(UBits(1, 1)))); - if (conditional_edges.size() == 1) { - XLS_ASSIGN_OR_RETURN( - Node * user_uses_node, - BuildVisibilityExpr(conditional_edges.begin()->operand, - conditional_edges.begin()->node, node, func)); - return user_uses_node ? user_uses_node : always_visible; - } absl::flat_hash_map node_to_visibility_ir_cache; absl::flat_hash_map, Node*> binary_op_cache; return BuildVisibilityIRExprFromEdges(func, node, node, conditional_edges, diff --git a/xls/passes/visibility_expr_builder_test.cc b/xls/passes/visibility_expr_builder_test.cc index 81c0f87b32..1dacc1a949 100644 --- a/xls/passes/visibility_expr_builder_test.cc +++ b/xls/passes/visibility_expr_builder_test.cc @@ -260,5 +260,36 @@ TEST_F(VisibilityExprBuilderTest, NotAFunctionOfSelf) { EXPECT_THAT(is_x_used.first, m::Ne(m::Param("y"), m::Literal(7))); } +TEST_F(VisibilityExprBuilderTest, VisibilityExpressionWithOneKeptEdge) { + auto p = CreatePackage(); + FunctionBuilder fb(TestName(), p.get()); + Type* u1 = p->GetBitsType(1); + BValue x = fb.Param("x", u1); + BValue y = fb.Param("y", u1); + + // z == select(y, {false, x}) == and(y, x) + BValue literal_false = fb.Literal(UBits(0, 1)); + BValue z = fb.Select(y, {literal_false, x}); + + BValue ret = fb.Xor(x, z); + XLS_ASSERT_OK_AND_ASSIGN(Function * f, fb.BuildWithReturnValue(ret)); + std::pair is_x_used; + XLS_ASSERT_OK_AND_ASSIGN(is_x_used, + BuildDefaultVisibilityExpr(f, x.node(), {})); + + // The visibility of `x` should be `Literal(1)`, since: + // XOR(x, z) == XOR(x, AND(y, x)), + // and our analysis can't determine the visibility of `x` through `XOR(x, z)`. + // + // In case we later improve the analysis, the correct visibility expression + // for `x` is `y == 0`. We can see this with a casewise analysis: + // If y == 0, then z == AND(y, x) == 0, so XOR(x, z) == XOR(x, 0) == x. + // If y == 1, then z == AND(y, x) == x, so XOR(x, z) == XOR(x, x) == 0. + // So the output is x if y == 0, and 0 if y == 1. Equivalently: + // ret == x & !y + // As such, `x` is actually visible iff `y == 0`. + EXPECT_THAT(is_x_used.first, m::Literal(1)); +} + } // namespace } // namespace xls