[patch] Fix wrong code for boolean negation in condition at -O

[Jeff Law]

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  <ebotcazou@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  <ebotcazou@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