This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Some wide-int review comments


Richi,

i am having a little trouble putting this back the way that you want. The issue is rem.
what is supposed to happen for INT_MIN % -1?

I would assume because i am failing the last case of gcc.dg/c90-const-expr-8.c that INT_MIN %-1 should not overflow even if INT_MIN / -1 does. however, this appears to be achieved by "accident" on the trunk see (fold-const.c:1053 and below) because there is code in the that is identified as being a special case for speed that selects out the DI and shorter mode cases but allows the TImode to be passed to the double int code. The double int code will then produce overflow for this case because it does not know if this is a div, a mod, or both.

In short, is the TImode code wrong here and we never produce overflow for mod? or is there something magical in the standard about the shorter types?

kenny







On 11/11/2013 10:29 AM, Richard Biener wrote:
On Mon, Nov 11, 2013 at 4:04 PM, Kenneth Zadeck
<zadeck@naturalbridge.com> wrote:
On 11/11/2013 09:42 AM, Richard Biener wrote:
On Mon, Nov 11, 2013 at 3:26 PM, Kenneth Zadeck
<zadeck@naturalbridge.com> wrote:
On 11/11/2013 06:49 AM, Richard Biener wrote:
On Sat, Nov 9, 2013 at 3:23 PM, Kenneth Zadeck
<zadeck@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



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]