diff mbox

Some wide-int review comments

Message ID 52962665.60106@naturalbridge.com
State New
Headers show

Commit Message

Kenneth Zadeck Nov. 27, 2013, 5:05 p.m. UTC 
committed as revision 205448 to trunk.
committed as revision 205455 to wide-int branch.
On 11/27/2013 05:50 AM, Richard Biener wrote:
> On Tue, Nov 26, 2013 at 5:33 PM, Kenneth Zadeck
> <zadeck@naturalbridge.com> wrote:
>> Richi,
>>
>> patch ping
> Ok.
>
> Thanks,
> Richard.
>
>> also two more pieces of information.    With further testing, this seems to
>> fix
>>
>> Tests that now work, but didn't before:
>> ===============
>> ext/random/hypergeometric_distribution/operators/values.cc (test for excess
>> errors)
>>
>> New tests that PASS:
>>
>> ext/random/hypergeometric_distribution/operators/values.cc execution test
>> ================
>>
>> also, the corresponding frag for fold-const.c on the wide-int branch will
>> look like
>> ================
>> Index: gcc/fold-const.c
>> ===================================================================
>> --- gcc/fold-const.c    (revision 205224)
>> +++ gcc/fold-const.c    (working copy)
>> @@ -1030,51 +1030,51 @@ int_const_binop_1 (enum tree_code code,
>>
>>       case TRUNC_DIV_EXPR:
>>       case EXACT_DIV_EXPR:
>> -      res = wi::div_trunc (arg1, arg2, sign, &overflow);
>> -      if (overflow)
>> +      if (arg2 == 0)
>>       return NULL_TREE;
>> +      res = wi::div_trunc (arg1, arg2, sign, &overflow);
>>         break;
>>
>>       case FLOOR_DIV_EXPR:
>> -      res = wi::div_floor (arg1, arg2, sign, &overflow);
>> -      if (overflow)
>> +      if (arg2 == 0)
>>       return NULL_TREE;
>> +      res = wi::div_floor (arg1, arg2, sign, &overflow);
>>         break;
>>
>>       case CEIL_DIV_EXPR:
>> -      res = wi::div_ceil (arg1, arg2, sign, &overflow);
>> -      if (overflow)
>> +      if (arg2 == 0)
>>       return NULL_TREE;
>> +      res = wi::div_ceil (arg1, arg2, sign, &overflow);
>>         break;
>>
>>       case ROUND_DIV_EXPR:
>> -      res = wi::div_round (arg1, arg2, sign, &overflow);
>> -      if (overflow)
>> +      if (arg2 == 0)
>>       return NULL_TREE;
>> +      res = wi::div_round (arg1, arg2, sign, &overflow);
>>         break;
>>
>>       case TRUNC_MOD_EXPR:
>> -      res = wi::mod_trunc (arg1, arg2, sign, &overflow);
>> -      if (overflow)
>> +      if (arg2 == 0)
>>       return NULL_TREE;
>> +      res = wi::mod_trunc (arg1, arg2, sign, &overflow);
>>         break;
>>
>>       case FLOOR_MOD_EXPR:
>> -      res = wi::mod_floor (arg1, arg2, sign, &overflow);
>> -      if (overflow)
>> +      if (arg2 == 0)
>>       return NULL_TREE;
>> +      res = wi::mod_floor (arg1, arg2, sign, &overflow);
>>         break;
>>
>>       case CEIL_MOD_EXPR:
>> -      res = wi::mod_ceil (arg1, arg2, sign, &overflow);
>> -      if (overflow)
>> +      if (arg2 == 0)
>>       return NULL_TREE;
>> +      res = wi::mod_ceil (arg1, arg2, sign, &overflow);
>>         break;
>>
>>       case ROUND_MOD_EXPR:
>> -      res = wi::mod_round (arg1, arg2, sign, &overflow);
>> -      if (overflow)
>> +      if (arg2 == 0)
>>       return NULL_TREE;
>> +      res = wi::mod_round (arg1, arg2, sign, &overflow);
>>         break;
>>
>>       case MIN_EXPR:
>>
>> ================
>>
>> On 11/20/2013 06:31 PM, Kenneth Zadeck wrote:
>>> On 11/13/2013 04:59 AM, Richard Biener wrote:
>>>> On Tue, Nov 12, 2013 at 5:43 PM, Kenneth Zadeck
>>>> <zadeck@naturalbridge.com> wrote:
>>>>> On 11/12/2013 11:27 AM, Joseph S. Myers wrote:
>>>>>> On Tue, 12 Nov 2013, Kenneth Zadeck wrote:
>>>>>>
>>>>>>> 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,
>>>>>> Given the conclusion in C11 that a%b should be considered undefined if
>>>>>> a/b
>>>>>> is not representable, I think it's reasonable to say INT_MIN % -1
>>>>>> *should*
>>>>>> be considered to overflow (for all C standard versions) (and bug 30484
>>>>>> is
>>>>>> only a bug for -fwrapv).
>>>>>>
>>>>> however, my local question is what do we want the api to be
>>>>> int-const-binop-1?    The existing behavior seems to be that at least
>>>>> for
>>>>> common modes this function silently returns 0 and it is up to the front
>>>>> ends
>>>>> to put their own spin on it.
>>>> For wide-int you create 1:1 the behavior of current trunk (if a change of
>>>> behavior in TImode is not tested in the testsuite then you can ignore
>>>> that).
>>>> Whatever change you do to semantics of functions you do separately
>>>> from wide-int (preferably first on trunk, or at your choice after the
>>>> wide-int
>>>> merge).
>>>>
>>>> For this case in question I'd say a % -1 should return 0, but for
>>>> INT_MIN % -1 that 0 should have TREE_OVERFLOW set (and
>>>> thus you need to adjust that c90-const-expr-8.c testcase).
>>>>
>>>> Richard.
>>>>
>>>>> kenny
>>> richi,
>>> I have done this exactly as you suggested.   bootstrapped and regression
>>> tested on x86-64.
>>>
>>> 2013-11-20  Kenneth Zadeck  <zadeck@naturalbridge.com>
>>>
>>>      * fold-const.c
>>>      (int_const_binop_1): Make INT_MIN % -1 return 0 with the overflow
>>>      bit set.
>>>
>>>
>>> 2013-11-20  Kenneth Zadeck  <zadeck@naturalbridge.com>
>>>
>>>      * gcc.dg/c90-const-expr-8.c: Look for overflow on INT_MIN % -1.
>>>      * gcc.dg/c99-const-expr-8.c: Look for overflow on INT_MIN % -1.
>>>
>>> ok to commit?
>>>
>>> kenny
>>>
diff mbox

Patch

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 205447)
+++ gcc/fold-const.c	(working copy)
@@ -1110,7 +1110,22 @@  int_const_binop_1 (enum tree_code code,
     case ROUND_MOD_EXPR:
       if (op2.is_zero ())
 	return NULL_TREE;
-      tmp = op1.divmod_with_overflow (op2, uns, code, &res, &overflow);
+
+      /* Check for the case the case of INT_MIN % -1 and return
+       overflow and result = 0.  The TImode case is handled properly
+       in double-int.  */
+      if (TYPE_PRECISION (type) <= HOST_BITS_PER_WIDE_INT 
+	  && !uns
+          && op2.is_minus_one () 
+	  && op1.high == (HOST_WIDE_INT) -1
+	  && (HOST_WIDE_INT) op1.low 
+	  == (((HOST_WIDE_INT)-1) << (TYPE_PRECISION (type) - 1)))
+	{
+	  overflow = 1;
+	  res = double_int_zero;
+	}
+      else
+	tmp = op1.divmod_with_overflow (op2, uns, code, &res, &overflow);
       break;
 
     case MIN_EXPR:
Index: gcc/testsuite/gcc.dg/c90-const-expr-8.c
===================================================================
--- gcc/testsuite/gcc.dg/c90-const-expr-8.c	(revision 205447)
+++ gcc/testsuite/gcc.dg/c90-const-expr-8.c	(working copy)
@@ -23,5 +23,6 @@  enum e {
   /* { dg-error "3:overflow in constant expression" "constant" { target *-*-* } 22 } */
   E6 = 0 * !-INT_MIN, /* { dg-warning "13:integer overflow in expression" } */
   /* { dg-error "8:not an integer constant" "constant" { target *-*-* } 24 } */
-  E7 = INT_MIN % -1 /* Not an overflow.  */
+  E7 = INT_MIN % -1 /* { dg-warning "16:integer overflow in expression" } */
+  /* { dg-error "1:overflow in constant expression" "constant" { target *-*-* } 28 } */
 };
Index: gcc/testsuite/gcc.dg/c99-const-expr-8.c
===================================================================
--- gcc/testsuite/gcc.dg/c99-const-expr-8.c	(revision 205447)
+++ gcc/testsuite/gcc.dg/c99-const-expr-8.c	(working copy)
@@ -23,5 +23,6 @@  enum e {
   /* { dg-error "overflow in constant expression" "constant" { target *-*-* } 22 } */
   E6 = 0 * !-INT_MIN, /* { dg-warning "integer overflow in expression" } */
   /* { dg-error "not an integer constant" "constant" { target *-*-* } 24 } */
-  E7 = INT_MIN % -1 /* Not an overflow.  */
+  E7 = INT_MIN % -1 /* { dg-warning "16:integer overflow in expression" } */
+  /* { dg-error "1:overflow in constant expression" "constant" { target *-*-* } 28 } */
 };