On Mon, Feb 25, 2019 at 10:09 AM Eric Botcazou <ebotca...@adacore.com> 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?

OK.

Richard.

>
> 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.
>
> --
> Eric Botcazou

Reply via email to