[PATCH] Fix fallout from VRP strict-overflow changes

[PATCH] Fix fallout from VRP strict-overflow changes

[Richard Biener]

I was notifed I broke proper handling of undefined overflow in
multiplicative ops handling.  The following resurrects previous
behavior (and adds a testcase).

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2017-08-17  Richard Biener  <rguenther@suse.de>

	* tree-vrp.c (vrp_int_const_binop): Do not set *overflow_p
	to true when overflow is undefined and we saturated the
	result.

	* gcc.dg/tree-ssa/vrp117.c: New testcase.

Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	(revision 251084)
+++ gcc/tree-vrp.c	(working copy)
@@ -1614,6 +1614,8 @@ vrp_int_const_binop (enum tree_code code
   signop sign = TYPE_SIGN (TREE_TYPE (val1));
   wide_int res;
 
+  *overflow_p = false;
+
   switch (code)
     {
     case RSHIFT_EXPR:
@@ -1685,8 +1687,6 @@ vrp_int_const_binop (enum tree_code code
       gcc_unreachable ();
     }
 
-  *overflow_p = overflow;
-
   if (overflow
       && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (val1)))
     {
@@ -1730,6 +1730,8 @@ vrp_int_const_binop (enum tree_code code
 			      TYPE_SIGN (TREE_TYPE (val1)));
     }
 
+  *overflow_p = overflow;
+
   return res;
 }
 
Index: gcc/testsuite/gcc.dg/tree-ssa/vrp117.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/vrp117.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/vrp117.c	(working copy)
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-evrp" } */
+
+void link_error (void);
+
+void foo (int i)
+{
+  if (i > __INT_MAX__ - 10)
+    {
+      int j = i * 10;
+      if (j < i)
+	link_error ();
+    }
+}
+
+/* { dg-final { scan-tree-dump-not "link_error" "evrp" } } */

Re: [PATCH] Fix fallout from VRP strict-overflow changes

[Andreas Schwab]

On Aug 17 2017, Richard Biener <rguenther@suse.de> wrote:

> I was notifed I broke proper handling of undefined overflow in
> multiplicative ops handling.  The following resurrects previous
> behavior (and adds a testcase).
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

This breaks gfortran.dg/alloc_comp_auto_array_2.f90 on aarch64 with
-mabi=ilp32 (only for -O3):

FAIL: gfortran.dg/alloc_comp_auto_array_2.f90   -O3 -g  (test for excess errors)
Excess errors:
/opt/gcc/gcc-20170818/gcc/testsuite/gfortran.dg/alloc_comp_auto_array_2.f90:33:0: Warning: '__builtin_memcpy' specified size between 2147483648 and 4294967295 exceeds maximum object size 2147483647 [-Wstringop-overflow=]

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Fix fallout from VRP strict-overflow changes

[Richard Biener]

On Sat, 19 Aug 2017, Andreas Schwab wrote:

> On Aug 17 2017, Richard Biener <rguenther@suse.de> wrote:
> 
> > I was notifed I broke proper handling of undefined overflow in
> > multiplicative ops handling.  The following resurrects previous
> > behavior (and adds a testcase).
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> 
> This breaks gfortran.dg/alloc_comp_auto_array_2.f90 on aarch64 with
> -mabi=ilp32 (only for -O3):
> 
> FAIL: gfortran.dg/alloc_comp_auto_array_2.f90   -O3 -g  (test for excess errors)
> Excess errors:
> /opt/gcc/gcc-20170818/gcc/testsuite/gfortran.dg/alloc_comp_auto_array_2.f90:33:0: Warning: '__builtin_memcpy' specified size between 2147483648 and 4294967295 exceeds maximum object size 2147483647 [-Wstringop-overflow=]

I believe this is an issue that went latent when I broke VRP earlier.

I have opened PR81908, will amend with some initial analysis.

Richard.

Re: [PATCH] Fix fallout from VRP strict-overflow changes

[Martin Sebor]

On 08/21/2017 01:51 AM, Richard Biener wrote:
> On Sat, 19 Aug 2017, Andreas Schwab wrote:
>
>> On Aug 17 2017, Richard Biener <rguenther@suse.de> wrote:
>>
>>> I was notifed I broke proper handling of undefined overflow in
>>> multiplicative ops handling.  The following resurrects previous
>>> behavior (and adds a testcase).
>>>
>>> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>>
>> This breaks gfortran.dg/alloc_comp_auto_array_2.f90 on aarch64 with
>> -mabi=ilp32 (only for -O3):
>>
>> FAIL: gfortran.dg/alloc_comp_auto_array_2.f90   -O3 -g  (test for excess errors)
>> Excess errors:
>> /opt/gcc/gcc-20170818/gcc/testsuite/gfortran.dg/alloc_comp_auto_array_2.f90:33:0: Warning: '__builtin_memcpy' specified size between 2147483648 and 4294967295 exceeds maximum object size 2147483647 [-Wstringop-overflow=]
>
> I believe this is an issue that went latent when I broke VRP earlier.
>
> I have opened PR81908, will amend with some initial analysis.

FWIW, the core of the problem is that the warning tends to either
expose limitations in optimizations that were not written to make
use of range information, or indicate additional optimization
opportunities due to range information.  In this case, since
the only valid value in the range the memcpy argument is in (i.e.,
~[0, INT_MAX]) is zero, the call could be eliminated.  But this
isn't noticed by any pass until the expander checks the call for
validity.

It seems to me that this could be handled by enhancing gimple-fold
in two ways: 1) fold arguments whose range contains only one valid
value into constants, and 2) transform calls with one or more
arguments entirely in invalid ranges into __builtin_unreachable.

I have been thinking prototyping this solution for a while but
haven't gotten around to it yet so I can't say what problems it
might run into.

Martin

Re: [PATCH] Fix fallout from VRP strict-overflow changes

[Richard Biener]

On Mon, 21 Aug 2017, Martin Sebor wrote:

> On 08/21/2017 01:51 AM, Richard Biener wrote:
> > On Sat, 19 Aug 2017, Andreas Schwab wrote:
> > 
> > > On Aug 17 2017, Richard Biener <rguenther@suse.de> wrote:
> > > 
> > > > I was notifed I broke proper handling of undefined overflow in
> > > > multiplicative ops handling.  The following resurrects previous
> > > > behavior (and adds a testcase).
> > > > 
> > > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> > > 
> > > This breaks gfortran.dg/alloc_comp_auto_array_2.f90 on aarch64 with
> > > -mabi=ilp32 (only for -O3):
> > > 
> > > FAIL: gfortran.dg/alloc_comp_auto_array_2.f90   -O3 -g  (test for excess
> > > errors)
> > > Excess errors:
> > > /opt/gcc/gcc-20170818/gcc/testsuite/gfortran.dg/alloc_comp_auto_array_2.f90:33:0:
> > > Warning: '__builtin_memcpy' specified size between 2147483648 and
> > > 4294967295 exceeds maximum object size 2147483647 [-Wstringop-overflow=]
> > 
> > I believe this is an issue that went latent when I broke VRP earlier.
> > 
> > I have opened PR81908, will amend with some initial analysis.
> 
> FWIW, the core of the problem is that the warning tends to either
> expose limitations in optimizations that were not written to make
> use of range information, or indicate additional optimization
> opportunities due to range information.  In this case, since
> the only valid value in the range the memcpy argument is in (i.e.,
> ~[0, INT_MAX]) is zero, the call could be eliminated.  But this
> isn't noticed by any pass until the expander checks the call for
> validity.
> 
> It seems to me that this could be handled by enhancing gimple-fold
> in two ways: 1) fold arguments whose range contains only one valid
> value into constants, and 2) transform calls with one or more
> arguments entirely in invalid ranges into __builtin_unreachable.
> 
> I have been thinking prototyping this solution for a while but
> haven't gotten around to it yet so I can't say what problems it
> might run into.

Well, there will always be missed optimizations so the correct fix
for this is to not warn about memcpy with size == 0.

Sure, folding can be improved but the get_size_range code is broken
and has to be fixed.

Richard.