Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Manish Goregaokar]

Yep, there are some test issues -- I don't have time right now but
plan to investigate further later.
-Manish


On Sat, Jul 9, 2016 at 12:06 AM, Jeff Law  wrote:
> On 07/06/2016 11:22 AM, Bernd Schmidt wrote:
>>
>> On 07/05/2016 12:41 PM, Richard Biener wrote:
>>>
>>> On Fri, Jul 1, 2016 at 3:10 PM, Manish Goregaokar 
>>> wrote:

 Added a test:
>>>
>>>
>>> Ok if this passed bootstrap/regtest.
>>
>>
 +  return flag_delete_null_pointer_checks
 +&& (tree_expr_nonzero_warnv_p (op0, strict_overflow_p)
 +|| tree_expr_nonzero_warnv_p (op1, strict_overflow_p));
  case PLUS_EXPR:
>>
>>
>> But please fix the wrapping - multi-line expressions like this should be
>> enclosed in parentheses to make the editor deal with them correctly.
>
> I believe this patch regresses several tests in constexpr-array-ptr10.C.
>
> jeff
>

Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Jeff Law]

On 07/06/2016 11:22 AM, Bernd Schmidt wrote:

On 07/05/2016 12:41 PM, Richard Biener wrote:

On Fri, Jul 1, 2016 at 3:10 PM, Manish Goregaokar 
wrote:

Added a test:


Ok if this passed bootstrap/regtest.



+  return flag_delete_null_pointer_checks
+&& (tree_expr_nonzero_warnv_p (op0, strict_overflow_p)
+|| tree_expr_nonzero_warnv_p (op1, strict_overflow_p));
 case PLUS_EXPR:


But please fix the wrapping - multi-line expressions like this should be
enclosed in parentheses to make the editor deal with them correctly.

I believe this patch regresses several tests in constexpr-array-ptr10.C.

jeff

Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Bernd Schmidt]

On 07/05/2016 12:41 PM, Richard Biener wrote:

On Fri, Jul 1, 2016 at 3:10 PM, Manish Goregaokar  wrote:

Added a test:


Ok if this passed bootstrap/regtest.



+  return flag_delete_null_pointer_checks
+&& (tree_expr_nonzero_warnv_p (op0, strict_overflow_p)
+|| tree_expr_nonzero_warnv_p (op1, strict_overflow_p));
 case PLUS_EXPR:


But please fix the wrapping - multi-line expressions like this should be 
enclosed in parentheses to make the editor deal with them correctly.



Bernd

Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Richard Biener]

On Fri, Jul 1, 2016 at 3:10 PM, Manish Goregaokar  wrote:
> Added a test:

Ok if this passed bootstrap/regtest.

Richard.

> gcc/ChangeLog:
> PR c/71699
> * fold-const.c (tree_binary_nonzero_warnv_p): Allow
> pointer addition to also be considered nonzero.
>
> gcc/testsuite/ChangeLog:
> PR c/71699
> * c-c++-common/pointer-addition-nonnull.c: New test for
> pointer addition.
> ---
>  gcc/fold-const.c|  3 +++
>  .../c-c++-common/pointer-addition-nonnull.c | 21 
> +
>  2 files changed, 24 insertions(+)
>  create mode 100644 gcc/testsuite/c-c++-common/pointer-addition-nonnull.c
>
> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> index 3b9500d..0d82018 100644
> --- a/gcc/fold-const.c
> +++ b/gcc/fold-const.c
> @@ -13199,6 +13199,9 @@ tree_binary_nonzero_warnv_p (enum tree_code code,
>switch (code)
>  {
>  case POINTER_PLUS_EXPR:
> +  return flag_delete_null_pointer_checks
> +&& (tree_expr_nonzero_warnv_p (op0, strict_overflow_p)
> +|| tree_expr_nonzero_warnv_p (op1, strict_overflow_p));
>  case PLUS_EXPR:
>if (ANY_INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_UNDEFINED (type))
>  {
> diff --git a/gcc/testsuite/c-c++-common/pointer-addition-nonnull.c
> b/gcc/testsuite/c-c++-common/pointer-addition-nonnull.c
> new file mode 100644
> index 000..10bc04c
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/pointer-addition-nonnull.c
> @@ -0,0 +1,21 @@
> +/* { dg-options "-c -O2 -Wall" } */
> +
> +char *xstrdup (const char *) __attribute__ ((__returns_nonnull__));
> +
> +#define PREFIX "some "
> +
> +int
> +main ()
> +{
> +  char *saveptr;
> +  char *name = xstrdup (PREFIX "name");
> +
> +  char *tail = name + sizeof (PREFIX) - 1;
> +
> +  if (tail == 0)
> +tail = saveptr;
> +  while (*tail == ' ')
> +++tail;
> +
> +  return 0;
> +}
> \ No newline at end of file
> --
> 2.8.3
>
> -Manish
>
>
> On Fri, Jul 1, 2016 at 1:40 PM, Richard Biener
>  wrote:
>> On Thu, Jun 30, 2016 at 5:14 PM, Marc Glisse  wrote:
>>> On Thu, 30 Jun 2016, Richard Biener wrote:
>>>
 points-to analysis already has the constraint that POINTER_PLUS_EXPR
 cannot leave the object op0 points to.  Of course currently nothing uses 
 the
 fact whether points-to computes pointed-to as nothing (aka NULL) - so the
 argument may be moot.

 Anyway, one of my points to the original patch was that POINTER_PLUS_EXPR
 handling should be clearly separate from PLUS_EXPR and that we have
 flag_delete_null_pointer_checks to allow targest to declare that 0 is a
 valid
 object pointer (and thus you can do 4 + -4 and reach NULL).
>>>
>>>
>>> Thanks. So the tricky point is that we are not allowed to transform g into f
>>> below:
>>>
>>> char*f(char*p){return p+4;}
>>> char*g(char*p){return (char*)((intptr_t)p+4);}
>>>
>>> That makes sense and seems much easier to guarantee than I feared, nice.
>>>
>>> (on the other hand, only RTL is able to simplify (long)p+4-(long)(p+4))
>>
>> Hmm, yeah - we have some match.pd stuff to handle these kind of cases,
>> like p + ((long)p2 - (long)p1)) and also (long)(p + x) - (long)p.
>>
>> OTOH to handle (long)p + 4 - (long)(p + 4) the only thing we need is to
>> transform (long)(p + 4) to (long)p + 4 ... that would simplify things but
>> of course we cannot ever undo that canonicalization if the result is
>> ever converted back to a pointer.  But maybe we can value-number it
>> the same with some tricks... (might be worth to file a bugreport)
>>
>> Richard.
>>
>>> --
>>> Marc Glisse

Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Mike Stump]

On Jul 1, 2016, at 6:10 AM, Manish Goregaokar  wrote:
> 
> +}
> \ No newline at end of file

Minor nit, please end all files with a newline...

Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Manish Goregaokar]

Added a test:

gcc/ChangeLog:
PR c/71699
* fold-const.c (tree_binary_nonzero_warnv_p): Allow
pointer addition to also be considered nonzero.

gcc/testsuite/ChangeLog:
PR c/71699
* c-c++-common/pointer-addition-nonnull.c: New test for
pointer addition.
---
 gcc/fold-const.c|  3 +++
 .../c-c++-common/pointer-addition-nonnull.c | 21 +
 2 files changed, 24 insertions(+)
 create mode 100644 gcc/testsuite/c-c++-common/pointer-addition-nonnull.c

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 3b9500d..0d82018 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -13199,6 +13199,9 @@ tree_binary_nonzero_warnv_p (enum tree_code code,
   switch (code)
 {
 case POINTER_PLUS_EXPR:
+  return flag_delete_null_pointer_checks
+&& (tree_expr_nonzero_warnv_p (op0, strict_overflow_p)
+|| tree_expr_nonzero_warnv_p (op1, strict_overflow_p));
 case PLUS_EXPR:
   if (ANY_INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_UNDEFINED (type))
 {
diff --git a/gcc/testsuite/c-c++-common/pointer-addition-nonnull.c
b/gcc/testsuite/c-c++-common/pointer-addition-nonnull.c
new file mode 100644
index 000..10bc04c
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pointer-addition-nonnull.c
@@ -0,0 +1,21 @@
+/* { dg-options "-c -O2 -Wall" } */
+
+char *xstrdup (const char *) __attribute__ ((__returns_nonnull__));
+
+#define PREFIX "some "
+
+int
+main ()
+{
+  char *saveptr;
+  char *name = xstrdup (PREFIX "name");
+
+  char *tail = name + sizeof (PREFIX) - 1;
+
+  if (tail == 0)
+tail = saveptr;
+  while (*tail == ' ')
+++tail;
+
+  return 0;
+}
\ No newline at end of file
-- 
2.8.3

-Manish


On Fri, Jul 1, 2016 at 1:40 PM, Richard Biener
 wrote:
> On Thu, Jun 30, 2016 at 5:14 PM, Marc Glisse  wrote:
>> On Thu, 30 Jun 2016, Richard Biener wrote:
>>
>>> points-to analysis already has the constraint that POINTER_PLUS_EXPR
>>> cannot leave the object op0 points to.  Of course currently nothing uses the
>>> fact whether points-to computes pointed-to as nothing (aka NULL) - so the
>>> argument may be moot.
>>>
>>> Anyway, one of my points to the original patch was that POINTER_PLUS_EXPR
>>> handling should be clearly separate from PLUS_EXPR and that we have
>>> flag_delete_null_pointer_checks to allow targest to declare that 0 is a
>>> valid
>>> object pointer (and thus you can do 4 + -4 and reach NULL).
>>
>>
>> Thanks. So the tricky point is that we are not allowed to transform g into f
>> below:
>>
>> char*f(char*p){return p+4;}
>> char*g(char*p){return (char*)((intptr_t)p+4);}
>>
>> That makes sense and seems much easier to guarantee than I feared, nice.
>>
>> (on the other hand, only RTL is able to simplify (long)p+4-(long)(p+4))
>
> Hmm, yeah - we have some match.pd stuff to handle these kind of cases,
> like p + ((long)p2 - (long)p1)) and also (long)(p + x) - (long)p.
>
> OTOH to handle (long)p + 4 - (long)(p + 4) the only thing we need is to
> transform (long)(p + 4) to (long)p + 4 ... that would simplify things but
> of course we cannot ever undo that canonicalization if the result is
> ever converted back to a pointer.  But maybe we can value-number it
> the same with some tricks... (might be worth to file a bugreport)
>
> Richard.
>
>> --
>> Marc Glisse

Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Richard Biener]

On Thu, Jun 30, 2016 at 5:14 PM, Marc Glisse  wrote:
> On Thu, 30 Jun 2016, Richard Biener wrote:
>
>> points-to analysis already has the constraint that POINTER_PLUS_EXPR
>> cannot leave the object op0 points to.  Of course currently nothing uses the
>> fact whether points-to computes pointed-to as nothing (aka NULL) - so the
>> argument may be moot.
>>
>> Anyway, one of my points to the original patch was that POINTER_PLUS_EXPR
>> handling should be clearly separate from PLUS_EXPR and that we have
>> flag_delete_null_pointer_checks to allow targest to declare that 0 is a
>> valid
>> object pointer (and thus you can do 4 + -4 and reach NULL).
>
>
> Thanks. So the tricky point is that we are not allowed to transform g into f
> below:
>
> char*f(char*p){return p+4;}
> char*g(char*p){return (char*)((intptr_t)p+4);}
>
> That makes sense and seems much easier to guarantee than I feared, nice.
>
> (on the other hand, only RTL is able to simplify (long)p+4-(long)(p+4))

Hmm, yeah - we have some match.pd stuff to handle these kind of cases,
like p + ((long)p2 - (long)p1)) and also (long)(p + x) - (long)p.

OTOH to handle (long)p + 4 - (long)(p + 4) the only thing we need is to
transform (long)(p + 4) to (long)p + 4 ... that would simplify things but
of course we cannot ever undo that canonicalization if the result is
ever converted back to a pointer.  But maybe we can value-number it
the same with some tricks... (might be worth to file a bugreport)

Richard.

> --
> Marc Glisse

Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Manish Goregaokar]

This is the full test failure summary. I will compare with the
previous commit tomorrow. The asan failure and the uninit-19 failure
are interesting. uninit-19 should not have failed, but I have no idea
what's going on with asan. I'll have a closer look tomorrow and look
for other failing tests.

cat <<'EOF' |
Native configuration is x86_64-apple-darwin15.3.0

=== g++ tests ===


Running target unix
FAIL: g++.dg/debug/dwarf2/imported-decl-2.C  -std=gnu++98
scan-assembler-times ascii "0".*ascii "0".*DIE
.0x[0-9a-f]*. DW_TAG_imported_declaration 1
FAIL: g++.dg/debug/dwarf2/imported-decl-2.C  -std=gnu++11
scan-assembler-times ascii "0".*ascii "0".*DIE
.0x[0-9a-f]*. DW_TAG_imported_declaration 1
FAIL: g++.dg/debug/dwarf2/imported-decl-2.C  -std=gnu++14
scan-assembler-times ascii "0".*ascii "0".*DIE
.0x[0-9a-f]*. DW_TAG_imported_declaration 1
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++11  (test for
errors, line 92)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++11  (test for
errors, line 93)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++11  (test for
errors, line 94)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++11  (test for
errors, line 95)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++11  (test for
errors, line 96)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++11  (test for
errors, line 98)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++11  (test for
errors, line 99)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++11  (test for
errors, line 100)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++11  (test for
errors, line 101)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++11  (test for
errors, line 102)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++14  (test for
errors, line 92)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++14  (test for
errors, line 93)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++14  (test for
errors, line 94)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++14  (test for
errors, line 95)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++14  (test for
errors, line 96)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++14  (test for
errors, line 98)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++14  (test for
errors, line 99)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++14  (test for
errors, line 100)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++14  (test for
errors, line 101)
FAIL: g++.dg/cpp0x/constexpr-array-ptr10.C  -std=c++14  (test for
errors, line 102)
FAIL: g++.dg/ext/sync-4.C  -std=gnu++98 execution test
FAIL: g++.dg/ext/sync-4.C  -std=gnu++11 execution test
FAIL: g++.dg/ext/sync-4.C  -std=gnu++14 execution test
FAIL: g++.dg/ipa/pr67056.C   scan-ipa-dump cp "Speculative outer
type:struct CompositeClass"
FAIL: c-c++-common/pr60226.c  -std=gnu++98 (test for excess errors)
FAIL: c-c++-common/pr60226.c  -std=gnu++11 (test for excess errors)
FAIL: c-c++-common/pr60226.c  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/tree-prof/pr57451.C compilation,  -fprofile-generate
-D_PROFILE_GENERATE
UNRESOLVED: g++.dg/tree-prof/pr57451.C execution,
-fprofile-generate -D_PROFILE_GENERATE
UNRESOLVED: g++.dg/tree-prof/pr57451.C compilation,  -fprofile-use
-D_PROFILE_USE
UNRESOLVED: g++.dg/tree-prof/pr57451.C execution,-fprofile-use
-D_PROFILE_USE
FAIL: g++.dg/tree-prof/pr63581.C compilation,  -fprofile-generate
-D_PROFILE_GENERATE
UNRESOLVED: g++.dg/tree-prof/pr63581.C execution,
-fprofile-generate -D_PROFILE_GENERATE
UNRESOLVED: g++.dg/tree-prof/pr63581.C compilation,  -fprofile-use
-D_PROFILE_USE
UNRESOLVED: g++.dg/tree-prof/pr63581.C execution,-fprofile-use
-D_PROFILE_USE

=== g++ Summary ===

# of expected passes98650
# of unexpected failures32
# of expected failures314
# of unresolved testcases6
# of unsupported tests4169
/Users/manishearth/dev/gcc-build/gcc/testsuite/g++/../../xg++  version
7.0.0 20160630 (experimental) (GCC)

=== gcc tests ===


Running target unix
FAIL: c-c++-common/asan/strncpy-overflow-1.c   -O0  execution test
FAIL: gcc.dg/debug/dwarf2/prod-options.c scan-assembler DW_AT_producer: "GNU C
FAIL: gcc.dg/darwin-minversion-1.c (test for excess errors)
FAIL: gcc.dg/darwin-minversion-2.c (test for excess errors)
FAIL: gcc.dg/darwin-version-1.c (test for excess errors)
FAIL: gcc.dg/framework-1.c (test for excess errors)
FAIL: gcc.dg/uninit-19.c  (test for warnings, line 22)
FAIL: gcc.dg/uninit-19.c (test for excess errors)
FAIL: c-c++-common/pr60226.c  -Wc++-compat  (test for excess errors)
FAIL: gcc.dg/graphite/scop-19.c scan-tree-dump-times graphite "number
of SCoPs: 0" 1
FAIL: gcc.dg/torture/pr68264.c   -O0  execution test
FAIL: gcc.dg/torture/pr68264.c   -O1  execution test
FAIL: gcc.dg/torture/pr68264.c   -O2  execution test
FAIL: gcc.dg/torture/pr68264.c   -O3 -g  execution test
FAIL: gcc.dg/torture/pr68264.c   -Os  execution test
FAIL: 

Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Marc Glisse]

On Thu, 30 Jun 2016, Richard Biener wrote:

points-to analysis already has the constraint that POINTER_PLUS_EXPR 
cannot leave the object op0 points to.  Of course currently nothing uses 
the fact whether points-to computes pointed-to as nothing (aka NULL) - 
so the argument may be moot.


Anyway, one of my points to the original patch was that POINTER_PLUS_EXPR
handling should be clearly separate from PLUS_EXPR and that we have
flag_delete_null_pointer_checks to allow targest to declare that 0 is a valid
object pointer (and thus you can do 4 + -4 and reach NULL).


Thanks. So the tricky point is that we are not allowed to transform g into 
f below:


char*f(char*p){return p+4;}
char*g(char*p){return (char*)((intptr_t)p+4);}

That makes sense and seems much easier to guarantee than I feared, nice.

(on the other hand, only RTL is able to simplify (long)p+4-(long)(p+4))

--
Marc Glisse

Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Manish Goregaokar]

I'm getting a failure on
./gcc/testsuite/c-c++-common/asan/strncpy-overflow-1.c with the latest
patch. Unsure if it's related; we're not doing any pointer arithmetic
there.

I'll test it again on trunk without the patch and see what happens.
-Manish


On Thu, Jun 30, 2016 at 7:18 PM, Richard Biener
 wrote:
> On Thu, Jun 30, 2016 at 3:38 PM, Marc Glisse  wrote:
>> On Thu, 30 Jun 2016, Manish Goregaokar wrote:
>>
>>> Alright, the following patch was tested and it works
>>>
>>> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
>>> index 3b9500d..0d82018 100644
>>> --- a/gcc/fold-const.c
>>> +++ b/gcc/fold-const.c
>>> @@ -13199,6 +13199,9 @@ tree_binary_nonzero_warnv_p (enum tree_code code,
>>>   switch (code)
>>> {
>>> case POINTER_PLUS_EXPR:
>>> +  return flag_delete_null_pointer_checks
>>> +&& (tree_expr_nonzero_warnv_p (op0, strict_overflow_p)
>>> +|| tree_expr_nonzero_warnv_p (op1, strict_overflow_p));
>>> case PLUS_EXPR:
>>>   if (ANY_INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_UNDEFINED (type))
>>> {
>>
>>
>> So, what prevents us from deciding that p+(0-p) is nonzero when p is? Not
>> sure if it is strictly legal in all languages, but I wouldn't be surprised
>> if there was at least one language where optimizations could lead to such
>> expressions.
>>
>> I think this is an exciting optimization, but I was always too scared to try
>> anything like this.
>
> ;)
>
> points-to analysis already has the constraint that POINTER_PLUS_EXPR cannot
> leave the object op0 points to.  Of course currently nothing uses the
> fact whether
> points-to computes pointed-to as nothing (aka NULL) - so the argument
> may be moot.
>
> Anyway, one of my points to the original patch was that POINTER_PLUS_EXPR
> handling should be clearly separate from PLUS_EXPR and that we have
> flag_delete_null_pointer_checks to allow targest to declare that 0 is a valid
> object pointer (and thus you can do 4 + -4 and reach NULL).
>
> Richard.
>
>> --
>> Marc Glisse

Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Richard Biener]

On Thu, Jun 30, 2016 at 3:38 PM, Marc Glisse  wrote:
> On Thu, 30 Jun 2016, Manish Goregaokar wrote:
>
>> Alright, the following patch was tested and it works
>>
>> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
>> index 3b9500d..0d82018 100644
>> --- a/gcc/fold-const.c
>> +++ b/gcc/fold-const.c
>> @@ -13199,6 +13199,9 @@ tree_binary_nonzero_warnv_p (enum tree_code code,
>>   switch (code)
>> {
>> case POINTER_PLUS_EXPR:
>> +  return flag_delete_null_pointer_checks
>> +&& (tree_expr_nonzero_warnv_p (op0, strict_overflow_p)
>> +|| tree_expr_nonzero_warnv_p (op1, strict_overflow_p));
>> case PLUS_EXPR:
>>   if (ANY_INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_UNDEFINED (type))
>> {
>
>
> So, what prevents us from deciding that p+(0-p) is nonzero when p is? Not
> sure if it is strictly legal in all languages, but I wouldn't be surprised
> if there was at least one language where optimizations could lead to such
> expressions.
>
> I think this is an exciting optimization, but I was always too scared to try
> anything like this.

;)

points-to analysis already has the constraint that POINTER_PLUS_EXPR cannot
leave the object op0 points to.  Of course currently nothing uses the
fact whether
points-to computes pointed-to as nothing (aka NULL) - so the argument
may be moot.

Anyway, one of my points to the original patch was that POINTER_PLUS_EXPR
handling should be clearly separate from PLUS_EXPR and that we have
flag_delete_null_pointer_checks to allow targest to declare that 0 is a valid
object pointer (and thus you can do 4 + -4 and reach NULL).

Richard.

> --
> Marc Glisse

Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Marc Glisse]

On Thu, 30 Jun 2016, Manish Goregaokar wrote:


Alright, the following patch was tested and it works

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 3b9500d..0d82018 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -13199,6 +13199,9 @@ tree_binary_nonzero_warnv_p (enum tree_code code,
  switch (code)
{
case POINTER_PLUS_EXPR:
+  return flag_delete_null_pointer_checks
+&& (tree_expr_nonzero_warnv_p (op0, strict_overflow_p)
+|| tree_expr_nonzero_warnv_p (op1, strict_overflow_p));
case PLUS_EXPR:
  if (ANY_INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_UNDEFINED (type))
{


So, what prevents us from deciding that p+(0-p) is nonzero when p is? Not 
sure if it is strictly legal in all languages, but I wouldn't be surprised 
if there was at least one language where optimizations could lead to such 
expressions.


I think this is an exciting optimization, but I was always too scared to 
try anything like this.


--
Marc Glisse

Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Manish Goregaokar]

Alright, the following patch was tested and it works

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 3b9500d..0d82018 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -13199,6 +13199,9 @@ tree_binary_nonzero_warnv_p (enum tree_code code,
   switch (code)
 {
 case POINTER_PLUS_EXPR:
+  return flag_delete_null_pointer_checks
+&& (tree_expr_nonzero_warnv_p (op0, strict_overflow_p)
+|| tree_expr_nonzero_warnv_p (op1, strict_overflow_p));
 case PLUS_EXPR:
   if (ANY_INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_UNDEFINED (type))
 {
-- 
2.8.3


-Manish


On Thu, Jun 30, 2016 at 6:45 PM, Richard Biener
 wrote:
> On Thu, Jun 30, 2016 at 2:03 PM, Manish Goregaokar  wrote:
>> What about ptr + intptr_t? I guess we should check nonnegative for op1 then?
>
> op1 will always be nonnegative as it is forced to sizetype type (which
> is unsigned).
>
> Richard.
>
>> -Manish
>>
>>
>> On Thu, Jun 30, 2016 at 5:25 PM, Richard Biener
>>  wrote:
>>> On Thu, Jun 30, 2016 at 1:17 PM, Manish Goregaokar  
>>> wrote:
 gcc/ChangeLog:
 PR c/71699
 * fold-const.c (tree_binary_nonzero_warnv_p): Allow
 pointer addition to also be considered nonzero.
 ---
  gcc/fold-const.c | 20 +---
  1 file changed, 13 insertions(+), 7 deletions(-)

 diff --git a/gcc/fold-const.c b/gcc/fold-const.c
 index 3b9500d..eda713e 100644
 --- a/gcc/fold-const.c
 +++ b/gcc/fold-const.c
 @@ -13200,16 +13200,22 @@ tree_binary_nonzero_warnv_p (enum tree_code code,
  {
  case POINTER_PLUS_EXPR:
  case PLUS_EXPR:
 -  if (ANY_INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_UNDEFINED (type))
 +  if ((ANY_INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_UNDEFINED (type))
 +  || POINTER_TYPE_P (type))
  {
 +  /* Pointers are always nonnegative, check integers.  */
 +  if (ANY_INTEGRAL_TYPE_P (type))
 +  {
 +  sub_strict_overflow_p = false;
 +  if (!tree_expr_nonnegative_warnv_p (op0,
 +  _strict_overflow_p)
 +  || !tree_expr_nonnegative_warnv_p (op1,
 + _strict_overflow_p))
 +return false;
 +  }
/* With the presence of negative values it is hard
   to say something.  */
 -  sub_strict_overflow_p = false;
 -  if (!tree_expr_nonnegative_warnv_p (op0,
 -  _strict_overflow_p)
 -  || !tree_expr_nonnegative_warnv_p (op1,
 - _strict_overflow_p))
 -return false;
 +
>>>
>>> Hmm, note that op1 of POINTER_PLUS_EXPR _is_ an integer that needs to be 
>>> treated
>>> as signed.
>>>
>>> POINTER_PLUS_EXPR is special in other ways in that iff
>>> flag_delete_null_pointer_checks
>>> is set we can assume that ptr + non-zero is never zero.  Thus simply do
>>>
>>>case POINTER_PLUS_EXPR:
>>>   return flag_delete_null_pointer_checks
>>>&& (tree_expr_nonzero_warnv_p (op0, strict_overflow_p)
>>>  || tree_expr_nonzero_warnv_p (op1, strict_overflow_p));
>>>
>>> OTOH ptr + -(uintptr_t)ptr is valid from a POINTER_PLUS_EXPR
>>> perspective as GCC does
>>> not have a special tree code for pointer subtraction (but IIRC it uses
>>> MINUS_EXPR on integers
>>> for this).
>>>
>>> Richard.
>>>
>>>
/* One of operands must be positive and the other non-negative.  */
/* We don't set *STRICT_OVERFLOW_P here: even if this value
   overflows, on a twos-complement machine the sum of two
 --
 2.8.3

Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Richard Biener]

On Thu, Jun 30, 2016 at 2:03 PM, Manish Goregaokar  wrote:
> What about ptr + intptr_t? I guess we should check nonnegative for op1 then?

op1 will always be nonnegative as it is forced to sizetype type (which
is unsigned).

Richard.

> -Manish
>
>
> On Thu, Jun 30, 2016 at 5:25 PM, Richard Biener
>  wrote:
>> On Thu, Jun 30, 2016 at 1:17 PM, Manish Goregaokar  
>> wrote:
>>> gcc/ChangeLog:
>>> PR c/71699
>>> * fold-const.c (tree_binary_nonzero_warnv_p): Allow
>>> pointer addition to also be considered nonzero.
>>> ---
>>>  gcc/fold-const.c | 20 +---
>>>  1 file changed, 13 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
>>> index 3b9500d..eda713e 100644
>>> --- a/gcc/fold-const.c
>>> +++ b/gcc/fold-const.c
>>> @@ -13200,16 +13200,22 @@ tree_binary_nonzero_warnv_p (enum tree_code code,
>>>  {
>>>  case POINTER_PLUS_EXPR:
>>>  case PLUS_EXPR:
>>> -  if (ANY_INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_UNDEFINED (type))
>>> +  if ((ANY_INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_UNDEFINED (type))
>>> +  || POINTER_TYPE_P (type))
>>>  {
>>> +  /* Pointers are always nonnegative, check integers.  */
>>> +  if (ANY_INTEGRAL_TYPE_P (type))
>>> +  {
>>> +  sub_strict_overflow_p = false;
>>> +  if (!tree_expr_nonnegative_warnv_p (op0,
>>> +  _strict_overflow_p)
>>> +  || !tree_expr_nonnegative_warnv_p (op1,
>>> + _strict_overflow_p))
>>> +return false;
>>> +  }
>>>/* With the presence of negative values it is hard
>>>   to say something.  */
>>> -  sub_strict_overflow_p = false;
>>> -  if (!tree_expr_nonnegative_warnv_p (op0,
>>> -  _strict_overflow_p)
>>> -  || !tree_expr_nonnegative_warnv_p (op1,
>>> - _strict_overflow_p))
>>> -return false;
>>> +
>>
>> Hmm, note that op1 of POINTER_PLUS_EXPR _is_ an integer that needs to be 
>> treated
>> as signed.
>>
>> POINTER_PLUS_EXPR is special in other ways in that iff
>> flag_delete_null_pointer_checks
>> is set we can assume that ptr + non-zero is never zero.  Thus simply do
>>
>>case POINTER_PLUS_EXPR:
>>   return flag_delete_null_pointer_checks
>>&& (tree_expr_nonzero_warnv_p (op0, strict_overflow_p)
>>  || tree_expr_nonzero_warnv_p (op1, strict_overflow_p));
>>
>> OTOH ptr + -(uintptr_t)ptr is valid from a POINTER_PLUS_EXPR
>> perspective as GCC does
>> not have a special tree code for pointer subtraction (but IIRC it uses
>> MINUS_EXPR on integers
>> for this).
>>
>> Richard.
>>
>>
>>>/* One of operands must be positive and the other non-negative.  */
>>>/* We don't set *STRICT_OVERFLOW_P here: even if this value
>>>   overflows, on a twos-complement machine the sum of two
>>> --
>>> 2.8.3

Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Manish Goregaokar]

What about ptr + intptr_t? I guess we should check nonnegative for op1 then?
-Manish


On Thu, Jun 30, 2016 at 5:25 PM, Richard Biener
 wrote:
> On Thu, Jun 30, 2016 at 1:17 PM, Manish Goregaokar  wrote:
>> gcc/ChangeLog:
>> PR c/71699
>> * fold-const.c (tree_binary_nonzero_warnv_p): Allow
>> pointer addition to also be considered nonzero.
>> ---
>>  gcc/fold-const.c | 20 +---
>>  1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
>> index 3b9500d..eda713e 100644
>> --- a/gcc/fold-const.c
>> +++ b/gcc/fold-const.c
>> @@ -13200,16 +13200,22 @@ tree_binary_nonzero_warnv_p (enum tree_code code,
>>  {
>>  case POINTER_PLUS_EXPR:
>>  case PLUS_EXPR:
>> -  if (ANY_INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_UNDEFINED (type))
>> +  if ((ANY_INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_UNDEFINED (type))
>> +  || POINTER_TYPE_P (type))
>>  {
>> +  /* Pointers are always nonnegative, check integers.  */
>> +  if (ANY_INTEGRAL_TYPE_P (type))
>> +  {
>> +  sub_strict_overflow_p = false;
>> +  if (!tree_expr_nonnegative_warnv_p (op0,
>> +  _strict_overflow_p)
>> +  || !tree_expr_nonnegative_warnv_p (op1,
>> + _strict_overflow_p))
>> +return false;
>> +  }
>>/* With the presence of negative values it is hard
>>   to say something.  */
>> -  sub_strict_overflow_p = false;
>> -  if (!tree_expr_nonnegative_warnv_p (op0,
>> -  _strict_overflow_p)
>> -  || !tree_expr_nonnegative_warnv_p (op1,
>> - _strict_overflow_p))
>> -return false;
>> +
>
> Hmm, note that op1 of POINTER_PLUS_EXPR _is_ an integer that needs to be 
> treated
> as signed.
>
> POINTER_PLUS_EXPR is special in other ways in that iff
> flag_delete_null_pointer_checks
> is set we can assume that ptr + non-zero is never zero.  Thus simply do
>
>case POINTER_PLUS_EXPR:
>   return flag_delete_null_pointer_checks
>&& (tree_expr_nonzero_warnv_p (op0, strict_overflow_p)
>  || tree_expr_nonzero_warnv_p (op1, strict_overflow_p));
>
> OTOH ptr + -(uintptr_t)ptr is valid from a POINTER_PLUS_EXPR
> perspective as GCC does
> not have a special tree code for pointer subtraction (but IIRC it uses
> MINUS_EXPR on integers
> for this).
>
> Richard.
>
>
>>/* One of operands must be positive and the other non-negative.  */
>>/* We don't set *STRICT_OVERFLOW_P here: even if this value
>>   overflows, on a twos-complement machine the sum of two
>> --
>> 2.8.3

Re: [PATCH PR c/71699] Handle pointer arithmetic in nonzero tree checks

[Richard Biener]

On Thu, Jun 30, 2016 at 1:17 PM, Manish Goregaokar  wrote:
> gcc/ChangeLog:
> PR c/71699
> * fold-const.c (tree_binary_nonzero_warnv_p): Allow
> pointer addition to also be considered nonzero.
> ---
>  gcc/fold-const.c | 20 +---
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> index 3b9500d..eda713e 100644
> --- a/gcc/fold-const.c
> +++ b/gcc/fold-const.c
> @@ -13200,16 +13200,22 @@ tree_binary_nonzero_warnv_p (enum tree_code code,
>  {
>  case POINTER_PLUS_EXPR:
>  case PLUS_EXPR:
> -  if (ANY_INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_UNDEFINED (type))
> +  if ((ANY_INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_UNDEFINED (type))
> +  || POINTER_TYPE_P (type))
>  {
> +  /* Pointers are always nonnegative, check integers.  */
> +  if (ANY_INTEGRAL_TYPE_P (type))
> +  {
> +  sub_strict_overflow_p = false;
> +  if (!tree_expr_nonnegative_warnv_p (op0,
> +  _strict_overflow_p)
> +  || !tree_expr_nonnegative_warnv_p (op1,
> + _strict_overflow_p))
> +return false;
> +  }
>/* With the presence of negative values it is hard
>   to say something.  */
> -  sub_strict_overflow_p = false;
> -  if (!tree_expr_nonnegative_warnv_p (op0,
> -  _strict_overflow_p)
> -  || !tree_expr_nonnegative_warnv_p (op1,
> - _strict_overflow_p))
> -return false;
> +

Hmm, note that op1 of POINTER_PLUS_EXPR _is_ an integer that needs to be treated
as signed.

POINTER_PLUS_EXPR is special in other ways in that iff
flag_delete_null_pointer_checks
is set we can assume that ptr + non-zero is never zero.  Thus simply do

   case POINTER_PLUS_EXPR:
  return flag_delete_null_pointer_checks
   && (tree_expr_nonzero_warnv_p (op0, strict_overflow_p)
 || tree_expr_nonzero_warnv_p (op1, strict_overflow_p));

OTOH ptr + -(uintptr_t)ptr is valid from a POINTER_PLUS_EXPR
perspective as GCC does
not have a special tree code for pointer subtraction (but IIRC it uses
MINUS_EXPR on integers
for this).

Richard.


>/* One of operands must be positive and the other non-negative.  */
>/* We don't set *STRICT_OVERFLOW_P here: even if this value
>   overflows, on a twos-complement machine the sum of two
> --
> 2.8.3