On Tue, Oct 18, 2016 at 8:35 AM, Eric Botcazou <ebotca...@adacore.com> wrote:
> Hi,
>
> this is a regression present on the mainline and 6 branch: the compiler now
> generates wrong code for the attached testcase at -O because of an internal
> conflict about boolean types. The sequence is as follows. In .mergephi3:
>
> boolean _22;
> p__enum res;
>
> <bb 9>:
> if (_22 != 0)
> goto <bb 10>;
> else
> goto <bb 11>;
>
> <bb 10>:
>
> <bb 11>:
> # res_17 = PHI <2(8), 0(9), 1(10)>
>
> is turned into:
>
> COND_EXPR in block 9 and PHI in block 11 converted to straightline code.
> PHI res_17 changed to factor conversion out from COND_EXPR.
> New stmt with CAST that defines res_17.
>
> boolean _33;
>
> <bb 10>:
> # _33 = PHI <2(8), _22(9)>
> res_17 = (p__enum) _33;
>
> [...]
>
> <bb 12>:
> if (res_17 != 0)
> goto <bb 13>;
> else
> goto <bb 14>;
>
> <bb 13>:
> _12 = res_17 == 2;
> _13 = (integer) _12
>
> in .phiopt1. So boolean _33 can have value 2. Later forwprop3 propagates _33
> into the uses of res_17:
>
> <bb 12>:
> if (_33 != 0)
> goto <bb 13>;
> else
> goto <bb 14>;
>
> <bb 13>:
> _12 = _33 == 2;
> _13 = (integer) _12;
>
> and DOM3 deduces:
>
> <bb 13>:
> _12 = 0;
> _13 = 0;
>
> because it computes that _33 has value 1 in BB 13 since it's a boolean.
>
> The problem was introduced by the new factor_out_conditional_conversion:
>
> /* If arg1 is an INTEGER_CST, fold it to new type. */
> if (INTEGRAL_TYPE_P (TREE_TYPE (new_arg0))
> && int_fits_type_p (arg1, TREE_TYPE (new_arg0)))
> {
> if (gimple_assign_cast_p (arg0_def_stmt))
> new_arg1 = fold_convert (TREE_TYPE (new_arg0), arg1);
> else
> return NULL;
> }
> else
> return NULL
>
> int_fits_type_p is documented as taking only INTEGER_TYPE, but is invoked
> on constant 2 and a BOOLEAN_TYPE and returns true.
>
> BOOLEAN_TYPE is special in Ada: it has precision 8 and range [0;255] so the
> outcome of int_fits_type_p is not unreasonable. But this goes against the
> various transformations applied to boolean types in the compiler, which all
> assume that they can only take values 0 or 1.
>
> Hence the attached fix (which should be a no-op except for Ada), tested on
> x86_64-suse-linux, OK for mainline and 6 branch?
Hmm, I wonder if the patch is a good approach as you are likely only papering
over a single of possibly very many affected places (wi::fits_to_tree_p comes
immediately to my mind). I suppose a better way would be for Ada to not
lie about those types and not use BOOLEAN_TYPE but INTEGER_TYPE.
Because BOOLEAN_TYPE types only have two values as documented in
tree.def:
/* Boolean type (true or false are the only values). Looks like an
INTEGRAL_TYPE. */
DEFTREECODE (BOOLEAN_TYPE, "boolean_type", tcc_type, 0)
There are not many references to BOOLEAN_TYPE in ada/gcc-interface
thus it shouldn't be hard to change ... (looks like Ada might even prefer
ENUMERAL_TYPE here).
Thanks,
Richard.
>
> 2016-10-18 Eric Botcazou <ebotca...@adacore.com>
>
> * tree.c (int_fits_type_p): Accept only 0 and 1 for boolean types.
>
>
> 2016-10-18 Eric Botcazou <ebotca...@adacore.com>
>
> * gnat.dg/opt59.adb: New test.
> * gnat.dg/opt59_pkg.ad[sb]: New helper.
>
> --
> Eric Botcazou
