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.
--
Eric BotcazouIndex: tree-ssa-dom.c
===================================================================
--- tree-ssa-dom.c (revision 268994)
+++ tree-ssa-dom.c (working copy)
@@ -170,11 +170,10 @@ edge_info::derive_equivalences (tree nam
gimple *def_stmt = SSA_NAME_DEF_STMT (name);
if (is_gimple_assign (def_stmt))
{
- /* We know the result of DEF_STMT was zero. See if that allows
- us to deduce anything about the SSA_NAMEs used on the RHS. */
enum tree_code code = gimple_assign_rhs_code (def_stmt);
switch (code)
{
+ /* If the result of an OR is zero, then its operands are, too. */
case BIT_IOR_EXPR:
if (integer_zerop (value))
{
@@ -188,8 +187,7 @@ edge_info::derive_equivalences (tree nam
}
break;
- /* We know the result of DEF_STMT was one. See if that allows
- us to deduce anything about the SSA_NAMEs used on the RHS. */
+ /* If the result of an AND is nonzero, then its operands are, too. */
case BIT_AND_EXPR:
if (!integer_zerop (value))
{
@@ -296,7 +294,6 @@ edge_info::derive_equivalences (tree nam
break;
}
-
case EQ_EXPR:
case NE_EXPR:
{
@@ -336,7 +333,28 @@ edge_info::derive_equivalences (tree nam
case NEGATE_EXPR:
{
tree rhs = gimple_assign_rhs1 (def_stmt);
- tree res = fold_build1 (code, TREE_TYPE (rhs), value);
+ tree res;
+ /* If this is a NOT and the operand has a boolean range, then we
+ know its value must be zero or one. We are not supposed to
+ have a BIT_NOT_EXPR for boolean types with precision > 1 in
+ the general case, see e.g. the handling of TRUTH_NOT_EXPR in
+ the gimplifier, but it can be generated by match.pd out of
+ a BIT_XOR_EXPR wrapped in a BIT_AND_EXPR. Now the handling
+ of BIT_AND_EXPR above already forces a specific semantics for
+ boolean types with precision > 1 so we must do the same here,
+ otherwise we could change the semantics of TRUTH_NOT_EXPR for
+ boolean types with precision > 1. */
+ if (code == BIT_NOT_EXPR
+ && TREE_CODE (rhs) == SSA_NAME
+ && ssa_name_has_boolean_range (rhs))
+ {
+ if (integer_zerop (value))
+ res = build_one_cst (TREE_TYPE (rhs));
+ else
+ res = build_zero_cst (TREE_TYPE (rhs));
+ }
+ else
+ res = fold_build1 (code, TREE_TYPE (rhs), value);
derive_equivalences (rhs, res, recursion_limit - 1);
break;
}
-- { dg-do run }
-- { dg-options "-O -fno-inline" }
with Opt77_Pkg; use Opt77_Pkg;
procedure Opt77 is
N : Natural := 0;
To_Add : Boolean;
begin
Proc ("One", N, To_Add);
if To_Add then
raise Program_Error;
end if;
end;
package Opt77_Pkg is
procedure Proc (S : String; N : in out Natural; To_Add : out Boolean);
end Opt77_Pkg;
package body Opt77_Pkg is
function Compare (S : String) return Boolean is
begin
return S = "Two";
end;
procedure Proc (S : String; N : in out Natural; To_Add : out Boolean) is
To_Take : Boolean := False;
To_Read : Boolean := False;
begin
To_Add := False;
if S = "One" then
To_Read := True;
To_Take := Compare (S);
end if;
if To_Read and not To_Take then
N := N + 1;
end if;
if To_Take then
To_Add := True;
end if;
end;
end Opt77_Pkg;
