On Mon, Nov 11, 2013 at 4:04 PM, Kenneth Zadeck
<zad...@naturalbridge.com> wrote:
> On 11/11/2013 09:42 AM, Richard Biener wrote:
>>
>> On Mon, Nov 11, 2013 at 3:26 PM, Kenneth Zadeck
>> <zad...@naturalbridge.com> wrote:
>>>
>>> On 11/11/2013 06:49 AM, Richard Biener wrote:
>>>>
>>>> On Sat, Nov 9, 2013 at 3:23 PM, Kenneth Zadeck
>>>> <zad...@naturalbridge.com>
>>>> wrote:
>>>>>
>>>>> On 11/08/2013 05:30 AM, Richard Sandiford wrote:
>>>>>>
>>>>>>    From tree-vrp.c:
>>>>>>
>>>>>> @@ -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
>>>>>> of wide-int?
>>>>>
>>>>> the api for division is different for wide-int than it was for
>>>>> double-int.
>>>>
>>>> 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)
>>>>
>>>> Richard.
>>>
>>> My reading of the code is that is that i changed int_const_binop to
>>> return
>>> null_tree for this case.
>>
>> Trunk has:
>>
>>      case TRUNC_DIV_EXPR:
>>      case FLOOR_DIV_EXPR: case CEIL_DIV_EXPR:
>>      case EXACT_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;
>>            break;
>>          }
>>
>>        /* ... fall through ...  */
>>
>>      case ROUND_DIV_EXPR:
>>        if (op2.is_zero ())
>>          return NULL_TREE;
>>
>> 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
> of int_const_binop_1.
>
> 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));


Thus

    case TRUNC_DIV_EXPR:
    case EXACT_DIV_EXPR:
      res = wi::div_trunc (arg1, arg2, sign, &overflow);
      if (overflow)
        return NULL_TREE;
      break;

should retain the arg2 == 0 checks (and return NULL_TREE) but
otherwise keep overflow handling the same.

Richard.

> Kenny
>
>>
>>> 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.
>>>
>>> Kenny
>>>>>>
>>>>>> Thanks,
>>>>>> Richard
>>>>>
>>>>>
>

Reply via email to