This is the mail archive of the
mailing list for the GCC project.
Re: Some wide-int review comments
- From: Kenneth Zadeck <zadeck at naturalbridge dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Mike Stump <mikestump at comcast dot net>, Richard Sandiford <rdsandiford at googlemail dot com>
- Date: Mon, 11 Nov 2013 10:38:03 -0500
- Subject: Re: Some wide-int review comments
- Authentication-results: sourceware.org; auth=none
- References: <87mwlfgp8s dot fsf at talisman dot default> <527E456B dot 4050203 at naturalbridge dot com> <CAFiYyc2qJB3onVWWqBu5sYMQE6Otpmys5U3ZpyQtWAfKOFCcCQ at mail dot gmail dot com> <5280E914 dot 2060501 at naturalbridge dot com> <CAFiYyc0tNHO54aS7PaRKLdsWMQ6qN++MdN8=63Eq1AD5GtHrPg at mail dot gmail dot com> <5280F219 dot 6050000 at naturalbridge dot com> <CAFiYyc0LmDSAtMYqAe4kBr=4WcAzNtQm=cjbeX+1OGGyyjS-=g at mail dot gmail dot com>
On 11/11/2013 10:29 AM, Richard Biener wrote:
I see your point. i did not think that it was necessary to distinguish
the two types of behavior by division.
On Mon, Nov 11, 2013 at 4:04 PM, Kenneth Zadeck
On 11/11/2013 09:42 AM, Richard Biener wrote:
On Mon, Nov 11, 2013 at 3:26 PM, Kenneth Zadeck
On 11/11/2013 06:49 AM, Richard Biener wrote:
On Sat, Nov 9, 2013 at 3:23 PM, Kenneth Zadeck
On 11/08/2013 05:30 AM, Richard Sandiford wrote:
@@ -1893,6 +1884,10 @@ vrp_int_const_binop (enum tree_code code
/* If the singed operation wraps then int_const_binop has done
everything we want. */
+ /* Signed division of -1/0 overflows and by the time it gets here
+ returns NULL_TREE. */
+ else if (!res)
+ return NULL_TREE;
else if ((TREE_OVERFLOW (res)
&& !TREE_OVERFLOW (val1)
&& !TREE_OVERFLOW (val2))
Why is this case different from trunk? Or is it a bug-fix independent
the api for division is different for wide-int than it was for
But this is using a tree API (int_const_binop) that didn't change
(it returned NULL for / 0 previously). So what makes us arrive here
now? (I agree there is a bug in VRP, but it shouldn't manifest itself
only on wide-int)
My reading of the code is that is that i changed int_const_binop to
null_tree for this case.
case FLOOR_DIV_EXPR: case CEIL_DIV_EXPR:
/* This is a shortcut for a common special case. */
if (op2.high == 0 && (HOST_WIDE_INT) op2.low > 0
&& !TREE_OVERFLOW (arg1)
&& !TREE_OVERFLOW (arg2)
&& op1.high == 0 && (HOST_WIDE_INT) op1.low >= 0)
if (code == CEIL_DIV_EXPR)
op1.low += op2.low - 1;
res.low = op1.low / op2.low, res.high = 0;
/* ... fall through ... */
if (op2.is_zero ())
so it already returns NULL_TREE on divide by zero.
I found the reason!!!! This is one of the many "tree-vrp was not properly
tested for TImode bugs."
on the trunk, the case 0/(smallest negative number) case will only trigger
overflow in TImode. On the wide-int branch, tree-vrp works at the
precision of the operands so overflow is triggered properly for this case.
So for HImode, the trunk produces the a result for 0/0x80 and then force_fit
code at the bottom of int_const_binop_1 turns this into an overflow tree
value rather than a null tree.
on the wide-int branch, this case causes the overflow bit to be returned
from the wide-int divide because the overflow case is properly handled for
all types and that overflow is turned into null_tree by the wide-int version
apparently there are no test cases that exercise the true divide by 0 case
but there are test cases that hit the 0/ largest negative number case for
modes smaller than TImode.
You probably mean <largest negative number> / -1? I don't see where
NULL_TREE is returned for any overflow case in int_const_binop_1.
Ah, you made it so. That looks like a bogus change. What's the
reason? int_const_binop_1 is supposed to return a value with
TREE_OVERFLOW set in these cases, also required for frontend
constant folding. Try compiling
const int i = (-__INT_MAX__ - 1) / -1;
which should say
./cc1 -quiet t.c
t.c:1:34: warning: integer overflow in expression [-Woverflow]
const int i = (-__INT_MAX__ - 1) / -1;
but not error or ICE. Seems to work on the branch, but only
because the expression is still folded by
12357 /* X / -1 is -X. */
12358 if (!TYPE_UNSIGNED (type)
12359 && TREE_CODE (arg1) == INTEGER_CST
12360 && wi::eq_p (arg1, -1))
12361 return fold_convert_loc (loc, type, negate_expr (arg0));
res = wi::div_trunc (arg1, arg2, sign, &overflow);
should retain the arg2 == 0 checks (and return NULL_TREE) but
otherwise keep overflow handling the same.
i will make this fix today and test and install it.
On the trunk, only rem returns null_tree for divide by 0, on the wide int
branch, both div and rem return null tree.
I know that this is going to bring on a string of questions that i do not
remember the answers to as to why i made that change. but
fold-const.c:int_const_binop_1 now returns null_tree and this is just
fallout from that change.