On 2/25/19 2:07 AM, Eric Botcazou wrote:
> Hi,
>
> this is a regression present on the mainline and 8 branch, introduced by the
> new code in edge_info::derive_equivalences dealing with BIT_AND_EXPR for SSA
> names with boolean range:
>
> /* If either operand has a boolean range, then we
> know its value must be one, otherwise we just know it
> is nonzero. The former is clearly useful, I haven't
> seen cases where the latter is helpful yet. */
> if (TREE_CODE (rhs1) == SSA_NAME)
> {
> if (ssa_name_has_boolean_range (rhs1))
> {
> value = build_one_cst (TREE_TYPE (rhs1));
> derive_equivalences (rhs1, value, recursion_limit - 1);
> }
> }
> if (TREE_CODE (rhs2) == SSA_NAME)
> {
> if (ssa_name_has_boolean_range (rhs2))
> {
> value = build_one_cst (TREE_TYPE (rhs2));
> derive_equivalences (rhs2, value, recursion_limit - 1);
> }
> }
>
> and visible on the attached Ada testcase at -O1 or above.
>
> The sequence of events is as follows: for boolean types with precision > 1
> (the normal boolean types in Ada), the gimplifier turns a TRUTH_NOT_EXPR into
> a BIT_XOR_EXPR with 1 in order to preserve the 0-or-1-value invariant:
>
> /* The parsers are careful to generate TRUTH_NOT_EXPR
> only with operands that are always zero or one.
> We do not fold here but handle the only interesting case
> manually, as fold may re-introduce the TRUTH_NOT_EXPR. */
> *expr_p = gimple_boolify (*expr_p);
> if (TYPE_PRECISION (TREE_TYPE (*expr_p)) == 1)
> *expr_p = build1_loc (input_location, BIT_NOT_EXPR,
> TREE_TYPE (*expr_p),
> TREE_OPERAND (*expr_p, 0));
> else
> *expr_p = build2_loc (input_location, BIT_XOR_EXPR,
> TREE_TYPE (*expr_p),
> TREE_OPERAND (*expr_p, 0),
> build_int_cst (TREE_TYPE (*expr_p), 1));
>
> Now this TRUTH_NOT_EXPR is part of a conjunction which has been turned into a
> BIT_AND_EXPR by the folder, so this gives BIT_AND_EXPR <BIT_XOR_EXPR <X, 1>>.
>
> After some optimization passes, the second operand of the BIT_AND_EXPR is
> also
> folded into 1 and, consequently, the following match.pd pattern kicks in:
>
> /* Fold (X & Y) ^ Y and (X ^ Y) & Y as ~X & Y. */
> (for opo (bit_and bit_xor)
> opi (bit_xor bit_and)
> (simplify
> (opo:c (opi:c @0 @1) @1)
> (bit_and (bit_not @0) @1)))
>
> and yields BIT_AND_EXPR <BIT_NOT_EXPR, 1>. This is still correct, in the
> sense that the 0-or-1-value invariant is preserved.
>
> Then the new code in edge_info::derive_equivalences above deduces from this
> that the BIT_NOT_EXPR has value 1 on one of the edges. But the same function
> also handles the BIT_NOT_EXPR itself and further deduces that its operand has
> value ~1 or 254 (the precision of boolean types is 8) on this edge, which
> breaks the 0-or-1-value invariant and leads to wrong code downstream.
>
> Given the new code for BIT_AND_EXPR in edge_info::derive_equivalences for
> boolean types, I think that the same special treatment must be added for
> boolean types in the BIT_NOT_EXPR case to preserve the 0-or-1-value invariant.
>
> Bootstrapped/regtested on x86_64-suse-linux, OK for mainline and 8 branch?
>
>
> 2019-02-25 Eric Botcazou <ebotca...@adacore.com>
>
> * tree-ssa-dom.c (edge_info::derive_equivalences) <BIT_IOR_EXPR>: Fix
> and move around comment.
> <BIT_AND_EXPR>: Likewise.
> <BIT_NOT_EXPR>: Add specific handling for boolean types.
>
>
> 2019-02-25 Eric Botcazou <ebotca...@adacore.com>
>
> * gnat.dg/opt77.adb: New test.
> * gnat.dg/opt77_pkg.ad[sb]: Likewise.
Umm, did you look at ssa_name_has_boolean_range? Isn't the problem that
Ada's BOOLEAN_TYPE has a wider range than [0..1] and thus this is the
wrong bit of code:
/* Boolean types always have a range [0..1]. */
if (TREE_CODE (TREE_TYPE (op)) == BOOLEAN_TYPE)
return true;
IIRC there are other places that have similar logic and rely on
ssa_name_has_boolean_range to filter out anything undesirable.
jeff
