Re: [PR 87059] kludge over MIN_EXPR problem causing VRP failure in value ranges
Agreed. Thank you Martin!
On Sat, Aug 25, 2018, 21:19 Jeff Law wrote:
> On 08/24/2018 10:50 AM, Aldy Hernandez wrote:
> > As discussed in the PR, the MIN_EXPR being passed to VRP has
> > incompatible signs. I expect MIN_EXPR to have the same type for all
> > arguments plus the MIN_EXPR node itself, but this is not the case.
> >
> > The culprit on PPC is expand_builtin_strncmp, but fixing it there causes
> > other problems on x86-64 (see PR). I believe Martin Sebor had some
> > questions related to the x86 fallout
> > (https://gcc.gnu.org/ml/gcc/2018-08/msg00164.html).
> >
> > Since it seems this has been broken for a while, and I'd like to unbreak
> > PPC without having to take my patch out, I suggest (for now) just
> > passing the sign of the first argument as VRP had been doing all along
> > (through int_const_binop):
> >
> > int_const_binop():
> > ...
> > tree type = TREE_TYPE (arg1);
> > ...
> > if (TREE_CODE (arg1) == INTEGER_CST && TREE_CODE (arg2) == INTEGER_CST)
> > {
> > wide_int warg1 = wi::to_wide (arg1), res;
> > wide_int warg2 = wi::to_wide (arg2, TYPE_PRECISION (type));
> > success = wide_int_binop (res, code, warg1, warg2, sign,
> );
> > poly_res = res;
> > }
> >
> > At some point later, if someone is sufficiently vexed by broken
> > MIN_EXPR, we could fix it and the x86 fall out.
> >
> > OK pending tests?
> I don't think we need this after fixing the strncmp code...
>
> Jeff
>
>
Re: [PR 87059] kludge over MIN_EXPR problem causing VRP failure in value ranges
On 08/24/2018 10:50 AM, Aldy Hernandez wrote:
> As discussed in the PR, the MIN_EXPR being passed to VRP has
> incompatible signs. I expect MIN_EXPR to have the same type for all
> arguments plus the MIN_EXPR node itself, but this is not the case.
>
> The culprit on PPC is expand_builtin_strncmp, but fixing it there causes
> other problems on x86-64 (see PR). I believe Martin Sebor had some
> questions related to the x86 fallout
> (https://gcc.gnu.org/ml/gcc/2018-08/msg00164.html).
>
> Since it seems this has been broken for a while, and I'd like to unbreak
> PPC without having to take my patch out, I suggest (for now) just
> passing the sign of the first argument as VRP had been doing all along
> (through int_const_binop):
>
> int_const_binop():
> ...
> tree type = TREE_TYPE (arg1);
> ...
> if (TREE_CODE (arg1) == INTEGER_CST && TREE_CODE (arg2) == INTEGER_CST)
> {
> wide_int warg1 = wi::to_wide (arg1), res;
> wide_int warg2 = wi::to_wide (arg2, TYPE_PRECISION (type));
> success = wide_int_binop (res, code, warg1, warg2, sign, );
> poly_res = res;
> }
>
> At some point later, if someone is sufficiently vexed by broken
> MIN_EXPR, we could fix it and the x86 fall out.
>
> OK pending tests?
I don't think we need this after fixing the strncmp code...
Jeff
[PR 87059] kludge over MIN_EXPR problem causing VRP failure in value ranges
As discussed in the PR, the MIN_EXPR being passed to VRP has
incompatible signs. I expect MIN_EXPR to have the same type for all
arguments plus the MIN_EXPR node itself, but this is not the case.
The culprit on PPC is expand_builtin_strncmp, but fixing it there causes
other problems on x86-64 (see PR). I believe Martin Sebor had some
questions related to the x86 fallout
(https://gcc.gnu.org/ml/gcc/2018-08/msg00164.html).
Since it seems this has been broken for a while, and I'd like to unbreak
PPC without having to take my patch out, I suggest (for now) just
passing the sign of the first argument as VRP had been doing all along
(through int_const_binop):
int_const_binop():
...
tree type = TREE_TYPE (arg1);
...
if (TREE_CODE (arg1) == INTEGER_CST && TREE_CODE (arg2) == INTEGER_CST)
{
wide_int warg1 = wi::to_wide (arg1), res;
wide_int warg2 = wi::to_wide (arg2, TYPE_PRECISION (type));
success = wide_int_binop (res, code, warg1, warg2, sign, );
poly_res = res;
}
At some point later, if someone is sufficiently vexed by broken
MIN_EXPR, we could fix it and the x86 fall out.
OK pending tests?
gcc/
PR 87059/tree-optimization
* tree-vrp.c (extract_range_from_binary_expr_1): Pass sign of
first argument to wide_int_range_min_max.
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 735b3646e81..7373011d901 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -1566,6 +1566,15 @@ extract_range_from_binary_expr_1 (value_range *vr,
wide_int vr1_min, vr1_max;
extract_range_into_wide_ints (, sign, prec, vr0_min, vr0_max);
extract_range_into_wide_ints (, sign, prec, vr1_min, vr1_max);
+
+ /* FIXME: Currently the sign of MIN_EXPR and the sign of its
+ arguments are not always the same. Fixing the creators of
+ these faulty nodes caused other problems. For now use the
+ sign of the first argument as VRP was previously doing. See
+ PR87059. */
+ if (vr0.min)
+ sign = TYPE_SIGN (TREE_TYPE (vr0.min));
+
if (wide_int_range_min_max (wmin, wmax, code, sign, prec,
vr0_min, vr0_max, vr1_min, vr1_max))
set_value_range (vr, VR_RANGE,
