operator new returns nonzero

operator new returns nonzero

[Marc Glisse]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1047 bytes --]

Hello,

this patch teaches the compiler that operator new, when it can throw, 
isn't allowed to return a null pointer. Note that it doesn't work for 
placement new (that one may have to be handled in the front-end or the 
inliner), and it will not work on user-defined operator new if they are 
inlined. I guess it would be possible to teach the inliner to insert an 
assert_expr or something when inlining such a function, but that's not for 
now. The code I removed from vrp_visit_stmt was duplicated in 
stmt_interesting_for_vrp and thus useless.

I ran the c,c++ testsuite on a non-bootstrap compiler. I'll do more proper 
testing when trunk bootstraps again.

2013-09-07  Marc Glisse  <marc.glisse@inria.fr>

 	PR c++/19476
gcc/
 	* fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
 	* tree-vrp.c (gimple_stmt_nonzero_warnv_p, stmt_interesting_for_vrp):
 	Likewise.
 	(vrp_visit_stmt): Remove duplicated code.

gcc/testsuite/
 	* g++.dg/tree-ssa/pr19476-1.C: New file.
 	* g++.dg/tree-ssa/pr19476-2.C: Likewise.

-- 
Marc Glisse

[-- Attachment #2: Type: TEXT/PLAIN, Size: 6022 bytes --]

Index: fold-const.c
===================================================================
--- fold-const.c	(revision 202351)
+++ fold-const.c	(working copy)
@@ -16171,21 +16171,29 @@ tree_expr_nonzero_warnv_p (tree t, bool
     case MODIFY_EXPR:
     case BIND_EXPR:
       return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 1),
 					strict_overflow_p);
 
     case SAVE_EXPR:
       return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 0),
 					strict_overflow_p);
 
     case CALL_EXPR:
-      return alloca_call_p (t);
+      {
+	tree fn = CALL_EXPR_FN (t);
+	if (TREE_CODE (fn) != ADDR_EXPR) return false;
+	tree fndecl = TREE_OPERAND (fn, 0);
+	if (TREE_CODE (fndecl) != FUNCTION_DECL) return false;
+	if (DECL_IS_OPERATOR_NEW (fndecl) && !TREE_NOTHROW (fndecl))
+	  return true;
+	return alloca_call_p (t);
+      }
 
     default:
       break;
     }
   return false;
 }
 
 /* Return true when T is an address and is known to be nonzero.
    Handle warnings about undefined signed overflow.  */
 
Index: testsuite/g++.dg/tree-ssa/pr19476-1.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-1.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-1.C	(revision 0)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-ccp1" } */
+
+#include <new>
+
+int f(){
+  return 33 + (0 == new(std::nothrow) int);
+}
+int g(){
+  return 42 + (0 == new int[50]);
+}
+
+/* { dg-final { scan-tree-dump     "return 42" "ccp1" } } */
+/* { dg-final { scan-tree-dump-not "return 33" "ccp1" } } */
+/* { dg-final { cleanup-tree-dump "ccp1" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-1.C
___________________________________________________________________
Added: svn:keywords
   + Author Date Id Revision URL
Added: svn:eol-style
   + native

Index: testsuite/g++.dg/tree-ssa/pr19476-2.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-2.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-2.C	(revision 0)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+#include <new>
+
+int f(){
+  int *p = new(std::nothrow) int;
+  return 33 + (0 == p);
+}
+int g(){
+  int *p = new int[50];
+  return 42 + (0 == p);
+}
+
+/* { dg-final { scan-tree-dump     "return 42" "optimized" } } */
+/* { dg-final { scan-tree-dump-not "return 33" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-2.C
___________________________________________________________________
Added: svn:eol-style
   + native
Added: svn:keywords
   + Author Date Id Revision URL

Index: tree-vrp.c
===================================================================
--- tree-vrp.c	(revision 202351)
+++ tree-vrp.c	(working copy)
@@ -1047,21 +1047,27 @@ gimple_assign_nonzero_warnv_p (gimple st
    *STRICT_OVERFLOW_P.*/
 
 static bool
 gimple_stmt_nonzero_warnv_p (gimple stmt, bool *strict_overflow_p)
 {
   switch (gimple_code (stmt))
     {
     case GIMPLE_ASSIGN:
       return gimple_assign_nonzero_warnv_p (stmt, strict_overflow_p);
     case GIMPLE_CALL:
-      return gimple_alloca_call_p (stmt);
+      {
+	tree fndecl = gimple_call_fndecl (stmt);
+	if (!fndecl) return false;
+	if (DECL_IS_OPERATOR_NEW (fndecl) && !TREE_NOTHROW (fndecl))
+	  return true;
+	return gimple_alloca_call_p (stmt);
+      }
     default:
       gcc_unreachable ();
     }
 }
 
 /* Like tree_expr_nonzero_warnv_p, but this function uses value ranges
    obtained so far.  */
 
 static bool
 vrp_stmt_computes_nonzero (gimple stmt, bool *strict_overflow_p)
@@ -6486,21 +6492,22 @@ stmt_interesting_for_vrp (gimple stmt)
       tree lhs = gimple_get_lhs (stmt);
 
       /* In general, assignments with virtual operands are not useful
 	 for deriving ranges, with the obvious exception of calls to
 	 builtin functions.  */
       if (lhs && TREE_CODE (lhs) == SSA_NAME
 	  && (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
 	      || POINTER_TYPE_P (TREE_TYPE (lhs)))
 	  && ((is_gimple_call (stmt)
 	       && gimple_call_fndecl (stmt) != NULL_TREE
-	       && DECL_BUILT_IN (gimple_call_fndecl (stmt)))
+	       && (DECL_BUILT_IN (gimple_call_fndecl (stmt))
+		   || DECL_IS_OPERATOR_NEW (gimple_call_fndecl (stmt))))
 	      || !gimple_vuse (stmt)))
 	return true;
     }
   else if (gimple_code (stmt) == GIMPLE_COND
 	   || gimple_code (stmt) == GIMPLE_SWITCH)
     return true;
 
   return false;
 }
 
@@ -7407,30 +7414,21 @@ vrp_visit_stmt (gimple stmt, edge *taken
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
       fprintf (dump_file, "\nVisiting statement:\n");
       print_gimple_stmt (dump_file, stmt, 0, dump_flags);
       fprintf (dump_file, "\n");
     }
 
   if (!stmt_interesting_for_vrp (stmt))
     gcc_assert (stmt_ends_bb_p (stmt));
   else if (is_gimple_assign (stmt) || is_gimple_call (stmt))
-    {
-      /* In general, assignments with virtual operands are not useful
-	 for deriving ranges, with the obvious exception of calls to
-	 builtin functions.  */
-      if ((is_gimple_call (stmt)
-	   && gimple_call_fndecl (stmt) != NULL_TREE
-	   && DECL_BUILT_IN (gimple_call_fndecl (stmt)))
-	  || !gimple_vuse (stmt))
-	return vrp_visit_assignment_or_call (stmt, output_p);
-    }
+    return vrp_visit_assignment_or_call (stmt, output_p);
   else if (gimple_code (stmt) == GIMPLE_COND)
     return vrp_visit_cond_stmt (stmt, taken_edge_p);
   else if (gimple_code (stmt) == GIMPLE_SWITCH)
     return vrp_visit_switch_stmt (stmt, taken_edge_p);
 
   /* All other statements produce nothing of interest for VRP, so mark
      their outputs varying and prevent further simulation.  */
   FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_DEF)
     set_value_range_to_varying (get_value_range (def));
 

Re: operator new returns nonzero

[Basile Starynkevitch]

On Sat, 2013-09-07 at 12:33 +0200, Marc Glisse wrote:
> Hello,
> 
> this patch teaches the compiler that operator new, when it can throw, 
> isn't allowed to return a null pointer. Note that it doesn't work for 
> placement new (that one may have to be handled in the front-end or the 
> inliner), and it will not work on user-defined operator new if they are 
> inlined. I guess it would be possible to teach the inliner to insert an 
> assert_expr or something when inlining such a function, but that's not for 
> now. The code I removed from vrp_visit_stmt was duplicated in 
> stmt_interesting_for_vrp and thus useless.


Interesting patch. However, I feel that we should give advanced users
the ability to say that their new operator is never returning null...
In particular, if I define my own new operator which never returns nil,
I find strange that it would be less optimized than the system's
operator new.

Perhaps we need an attribute `nonnullresult'  which whould means that.
(we already the nonnull attribute for function arguments)


Cheers.
-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

Re: operator new returns nonzero

[Gabriel Dos Reis]

On Sat, Sep 7, 2013 at 6:44 AM, Basile Starynkevitch
<basile@starynkevitch.net> wrote:
> On Sat, 2013-09-07 at 12:33 +0200, Marc Glisse wrote:
>> Hello,
>>
>> this patch teaches the compiler that operator new, when it can throw,
>> isn't allowed to return a null pointer. Note that it doesn't work for
>> placement new (that one may have to be handled in the front-end or the
>> inliner), and it will not work on user-defined operator new if they are
>> inlined. I guess it would be possible to teach the inliner to insert an
>> assert_expr or something when inlining such a function, but that's not for
>> now. The code I removed from vrp_visit_stmt was duplicated in
>> stmt_interesting_for_vrp and thus useless.
>
>
> Interesting patch. However, I feel that we should give advanced users
> the ability to say that their new operator is never returning null...
> In particular, if I define my own new operator which never returns nil,
> I find strange that it would be less optimized than the system's
> operator new.
>
> Perhaps we need an attribute `nonnullresult'  which whould means that.
> (we already the nonnull attribute for function arguments)

'operator new' is a replaceable function, so when you define it, you have
to abide by the standard semantics.

-- Gaby

Re: operator new returns nonzero

[Marc Glisse]

On Sat, 7 Sep 2013, Basile Starynkevitch wrote:

> On Sat, 2013-09-07 at 12:33 +0200, Marc Glisse wrote:
>> Hello,
>>
>> this patch teaches the compiler that operator new, when it can throw,
>> isn't allowed to return a null pointer. Note that it doesn't work for
>> placement new (that one may have to be handled in the front-end or the
>> inliner), and it will not work on user-defined operator new if they are
>> inlined. I guess it would be possible to teach the inliner to insert an
>> assert_expr or something when inlining such a function, but that's not for
>> now. The code I removed from vrp_visit_stmt was duplicated in
>> stmt_interesting_for_vrp and thus useless.
>
>
> Interesting patch. However, I feel that we should give advanced users
> the ability to say that their new operator is never returning null...

Easy, don't specify noexcept on it and that's what the patch already does, 
as long as the function is not inlined.

> In particular, if I define my own new operator which never returns nil,
> I find strange that it would be less optimized than the system's
> operator new.
>
> Perhaps we need an attribute `nonnullresult'  which whould means that.
> (we already the nonnull attribute for function arguments)

There is already a PR about that, linked from this PR. But even if we 
create this attribute, we will still want to be able to guess that new 
should have it even if it isn't written.

-- 
Marc Glisse

Re: operator new returns nonzero

[Marc Glisse]

On Sat, 7 Sep 2013, Marc Glisse wrote:

> this patch teaches the compiler that operator new, when it can throw, isn't 
> allowed to return a null pointer. Note that it doesn't work for placement new 
> (that one may have to be handled in the front-end or the inliner),

Placement new is a completely different thing. For one, it is nothrow (so 
the test doesn't apply). But mostly, it is a condition on the argument 
more than the result. Using the nonnull attribute would make sense, but 
the compiler doesn't seem clever enough to use that information. The 
easiest seems to be in the library:

--- o/new	2013-09-07 11:11:17.388756320 +0200
+++ i/new	2013-09-07 18:00:32.204797291 +0200
@@ -144,9 +144,9 @@

  // Default placement versions of operator new.
  inline void* operator new(std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
-{ return __p; }
+{ if (!__p) __builtin_unreachable(); return __p; }
  inline void* operator new[](std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
-{ return __p; }
+{ if (!__p) __builtin_unreachable(); return __p; }

  // Default placement versions of operator delete.
  inline void operator delete  (void*, void*) _GLIBCXX_USE_NOEXCEPT { }



Though I am not sure what the policy is for this kind of thing. And that's 
assuming it is indeed undefined to pass a null pointer to it, I don't have 
a good reference for that.


> and it will not work on user-defined operator new if they are inlined.

But then if that operator new is inlined, it may already contain a line 
like if(p==0)throw(); which lets the compiler know all it needs.

> I guess it 
> would be possible to teach the inliner to insert an assert_expr or something 
> when inlining such a function, but that's not for now. The code I removed 
> from vrp_visit_stmt was duplicated in stmt_interesting_for_vrp and thus 
> useless.
>
> I ran the c,c++ testsuite on a non-bootstrap compiler. I'll do more proper 
> testing when trunk bootstraps again.
>
> 2013-09-07  Marc Glisse  <marc.glisse@inria.fr>
>
> 	PR c++/19476
> gcc/
> 	* fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
> 	* tree-vrp.c (gimple_stmt_nonzero_warnv_p, stmt_interesting_for_vrp):
> 	Likewise.
> 	(vrp_visit_stmt): Remove duplicated code.
>
> gcc/testsuite/
> 	* g++.dg/tree-ssa/pr19476-1.C: New file.
> 	* g++.dg/tree-ssa/pr19476-2.C: Likewise.

-- 
Marc Glisse

Re: operator new returns nonzero

[Gabriel Dos Reis]

On Sat, Sep 7, 2013 at 11:42 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Sat, 7 Sep 2013, Marc Glisse wrote:
>
>> this patch teaches the compiler that operator new, when it can throw,
>> isn't allowed to return a null pointer. Note that it doesn't work for
>> placement new (that one may have to be handled in the front-end or the
>> inliner),
>
>
> Placement new is a completely different thing. For one, it is nothrow (so
> the test doesn't apply). But mostly, it is a condition on the argument more
> than the result. Using the nonnull attribute would make sense, but the
> compiler doesn't seem clever enough to use that information. The easiest
> seems to be in the library:
>
> --- o/new       2013-09-07 11:11:17.388756320 +0200
> +++ i/new       2013-09-07 18:00:32.204797291 +0200
> @@ -144,9 +144,9 @@
>
>  // Default placement versions of operator new.
>  inline void* operator new(std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
> -{ return __p; }
> +{ if (!__p) __builtin_unreachable(); return __p; }
>  inline void* operator new[](std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
> -{ return __p; }
> +{ if (!__p) __builtin_unreachable(); return __p; }
>
>  // Default placement versions of operator delete.
>  inline void operator delete  (void*, void*) _GLIBCXX_USE_NOEXCEPT { }
>
>
>
> Though I am not sure what the policy is for this kind of thing. And that's
> assuming it is indeed undefined to pass a null pointer to it, I don't have a
> good reference for that.
>
>
>
>> and it will not work on user-defined operator new if they are inlined.
>
>
> But then if that operator new is inlined, it may already contain a line like
> if(p==0)throw(); which lets the compiler know all it needs.

I am not sure I like this version of the patch.

I think the appropriate patch should be in the compiler, not
here.  C++ has several places where certain parameters cannot
have non-null values. For example, the implicit parameter 'this'
in non-static member functions. This is another instance.

Furthermore, I do think that the compiler should have special nodes
for both standard placement new and the global operator new functions,
as I explained in previous messages.

-- Gaby

>
>
>> I guess it would be possible to teach the inliner to insert an assert_expr
>> or something when inlining such a function, but that's not for now. The code
>> I removed from vrp_visit_stmt was duplicated in stmt_interesting_for_vrp and
>> thus useless.
>>
>> I ran the c,c++ testsuite on a non-bootstrap compiler. I'll do more proper
>> testing when trunk bootstraps again.
>>
>> 2013-09-07  Marc Glisse  <marc.glisse@inria.fr>
>>
>>         PR c++/19476
>> gcc/
>>         * fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
>>         * tree-vrp.c (gimple_stmt_nonzero_warnv_p,
>> stmt_interesting_for_vrp):
>>         Likewise.
>>         (vrp_visit_stmt): Remove duplicated code.
>>
>> gcc/testsuite/
>>         * g++.dg/tree-ssa/pr19476-1.C: New file.
>>         * g++.dg/tree-ssa/pr19476-2.C: Likewise.
>
>
> --
> Marc Glisse

Re: operator new returns nonzero

[Marc Glisse]

On Sat, 7 Sep 2013, Gabriel Dos Reis wrote:

> On Sat, Sep 7, 2013 at 11:42 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> On Sat, 7 Sep 2013, Marc Glisse wrote:
>>
>>> this patch teaches the compiler that operator new, when it can throw,
>>> isn't allowed to return a null pointer. Note that it doesn't work for
>>> placement new (that one may have to be handled in the front-end or the
>>> inliner),
>>
>>
>> Placement new is a completely different thing. For one, it is nothrow (so
>> the test doesn't apply). But mostly, it is a condition on the argument more
>> than the result. Using the nonnull attribute would make sense, but the
>> compiler doesn't seem clever enough to use that information. The easiest
>> seems to be in the library:
>>
>> --- o/new       2013-09-07 11:11:17.388756320 +0200
>> +++ i/new       2013-09-07 18:00:32.204797291 +0200
>> @@ -144,9 +144,9 @@
>>
>>  // Default placement versions of operator new.
>>  inline void* operator new(std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
>> -{ return __p; }
>> +{ if (!__p) __builtin_unreachable(); return __p; }
>>  inline void* operator new[](std::size_t, void* __p) _GLIBCXX_USE_NOEXCEPT
>> -{ return __p; }
>> +{ if (!__p) __builtin_unreachable(); return __p; }
>>
>>  // Default placement versions of operator delete.
>>  inline void operator delete  (void*, void*) _GLIBCXX_USE_NOEXCEPT { }
>>
>>
>>
>> Though I am not sure what the policy is for this kind of thing. And that's
>> assuming it is indeed undefined to pass a null pointer to it, I don't have a
>> good reference for that.
>>
>>
>>
>>> and it will not work on user-defined operator new if they are inlined.
>>
>>
>> But then if that operator new is inlined, it may already contain a line like
>> if(p==0)throw(); which lets the compiler know all it needs.
>
> I am not sure I like this version of the patch.

The 2 patches are really independent, one (in the compiler) for regular 
operator new, and one (in the library) for the placement version.

I mostly like this second patch because it is so easy ;-)
But I will be happy if someone posts a better solution.

> I think the appropriate patch should be in the compiler, not
> here.  C++ has several places where certain parameters cannot
> have non-null values. For example, the implicit parameter 'this'
> in non-static member functions. This is another instance.

Indeed, I didn't check how gcc handles "this" being non-0.

> Furthermore, I do think that the compiler should have special nodes
> for both standard placement new and the global operator new functions,

That's one way to do it. Since this is the first time I look at those, I 
don't really see the advantage compared to the current status, but I 
trust you. What would you do with this special-node placement new? Keep it 
as is until after vrp so we can use the !=0 property and then expand it to 
its first argument? Or expand it early to the equivalent of the library 
code I wrote above?

> as I explained in previous messages.

I couldn't find them, sorry if they contained information that makes this 
post irrelevant.

-- 
Marc Glisse

Re: operator new returns nonzero

[Mike Stump]

On Sep 7, 2013, at 3:33 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> this patch teaches the compiler that operator new, when it can throw, isn't allowed to return a null pointer.

You sure:

@item -fcheck-new
@opindex fcheck-new
Check that the pointer returned by @code{operator new} is non-null
before attempting to modify the storage allocated.  This check is
normally unnecessary because the C++ standard specifies that
@code{operator new} only returns @code{0} if it is declared
@samp{throw()}, in which case the compiler always checks the
return value even without this option.  In all other cases, when
@code{operator new} has a non-empty exception specification, memory
exhaustion is signalled by throwing @code{std::bad_alloc}.  See also
@samp{new (nothrow)}.

?

Re: operator new returns nonzero

[Gabriel Dos Reis]

On Sat, Sep 7, 2013 at 12:59 PM, Marc Glisse <marc.glisse@inria.fr> wrote:

>> Furthermore, I do think that the compiler should have special nodes
>> for both standard placement new and the global operator new functions,
>
>
> That's one way to do it. Since this is the first time I look at those, I
> don't really see the advantage compared to the current status, but I trust
> you. What would you do with this special-node placement new?

placement new really is about "calling" a contractor, and marking the
beginning of the lifetime of a new object, hence aiding lifetime-based
alias analysis.

> Keep it as is
> until after vrp so we can use the !=0 property and then expand it to its
> first argument? Or expand it early to the equivalent of the library code I
> wrote above?
>
>
>> as I explained in previous messages.
>
>
> I couldn't find them, sorry if they contained information that makes this
> post irrelevant.

  http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01276.html
  http://gcc.gnu.org/ml/gcc-patches/2013-08/msg01291.html

-- Gaby

Re: operator new returns nonzero

[Marc Glisse]

On Sat, 7 Sep 2013, Mike Stump wrote:

> On Sep 7, 2013, at 3:33 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> this patch teaches the compiler that operator new, when it can throw, isn't allowed to return a null pointer.
>
> You sure:
>
> @item -fcheck-new
> @opindex fcheck-new
> Check that the pointer returned by @code{operator new} is non-null
> before attempting to modify the storage allocated.  This check is
> normally unnecessary because the C++ standard specifies that
> @code{operator new} only returns @code{0} if it is declared
> @samp{throw()}, in which case the compiler always checks the
> return value even without this option.  In all other cases, when
> @code{operator new} has a non-empty exception specification, memory
> exhaustion is signalled by throwing @code{std::bad_alloc}.  See also
> @samp{new (nothrow)}.
>
> ?

Thanks, I didn't know that option. But it doesn't do the same. -fcheck-new 
is about the front-end inserting a test !=0 between malloc and the 
constructor. My patch is about teaching the middle-end that the value is 
not zero (so even user-coded comparisons with 0 can be simplified).

Now flag_check_new should probably disable this optimization... The 3 
-fcheck-new testcases in the testsuite probably only pass because they 
don't have -O2.

-- 
Marc Glisse

Re: operator new returns nonzero

[Gabriel Dos Reis]

On Sat, Sep 7, 2013 at 2:27 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Sat, 7 Sep 2013, Mike Stump wrote:
>
>> On Sep 7, 2013, at 3:33 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>
>>> this patch teaches the compiler that operator new, when it can throw,
>>> isn't allowed to return a null pointer.
>>
>>
>> You sure:
>>
>> @item -fcheck-new
>> @opindex fcheck-new
>> Check that the pointer returned by @code{operator new} is non-null
>> before attempting to modify the storage allocated.  This check is
>> normally unnecessary because the C++ standard specifies that
>> @code{operator new} only returns @code{0} if it is declared
>> @samp{throw()}, in which case the compiler always checks the
>> return value even without this option.  In all other cases, when
>> @code{operator new} has a non-empty exception specification, memory
>> exhaustion is signalled by throwing @code{std::bad_alloc}.  See also
>> @samp{new (nothrow)}.
>>
>> ?
>
>
> Thanks, I didn't know that option. But it doesn't do the same.

Indeed.

-- Gaby

Re: operator new returns nonzero

[Mike Stump]

On Sep 7, 2013, at 12:37 PM, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:

> On Sat, Sep 7, 2013 at 2:27 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> On Sat, 7 Sep 2013, Mike Stump wrote:
>> 
>>> On Sep 7, 2013, at 3:33 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>> 
>>>> this patch teaches the compiler that operator new, when it can throw,
>>>> isn't allowed to return a null pointer.
>>> 
>>> 
>>> You sure:
>>> 
>>> @item -fcheck-new
>>> @opindex fcheck-new
>>> Check that the pointer returned by @code{operator new} is non-null
>>> before attempting to modify the storage allocated.  This check is
>>> normally unnecessary because the C++ standard specifies that
>>> @code{operator new} only returns @code{0} if it is declared
>>> @samp{throw()}, in which case the compiler always checks the
>>> return value even without this option.  In all other cases, when
>>> @code{operator new} has a non-empty exception specification, memory
>>> exhaustion is signalled by throwing @code{std::bad_alloc}.  See also
>>> @samp{new (nothrow)}.
>>> 
>>> ?
>> 
>> 
>> Thanks, I didn't know that option. But it doesn't do the same.
> 
> Indeed.

Can this throw:

void *operator new (long unsigned int s) {
  return 0;
}

?  Is this allowed to return 0?

Re: operator new returns nonzero

[Mike Stump]

On Sep 7, 2013, at 12:27 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Now flag_check_new should probably disable this optimization…

Yes, this why my point.

Re: operator new returns nonzero

[Marc Glisse]

On Sat, 7 Sep 2013, Mike Stump wrote:

>
> On Sep 7, 2013, at 12:37 PM, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
>
>> On Sat, Sep 7, 2013 at 2:27 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>> On Sat, 7 Sep 2013, Mike Stump wrote:
>>>
>>>> On Sep 7, 2013, at 3:33 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>>>
>>>>> this patch teaches the compiler that operator new, when it can throw,
>>>>> isn't allowed to return a null pointer.
>>>>
>>>>
>>>> You sure:
>>>>
>>>> @item -fcheck-new
>>>> @opindex fcheck-new
>>>> Check that the pointer returned by @code{operator new} is non-null
>>>> before attempting to modify the storage allocated.  This check is
>>>> normally unnecessary because the C++ standard specifies that
>>>> @code{operator new} only returns @code{0} if it is declared
>>>> @samp{throw()}, in which case the compiler always checks the
>>>> return value even without this option.  In all other cases, when
>>>> @code{operator new} has a non-empty exception specification, memory
>>>> exhaustion is signalled by throwing @code{std::bad_alloc}.  See also
>>>> @samp{new (nothrow)}.
>>>>
>>>> ?
>>>
>>>
>>> Thanks, I didn't know that option. But it doesn't do the same.
>>
>> Indeed.
>
> Can this throw:
>
> void *operator new (long unsigned int s) {
>  return 0;
> }
>
> ?  Is this allowed to return 0?

I think using this function is illegal. It isn't marked noexcept, so it 
isn't allowed to return 0.

-- 
Marc Glisse

Re: operator new returns nonzero

[Marc Glisse]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 700 bytes --]

On Sat, 7 Sep 2013, Mike Stump wrote:

> On Sep 7, 2013, at 12:27 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Now flag_check_new should probably disable this optimizationÂ…
>
> Yes, this why my point.

Ok, here it is (again, no proper testing until bootstrap is fixed)

2013-09-07  Marc Glisse  <marc.glisse@inria.fr>

 	PR c++/19476
gcc/
 	* fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
 	* tree-vrp.c (gimple_stmt_nonzero_warnv_p, stmt_interesting_for_vrp):
 	Likewise.
 	(vrp_visit_stmt): Remove duplicated code.

gcc/testsuite/
 	* g++.dg/tree-ssa/pr19476-1.C: New file.
 	* g++.dg/tree-ssa/pr19476-2.C: Likewise.
 	* g++.dg/tree-ssa/pr19476-3.C: Likewise.

-- 
Marc Glisse

[-- Attachment #2: Type: TEXT/PLAIN, Size: 6841 bytes --]

Index: testsuite/g++.dg/tree-ssa/pr19476-1.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-1.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-1.C	(revision 0)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-ccp1" } */
+
+#include <new>
+
+int f(){
+  return 33 + (0 == new(std::nothrow) int);
+}
+int g(){
+  return 42 + (0 == new int[50]);
+}
+
+/* { dg-final { scan-tree-dump     "return 42" "ccp1" } } */
+/* { dg-final { scan-tree-dump-not "return 33" "ccp1" } } */
+/* { dg-final { cleanup-tree-dump "ccp1" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-1.C
___________________________________________________________________
Added: svn:keywords
   + Author Date Id Revision URL
Added: svn:eol-style
   + native

Index: testsuite/g++.dg/tree-ssa/pr19476-2.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-2.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-2.C	(revision 0)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+#include <new>
+
+int f(){
+  int *p = new(std::nothrow) int;
+  return 33 + (0 == p);
+}
+int g(){
+  int *p = new int[50];
+  return 42 + (0 == p);
+}
+
+/* { dg-final { scan-tree-dump     "return 42" "optimized" } } */
+/* { dg-final { scan-tree-dump-not "return 33" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-2.C
___________________________________________________________________
Added: svn:keywords
   + Author Date Id Revision URL
Added: svn:eol-style
   + native

Index: testsuite/g++.dg/tree-ssa/pr19476-3.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-3.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-3.C	(revision 0)
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fcheck-new -fdump-tree-optimized" } */
+
+#include <new>
+
+int g(){
+  return 42 + (0 == new int);
+}
+
+/* { dg-final { scan-tree-dump-not "return 42" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-3.C
___________________________________________________________________
Added: svn:keywords
   + Author Date Id Revision URL
Added: svn:eol-style
   + native

Index: fold-const.c
===================================================================
--- fold-const.c	(revision 202351)
+++ fold-const.c	(working copy)
@@ -16171,21 +16171,31 @@ tree_expr_nonzero_warnv_p (tree t, bool
     case MODIFY_EXPR:
     case BIND_EXPR:
       return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 1),
 					strict_overflow_p);
 
     case SAVE_EXPR:
       return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 0),
 					strict_overflow_p);
 
     case CALL_EXPR:
-      return alloca_call_p (t);
+      {
+	tree fn = CALL_EXPR_FN (t);
+	if (TREE_CODE (fn) != ADDR_EXPR) return false;
+	tree fndecl = TREE_OPERAND (fn, 0);
+	if (TREE_CODE (fndecl) != FUNCTION_DECL) return false;
+	if (!flag_check_new
+	    && DECL_IS_OPERATOR_NEW (fndecl)
+	    && !TREE_NOTHROW (fndecl))
+	  return true;
+	return alloca_call_p (t);
+      }
 
     default:
       break;
     }
   return false;
 }
 
 /* Return true when T is an address and is known to be nonzero.
    Handle warnings about undefined signed overflow.  */
 
Index: tree-vrp.c
===================================================================
--- tree-vrp.c	(revision 202351)
+++ tree-vrp.c	(working copy)
@@ -1047,21 +1047,29 @@ gimple_assign_nonzero_warnv_p (gimple st
    *STRICT_OVERFLOW_P.*/
 
 static bool
 gimple_stmt_nonzero_warnv_p (gimple stmt, bool *strict_overflow_p)
 {
   switch (gimple_code (stmt))
     {
     case GIMPLE_ASSIGN:
       return gimple_assign_nonzero_warnv_p (stmt, strict_overflow_p);
     case GIMPLE_CALL:
-      return gimple_alloca_call_p (stmt);
+      {
+	tree fndecl = gimple_call_fndecl (stmt);
+	if (!fndecl) return false;
+	if (!flag_check_new
+	    && DECL_IS_OPERATOR_NEW (fndecl)
+	    && !TREE_NOTHROW (fndecl))
+	  return true;
+	return gimple_alloca_call_p (stmt);
+      }
     default:
       gcc_unreachable ();
     }
 }
 
 /* Like tree_expr_nonzero_warnv_p, but this function uses value ranges
    obtained so far.  */
 
 static bool
 vrp_stmt_computes_nonzero (gimple stmt, bool *strict_overflow_p)
@@ -6486,21 +6494,22 @@ stmt_interesting_for_vrp (gimple stmt)
       tree lhs = gimple_get_lhs (stmt);
 
       /* In general, assignments with virtual operands are not useful
 	 for deriving ranges, with the obvious exception of calls to
 	 builtin functions.  */
       if (lhs && TREE_CODE (lhs) == SSA_NAME
 	  && (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
 	      || POINTER_TYPE_P (TREE_TYPE (lhs)))
 	  && ((is_gimple_call (stmt)
 	       && gimple_call_fndecl (stmt) != NULL_TREE
-	       && DECL_BUILT_IN (gimple_call_fndecl (stmt)))
+	       && (DECL_BUILT_IN (gimple_call_fndecl (stmt))
+		   || DECL_IS_OPERATOR_NEW (gimple_call_fndecl (stmt))))
 	      || !gimple_vuse (stmt)))
 	return true;
     }
   else if (gimple_code (stmt) == GIMPLE_COND
 	   || gimple_code (stmt) == GIMPLE_SWITCH)
     return true;
 
   return false;
 }
 
@@ -7407,30 +7416,21 @@ vrp_visit_stmt (gimple stmt, edge *taken
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
       fprintf (dump_file, "\nVisiting statement:\n");
       print_gimple_stmt (dump_file, stmt, 0, dump_flags);
       fprintf (dump_file, "\n");
     }
 
   if (!stmt_interesting_for_vrp (stmt))
     gcc_assert (stmt_ends_bb_p (stmt));
   else if (is_gimple_assign (stmt) || is_gimple_call (stmt))
-    {
-      /* In general, assignments with virtual operands are not useful
-	 for deriving ranges, with the obvious exception of calls to
-	 builtin functions.  */
-      if ((is_gimple_call (stmt)
-	   && gimple_call_fndecl (stmt) != NULL_TREE
-	   && DECL_BUILT_IN (gimple_call_fndecl (stmt)))
-	  || !gimple_vuse (stmt))
-	return vrp_visit_assignment_or_call (stmt, output_p);
-    }
+    return vrp_visit_assignment_or_call (stmt, output_p);
   else if (gimple_code (stmt) == GIMPLE_COND)
     return vrp_visit_cond_stmt (stmt, taken_edge_p);
   else if (gimple_code (stmt) == GIMPLE_SWITCH)
     return vrp_visit_switch_stmt (stmt, taken_edge_p);
 
   /* All other statements produce nothing of interest for VRP, so mark
      their outputs varying and prevent further simulation.  */
   FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_DEF)
     set_value_range_to_varying (get_value_range (def));
 

Re: operator new returns nonzero

[Marc Glisse]

On Sat, 7 Sep 2013, Marc Glisse wrote:

> On Sat, 7 Sep 2013, Mike Stump wrote:
>
>> Can this throw:
>> 
>> void *operator new (long unsigned int s) {
>>  return 0;
>> }
>> 
>> ?  Is this allowed to return 0?
>
> I think using this function is illegal. It isn't marked noexcept, so it isn't 
> allowed to return 0.

And if I compile your code with gcc, I get nice warnings (though I get 
them twice and the column number is not so good):

m.cc: In function 'void* operator new(long unsigned int)':
m.cc:2:12: warning: 'operator new' must not return NULL unless it is 
declared 'throw()' (or -fcheck-new is in effect) [enabled by default]
      return 0;
             ^
m.cc: At global scope:
m.cc:1:7: warning: unused parameter 's' [-Wunused-parameter]
  void *operator new (long unsigned int s) {
        ^


-- 
Marc Glisse

Re: operator new returns nonzero

[Gabriel Dos Reis]

On Sat, Sep 7, 2013 at 2:46 PM, Mike Stump <mikestump@comcast.net> wrote:
>
> On Sep 7, 2013, at 12:37 PM, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
>
>> On Sat, Sep 7, 2013 at 2:27 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>> On Sat, 7 Sep 2013, Mike Stump wrote:
>>>
>>>> On Sep 7, 2013, at 3:33 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>>>
>>>>> this patch teaches the compiler that operator new, when it can throw,
>>>>> isn't allowed to return a null pointer.
>>>>
>>>>
>>>> You sure:
>>>>
>>>> @item -fcheck-new
>>>> @opindex fcheck-new
>>>> Check that the pointer returned by @code{operator new} is non-null
>>>> before attempting to modify the storage allocated.  This check is
>>>> normally unnecessary because the C++ standard specifies that
>>>> @code{operator new} only returns @code{0} if it is declared
>>>> @samp{throw()}, in which case the compiler always checks the
>>>> return value even without this option.  In all other cases, when
>>>> @code{operator new} has a non-empty exception specification, memory
>>>> exhaustion is signalled by throwing @code{std::bad_alloc}.  See also
>>>> @samp{new (nothrow)}.
>>>>
>>>> ?
>>>
>>>
>>> Thanks, I didn't know that option. But it doesn't do the same.
>>
>> Indeed.
>
> Can this throw:
>
> void *operator new (long unsigned int s) {
>   return 0;
> }
>
> ?  Is this allowed to return 0?

If that is supposed to be a definition for the global function 'operator new',
then it fails the requirement of the C++ standards since 1998.

-- Gaby

Re: operator new returns nonzero

[Richard Biener]

On Sat, Sep 7, 2013 at 11:00 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Sat, 7 Sep 2013, Mike Stump wrote:
>
>> On Sep 7, 2013, at 12:27 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>
>>> Now flag_check_new should probably disable this optimization…
>>
>>
>> Yes, this why my point.
>
>
> Ok, here it is (again, no proper testing until bootstrap is fixed)

I wonder what happens on targets where 0 is a valid address of an object
(stated by !flag_delete_null_pointer_checks)?

Richard.

>
> 2013-09-07  Marc Glisse  <marc.glisse@inria.fr>
>
>         PR c++/19476
> gcc/
>         * fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
>         * tree-vrp.c (gimple_stmt_nonzero_warnv_p,
> stmt_interesting_for_vrp):
>         Likewise.
>         (vrp_visit_stmt): Remove duplicated code.
>
> gcc/testsuite/
>         * g++.dg/tree-ssa/pr19476-1.C: New file.
>         * g++.dg/tree-ssa/pr19476-2.C: Likewise.
>         * g++.dg/tree-ssa/pr19476-3.C: Likewise.
>
> --
> Marc Glisse
> Index: testsuite/g++.dg/tree-ssa/pr19476-1.C
> ===================================================================
> --- testsuite/g++.dg/tree-ssa/pr19476-1.C       (revision 0)
> +++ testsuite/g++.dg/tree-ssa/pr19476-1.C       (revision 0)
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O -fdump-tree-ccp1" } */
> +
> +#include <new>
> +
> +int f(){
> +  return 33 + (0 == new(std::nothrow) int);
> +}
> +int g(){
> +  return 42 + (0 == new int[50]);
> +}
> +
> +/* { dg-final { scan-tree-dump     "return 42" "ccp1" } } */
> +/* { dg-final { scan-tree-dump-not "return 33" "ccp1" } } */
> +/* { dg-final { cleanup-tree-dump "ccp1" } } */
>
> Property changes on: testsuite/g++.dg/tree-ssa/pr19476-1.C
> ___________________________________________________________________
> Added: svn:keywords
>    + Author Date Id Revision URL
> Added: svn:eol-style
>    + native
>
> Index: testsuite/g++.dg/tree-ssa/pr19476-2.C
> ===================================================================
> --- testsuite/g++.dg/tree-ssa/pr19476-2.C       (revision 0)
> +++ testsuite/g++.dg/tree-ssa/pr19476-2.C       (revision 0)
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +
> +#include <new>
> +
> +int f(){
> +  int *p = new(std::nothrow) int;
> +  return 33 + (0 == p);
> +}
> +int g(){
> +  int *p = new int[50];
> +  return 42 + (0 == p);
> +}
> +
> +/* { dg-final { scan-tree-dump     "return 42" "optimized" } } */
> +/* { dg-final { scan-tree-dump-not "return 33" "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
>
> Property changes on: testsuite/g++.dg/tree-ssa/pr19476-2.C
> ___________________________________________________________________
> Added: svn:keywords
>    + Author Date Id Revision URL
> Added: svn:eol-style
>    + native
>
> Index: testsuite/g++.dg/tree-ssa/pr19476-3.C
> ===================================================================
> --- testsuite/g++.dg/tree-ssa/pr19476-3.C       (revision 0)
> +++ testsuite/g++.dg/tree-ssa/pr19476-3.C       (revision 0)
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -fcheck-new -fdump-tree-optimized" } */
> +
> +#include <new>
> +
> +int g(){
> +  return 42 + (0 == new int);
> +}
> +
> +/* { dg-final { scan-tree-dump-not "return 42" "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
>
> Property changes on: testsuite/g++.dg/tree-ssa/pr19476-3.C
> ___________________________________________________________________
> Added: svn:keywords
>    + Author Date Id Revision URL
> Added: svn:eol-style
>    + native
>
> Index: fold-const.c
> ===================================================================
> --- fold-const.c        (revision 202351)
> +++ fold-const.c        (working copy)
> @@ -16171,21 +16171,31 @@ tree_expr_nonzero_warnv_p (tree t, bool
>      case MODIFY_EXPR:
>      case BIND_EXPR:
>        return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 1),
>                                         strict_overflow_p);
>
>      case SAVE_EXPR:
>        return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 0),
>                                         strict_overflow_p);
>
>      case CALL_EXPR:
> -      return alloca_call_p (t);
> +      {
> +       tree fn = CALL_EXPR_FN (t);
> +       if (TREE_CODE (fn) != ADDR_EXPR) return false;
> +       tree fndecl = TREE_OPERAND (fn, 0);
> +       if (TREE_CODE (fndecl) != FUNCTION_DECL) return false;
> +       if (!flag_check_new
> +           && DECL_IS_OPERATOR_NEW (fndecl)
> +           && !TREE_NOTHROW (fndecl))
> +         return true;
> +       return alloca_call_p (t);
> +      }
>
>      default:
>        break;
>      }
>    return false;
>  }
>
>  /* Return true when T is an address and is known to be nonzero.
>     Handle warnings about undefined signed overflow.  */
>
> Index: tree-vrp.c
> ===================================================================
> --- tree-vrp.c  (revision 202351)
> +++ tree-vrp.c  (working copy)
> @@ -1047,21 +1047,29 @@ gimple_assign_nonzero_warnv_p (gimple st
>     *STRICT_OVERFLOW_P.*/
>
>  static bool
>  gimple_stmt_nonzero_warnv_p (gimple stmt, bool *strict_overflow_p)
>  {
>    switch (gimple_code (stmt))
>      {
>      case GIMPLE_ASSIGN:
>        return gimple_assign_nonzero_warnv_p (stmt, strict_overflow_p);
>      case GIMPLE_CALL:
> -      return gimple_alloca_call_p (stmt);
> +      {
> +       tree fndecl = gimple_call_fndecl (stmt);
> +       if (!fndecl) return false;
> +       if (!flag_check_new
> +           && DECL_IS_OPERATOR_NEW (fndecl)
> +           && !TREE_NOTHROW (fndecl))
> +         return true;
> +       return gimple_alloca_call_p (stmt);
> +      }
>      default:
>        gcc_unreachable ();
>      }
>  }
>
>  /* Like tree_expr_nonzero_warnv_p, but this function uses value ranges
>     obtained so far.  */
>
>  static bool
>  vrp_stmt_computes_nonzero (gimple stmt, bool *strict_overflow_p)
> @@ -6486,21 +6494,22 @@ stmt_interesting_for_vrp (gimple stmt)
>        tree lhs = gimple_get_lhs (stmt);
>
>        /* In general, assignments with virtual operands are not useful
>          for deriving ranges, with the obvious exception of calls to
>          builtin functions.  */
>        if (lhs && TREE_CODE (lhs) == SSA_NAME
>           && (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
>               || POINTER_TYPE_P (TREE_TYPE (lhs)))
>           && ((is_gimple_call (stmt)
>                && gimple_call_fndecl (stmt) != NULL_TREE
> -              && DECL_BUILT_IN (gimple_call_fndecl (stmt)))
> +              && (DECL_BUILT_IN (gimple_call_fndecl (stmt))
> +                  || DECL_IS_OPERATOR_NEW (gimple_call_fndecl (stmt))))
>               || !gimple_vuse (stmt)))
>         return true;
>      }
>    else if (gimple_code (stmt) == GIMPLE_COND
>            || gimple_code (stmt) == GIMPLE_SWITCH)
>      return true;
>
>    return false;
>  }
>
> @@ -7407,30 +7416,21 @@ vrp_visit_stmt (gimple stmt, edge *taken
>    if (dump_file && (dump_flags & TDF_DETAILS))
>      {
>        fprintf (dump_file, "\nVisiting statement:\n");
>        print_gimple_stmt (dump_file, stmt, 0, dump_flags);
>        fprintf (dump_file, "\n");
>      }
>
>    if (!stmt_interesting_for_vrp (stmt))
>      gcc_assert (stmt_ends_bb_p (stmt));
>    else if (is_gimple_assign (stmt) || is_gimple_call (stmt))
> -    {
> -      /* In general, assignments with virtual operands are not useful
> -        for deriving ranges, with the obvious exception of calls to
> -        builtin functions.  */
> -      if ((is_gimple_call (stmt)
> -          && gimple_call_fndecl (stmt) != NULL_TREE
> -          && DECL_BUILT_IN (gimple_call_fndecl (stmt)))
> -         || !gimple_vuse (stmt))
> -       return vrp_visit_assignment_or_call (stmt, output_p);
> -    }
> +    return vrp_visit_assignment_or_call (stmt, output_p);
>    else if (gimple_code (stmt) == GIMPLE_COND)
>      return vrp_visit_cond_stmt (stmt, taken_edge_p);
>    else if (gimple_code (stmt) == GIMPLE_SWITCH)
>      return vrp_visit_switch_stmt (stmt, taken_edge_p);
>
>    /* All other statements produce nothing of interest for VRP, so mark
>       their outputs varying and prevent further simulation.  */
>    FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_DEF)
>      set_value_range_to_varying (get_value_range (def));
>
>

Re: operator new returns nonzero

[Marc Glisse]

On Mon, 9 Sep 2013, Richard Biener wrote:

> On Sat, Sep 7, 2013 at 11:00 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> On Sat, 7 Sep 2013, Mike Stump wrote:
>>
>>> On Sep 7, 2013, at 12:27 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>>
>>>> Now flag_check_new should probably disable this optimizationÂ…
>>>
>>>
>>> Yes, this why my point.
>>
>>
>> Ok, here it is (again, no proper testing until bootstrap is fixed)
>
> I wonder what happens on targets where 0 is a valid address of an object
> (stated by !flag_delete_null_pointer_checks)?

I am not at all familiar with those targets (I thought you had to write
asm to access 0 so the compiler doesn't mess with your code), but it
makes sense to me to test (flag_delete_null_pointer_checks &&
!flag_check_new) instead of just !flag_check_new. Consider the patch
changed this way. (we have so many options, I wouldn't be surprised if
there is yet another one to check...)

-- 
Marc Glisse

Re: operator new returns nonzero

[Gabriel Dos Reis]

On Mon, Sep 9, 2013 at 4:19 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Mon, 9 Sep 2013, Richard Biener wrote:
>
>> On Sat, Sep 7, 2013 at 11:00 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>
>>> On Sat, 7 Sep 2013, Mike Stump wrote:
>>>
>>>> On Sep 7, 2013, at 12:27 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>>>
>>>>>
>>>>> Now flag_check_new should probably disable this optimization…
>>>>
>>>>
>>>>
>>>> Yes, this why my point.
>>>
>>>
>>>
>>> Ok, here it is (again, no proper testing until bootstrap is fixed)
>>
>>
>> I wonder what happens on targets where 0 is a valid address of an object
>> (stated by !flag_delete_null_pointer_checks)?
>
>
> I am not at all familiar with those targets (I thought you had to write
> asm to access 0 so the compiler doesn't mess with your code), but it
> makes sense to me to test (flag_delete_null_pointer_checks &&
> !flag_check_new) instead of just !flag_check_new. Consider the patch
> changed this way. (we have so many options, I wouldn't be surprised if
> there is yet another one to check…)

If we have such a target (do we?) where 0 is a valid address of an object,
I would not be surprised if it breaks in so many other ways, since the
language explicitly states that can never happen (programmers write
code depending on that).


>
> --
> Marc Glisse

Re: operator new returns nonzero

[Gabriel Dos Reis]

On Mon, Sep 9, 2013 at 4:06 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Sat, Sep 7, 2013 at 11:00 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> On Sat, 7 Sep 2013, Mike Stump wrote:
>>
>>> On Sep 7, 2013, at 12:27 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>>
>>>> Now flag_check_new should probably disable this optimization…
>>>
>>>
>>> Yes, this why my point.
>>
>>
>> Ok, here it is (again, no proper testing until bootstrap is fixed)
>
> I wonder what happens on targets where 0 is a valid address of an object
> (stated by !flag_delete_null_pointer_checks)?

We should distinguish between front-end notion of null pointer,
and backend notion of address zero.  The language, and therefore
the front-end, does not allow an object to have 'this' value to be
a null pointer, nor does is allow 'operator new' to return null pointer.

Consequently, if we have a target (which ones?) where zero is
a valid address for an object, that target should take precaution
to satisfy the requirement of the language.

-- Gaby

>
> Richard.
>
>>
>> 2013-09-07  Marc Glisse  <marc.glisse@inria.fr>
>>
>>         PR c++/19476
>> gcc/
>>         * fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
>>         * tree-vrp.c (gimple_stmt_nonzero_warnv_p,
>> stmt_interesting_for_vrp):
>>         Likewise.
>>         (vrp_visit_stmt): Remove duplicated code.
>>
>> gcc/testsuite/
>>         * g++.dg/tree-ssa/pr19476-1.C: New file.
>>         * g++.dg/tree-ssa/pr19476-2.C: Likewise.
>>         * g++.dg/tree-ssa/pr19476-3.C: Likewise.
>>
>> --
>> Marc Glisse
>> Index: testsuite/g++.dg/tree-ssa/pr19476-1.C
>> ===================================================================
>> --- testsuite/g++.dg/tree-ssa/pr19476-1.C       (revision 0)
>> +++ testsuite/g++.dg/tree-ssa/pr19476-1.C       (revision 0)
>> @@ -0,0 +1,15 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O -fdump-tree-ccp1" } */
>> +
>> +#include <new>
>> +
>> +int f(){
>> +  return 33 + (0 == new(std::nothrow) int);
>> +}
>> +int g(){
>> +  return 42 + (0 == new int[50]);
>> +}
>> +
>> +/* { dg-final { scan-tree-dump     "return 42" "ccp1" } } */
>> +/* { dg-final { scan-tree-dump-not "return 33" "ccp1" } } */
>> +/* { dg-final { cleanup-tree-dump "ccp1" } } */
>>
>> Property changes on: testsuite/g++.dg/tree-ssa/pr19476-1.C
>> ___________________________________________________________________
>> Added: svn:keywords
>>    + Author Date Id Revision URL
>> Added: svn:eol-style
>>    + native
>>
>> Index: testsuite/g++.dg/tree-ssa/pr19476-2.C
>> ===================================================================
>> --- testsuite/g++.dg/tree-ssa/pr19476-2.C       (revision 0)
>> +++ testsuite/g++.dg/tree-ssa/pr19476-2.C       (revision 0)
>> @@ -0,0 +1,17 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -fdump-tree-optimized" } */
>> +
>> +#include <new>
>> +
>> +int f(){
>> +  int *p = new(std::nothrow) int;
>> +  return 33 + (0 == p);
>> +}
>> +int g(){
>> +  int *p = new int[50];
>> +  return 42 + (0 == p);
>> +}
>> +
>> +/* { dg-final { scan-tree-dump     "return 42" "optimized" } } */
>> +/* { dg-final { scan-tree-dump-not "return 33" "optimized" } } */
>> +/* { dg-final { cleanup-tree-dump "optimized" } } */
>>
>> Property changes on: testsuite/g++.dg/tree-ssa/pr19476-2.C
>> ___________________________________________________________________
>> Added: svn:keywords
>>    + Author Date Id Revision URL
>> Added: svn:eol-style
>>    + native
>>
>> Index: testsuite/g++.dg/tree-ssa/pr19476-3.C
>> ===================================================================
>> --- testsuite/g++.dg/tree-ssa/pr19476-3.C       (revision 0)
>> +++ testsuite/g++.dg/tree-ssa/pr19476-3.C       (revision 0)
>> @@ -0,0 +1,11 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O3 -fcheck-new -fdump-tree-optimized" } */
>> +
>> +#include <new>
>> +
>> +int g(){
>> +  return 42 + (0 == new int);
>> +}
>> +
>> +/* { dg-final { scan-tree-dump-not "return 42" "optimized" } } */
>> +/* { dg-final { cleanup-tree-dump "optimized" } } */
>>
>> Property changes on: testsuite/g++.dg/tree-ssa/pr19476-3.C
>> ___________________________________________________________________
>> Added: svn:keywords
>>    + Author Date Id Revision URL
>> Added: svn:eol-style
>>    + native
>>
>> Index: fold-const.c
>> ===================================================================
>> --- fold-const.c        (revision 202351)
>> +++ fold-const.c        (working copy)
>> @@ -16171,21 +16171,31 @@ tree_expr_nonzero_warnv_p (tree t, bool
>>      case MODIFY_EXPR:
>>      case BIND_EXPR:
>>        return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 1),
>>                                         strict_overflow_p);
>>
>>      case SAVE_EXPR:
>>        return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 0),
>>                                         strict_overflow_p);
>>
>>      case CALL_EXPR:
>> -      return alloca_call_p (t);
>> +      {
>> +       tree fn = CALL_EXPR_FN (t);
>> +       if (TREE_CODE (fn) != ADDR_EXPR) return false;
>> +       tree fndecl = TREE_OPERAND (fn, 0);
>> +       if (TREE_CODE (fndecl) != FUNCTION_DECL) return false;
>> +       if (!flag_check_new
>> +           && DECL_IS_OPERATOR_NEW (fndecl)
>> +           && !TREE_NOTHROW (fndecl))
>> +         return true;
>> +       return alloca_call_p (t);
>> +      }
>>
>>      default:
>>        break;
>>      }
>>    return false;
>>  }
>>
>>  /* Return true when T is an address and is known to be nonzero.
>>     Handle warnings about undefined signed overflow.  */
>>
>> Index: tree-vrp.c
>> ===================================================================
>> --- tree-vrp.c  (revision 202351)
>> +++ tree-vrp.c  (working copy)
>> @@ -1047,21 +1047,29 @@ gimple_assign_nonzero_warnv_p (gimple st
>>     *STRICT_OVERFLOW_P.*/
>>
>>  static bool
>>  gimple_stmt_nonzero_warnv_p (gimple stmt, bool *strict_overflow_p)
>>  {
>>    switch (gimple_code (stmt))
>>      {
>>      case GIMPLE_ASSIGN:
>>        return gimple_assign_nonzero_warnv_p (stmt, strict_overflow_p);
>>      case GIMPLE_CALL:
>> -      return gimple_alloca_call_p (stmt);
>> +      {
>> +       tree fndecl = gimple_call_fndecl (stmt);
>> +       if (!fndecl) return false;
>> +       if (!flag_check_new
>> +           && DECL_IS_OPERATOR_NEW (fndecl)
>> +           && !TREE_NOTHROW (fndecl))
>> +         return true;
>> +       return gimple_alloca_call_p (stmt);
>> +      }
>>      default:
>>        gcc_unreachable ();
>>      }
>>  }
>>
>>  /* Like tree_expr_nonzero_warnv_p, but this function uses value ranges
>>     obtained so far.  */
>>
>>  static bool
>>  vrp_stmt_computes_nonzero (gimple stmt, bool *strict_overflow_p)
>> @@ -6486,21 +6494,22 @@ stmt_interesting_for_vrp (gimple stmt)
>>        tree lhs = gimple_get_lhs (stmt);
>>
>>        /* In general, assignments with virtual operands are not useful
>>          for deriving ranges, with the obvious exception of calls to
>>          builtin functions.  */
>>        if (lhs && TREE_CODE (lhs) == SSA_NAME
>>           && (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
>>               || POINTER_TYPE_P (TREE_TYPE (lhs)))
>>           && ((is_gimple_call (stmt)
>>                && gimple_call_fndecl (stmt) != NULL_TREE
>> -              && DECL_BUILT_IN (gimple_call_fndecl (stmt)))
>> +              && (DECL_BUILT_IN (gimple_call_fndecl (stmt))
>> +                  || DECL_IS_OPERATOR_NEW (gimple_call_fndecl (stmt))))
>>               || !gimple_vuse (stmt)))
>>         return true;
>>      }
>>    else if (gimple_code (stmt) == GIMPLE_COND
>>            || gimple_code (stmt) == GIMPLE_SWITCH)
>>      return true;
>>
>>    return false;
>>  }
>>
>> @@ -7407,30 +7416,21 @@ vrp_visit_stmt (gimple stmt, edge *taken
>>    if (dump_file && (dump_flags & TDF_DETAILS))
>>      {
>>        fprintf (dump_file, "\nVisiting statement:\n");
>>        print_gimple_stmt (dump_file, stmt, 0, dump_flags);
>>        fprintf (dump_file, "\n");
>>      }
>>
>>    if (!stmt_interesting_for_vrp (stmt))
>>      gcc_assert (stmt_ends_bb_p (stmt));
>>    else if (is_gimple_assign (stmt) || is_gimple_call (stmt))
>> -    {
>> -      /* In general, assignments with virtual operands are not useful
>> -        for deriving ranges, with the obvious exception of calls to
>> -        builtin functions.  */
>> -      if ((is_gimple_call (stmt)
>> -          && gimple_call_fndecl (stmt) != NULL_TREE
>> -          && DECL_BUILT_IN (gimple_call_fndecl (stmt)))
>> -         || !gimple_vuse (stmt))
>> -       return vrp_visit_assignment_or_call (stmt, output_p);
>> -    }
>> +    return vrp_visit_assignment_or_call (stmt, output_p);
>>    else if (gimple_code (stmt) == GIMPLE_COND)
>>      return vrp_visit_cond_stmt (stmt, taken_edge_p);
>>    else if (gimple_code (stmt) == GIMPLE_SWITCH)
>>      return vrp_visit_switch_stmt (stmt, taken_edge_p);
>>
>>    /* All other statements produce nothing of interest for VRP, so mark
>>       their outputs varying and prevent further simulation.  */
>>    FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_DEF)
>>      set_value_range_to_varying (get_value_range (def));
>>
>>

Re: operator new returns nonzero

[Mike Stump]

On Sep 7, 2013, at 5:34 PM, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
> On Sat, Sep 7, 2013 at 2:46 PM, Mike Stump <mikestump@comcast.net> wrote:
>> 
>> On Sep 7, 2013, at 12:37 PM, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
>> 
>>> On Sat, Sep 7, 2013 at 2:27 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>> On Sat, 7 Sep 2013, Mike Stump wrote:
>>>> 
>>>>> On Sep 7, 2013, at 3:33 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>>>> 
>>>>>> this patch teaches the compiler that operator new, when it can throw,
>>>>>> isn't allowed to return a null pointer.
>>>>> 
>>>>> 
>>>>> You sure:
>>>>> 
>>>>> @item -fcheck-new
>>>>> @opindex fcheck-new
>>>>> Check that the pointer returned by @code{operator new} is non-null
>>>>> before attempting to modify the storage allocated.  This check is
>>>>> normally unnecessary because the C++ standard specifies that
>>>>> @code{operator new} only returns @code{0} if it is declared
>>>>> @samp{throw()}, in which case the compiler always checks the
>>>>> return value even without this option.  In all other cases, when
>>>>> @code{operator new} has a non-empty exception specification, memory
>>>>> exhaustion is signalled by throwing @code{std::bad_alloc}.  See also
>>>>> @samp{new (nothrow)}.
>>>>> 
>>>>> ?
>>>> 
>>>> 
>>>> Thanks, I didn't know that option. But it doesn't do the same.
>>> 
>>> Indeed.
>> 
>> Can this throw:
>> 
>> void *operator new (long unsigned int s) {
>>  return 0;
>> }
>> 
>> ?  Is this allowed to return 0?
> 
> If that is supposed to be a definition for the global function 'operator new',
> then it fails the requirement of the C++ standards since 1998.

And that is irrelevant to the point at hand.  I'll note that you failed to answer both questions.  The answer is, the declaration of the function is not no throw, so by declaration, that operator new can throw (though, this definition will never throw).  And yes, it is allowed to return 0 because the language selected by the option informs the compiler of that.  Surely since I pointed out the existence of the option, you know what it is for, right?  Were you able to read and understand the option or not?

Re: operator new returns nonzero

[Mike Stump]

On Sep 9, 2013, at 5:41 AM, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
> On Mon, Sep 9, 2013 at 4:06 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Sat, Sep 7, 2013 at 11:00 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>> On Sat, 7 Sep 2013, Mike Stump wrote:
>>> 
>>>> On Sep 7, 2013, at 12:27 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>>> 
>>>>> Now flag_check_new should probably disable this optimization…
>>>> 
>>>> 
>>>> Yes, this why my point.
>>> 
>>> 
>>> Ok, here it is (again, no proper testing until bootstrap is fixed)
>> 
>> I wonder what happens on targets where 0 is a valid address of an object
>> (stated by !flag_delete_null_pointer_checks)?
> 
> We should distinguish between front-end notion of null pointer,
> and backend notion of address zero.  The language, and therefore
> the front-end, does not allow an object to have 'this' value to be
> a null pointer, nor does is allow 'operator new' to return null pointer.

You've failed to understand g++.  Please seek to understand the compiler, before you weigh in.

Re: operator new returns nonzero

[Mike Stump]

On Sep 7, 2013, at 1:03 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Sat, 7 Sep 2013, Mike Stump wrote:
> 
>> 
>> On Sep 7, 2013, at 12:37 PM, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
>> 
>>> On Sat, Sep 7, 2013 at 2:27 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>> On Sat, 7 Sep 2013, Mike Stump wrote:
>>>> 
>>>>> On Sep 7, 2013, at 3:33 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>>>> 
>>>>>> this patch teaches the compiler that operator new, when it can throw,
>>>>>> isn't allowed to return a null pointer.
>>>>> 
>>>>> 
>>>>> You sure:
>>>>> 
>>>>> @item -fcheck-new
>>>>> @opindex fcheck-new
>>>>> Check that the pointer returned by @code{operator new} is non-null
>>>>> before attempting to modify the storage allocated.  This check is
>>>>> normally unnecessary because the C++ standard specifies that
>>>>> @code{operator new} only returns @code{0} if it is declared
>>>>> @samp{throw()}, in which case the compiler always checks the
>>>>> return value even without this option.  In all other cases, when
>>>>> @code{operator new} has a non-empty exception specification, memory
>>>>> exhaustion is signalled by throwing @code{std::bad_alloc}.  See also
>>>>> @samp{new (nothrow)}.
>>>>> 
>>>>> ?
>>>> 
>>>> 
>>>> Thanks, I didn't know that option. But it doesn't do the same.
>>> 
>>> Indeed.
>> 
>> Can this throw:
>> 
>> void *operator new (long unsigned int s) {
>> return 0;
>> }
>> 
>> ?  Is this allowed to return 0?
> 
> I think using this function is illegal.

Yes, according the the standard, and without any compilation flags, it is.

However, with -fcheck-new and in the g++ language, it is not.

Re: operator new returns nonzero

[Mike Stump]

On Sep 7, 2013, at 2:00 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> @@ -16171,21 +16171,31 @@ tree_expr_nonzero_warnv_p (tree t, bool

> +	if (TREE_CODE (fndecl) != FUNCTION_DECL) return false;
> +	if (!flag_check_new
> +	    && DECL_IS_OPERATOR_NEW (fndecl)
> +	    && !TREE_NOTHROW (fndecl))
> +	  return true;
> +	return alloca_call_p (t);

I think this is correct, thanks.

Re: operator new returns nonzero

[Gabriel Dos Reis]

On Mon, Sep 9, 2013 at 11:16 AM, Mike Stump <mikestump@comcast.net> wrote:
> You've failed to understand g++.  Please seek to understand the compiler, before you weigh in.

Gee Mike, see a medical assistance for your and our benefits.

-- Gaby

Re: operator new returns nonzero

[Mike Stump]

On Sep 9, 2013, at 10:25 AM, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
> On Mon, Sep 9, 2013 at 11:16 AM, Mike Stump <mikestump@comcast.net> wrote:
>> You've failed to understand g++.  Please seek to understand the compiler, before you weigh in.
> 
> Gee Mike, see a medical assistance for your and our benefits.

Seems overly hostile to me.  Why?

I accomplished my goal.  The author of the patch understands the issue and put in the necessary code.

Re: operator new returns nonzero

[Gabriel Dos Reis]

On Mon, Sep 9, 2013 at 12:54 PM, Mike Stump <mikestump@comcast.net> wrote:
> On Sep 9, 2013, at 10:25 AM, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
>> On Mon, Sep 9, 2013 at 11:16 AM, Mike Stump <mikestump@comcast.net> wrote:
>>> You've failed to understand g++.  Please seek to understand the compiler, before you weigh in.
>>
>> Gee Mike, see a medical assistance for your and our benefits.
>
> Seems overly hostile to me.

Funny you would say that.

-- Gaby

Re: operator new returns nonzero

[Marc Glisse]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 536 bytes --]

I have now tested bootstrap+testsuite and there was no regression.

2013-09-07  Marc Glisse  <marc.glisse@inria.fr>

 	PR c++/19476
gcc/
 	* fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
 	* tree-vrp.c (gimple_stmt_nonzero_warnv_p, stmt_interesting_for_vrp):
 	Likewise.
 	(vrp_visit_stmt): Remove duplicated code.

gcc/testsuite/
 	* g++.dg/tree-ssa/pr19476-1.C: New file.
 	* g++.dg/tree-ssa/pr19476-2.C: Likewise.
 	* g++.dg/tree-ssa/pr19476-3.C: Likewise.
 	* g++.dg/tree-ssa/pr19476-4.C: Likewise.

-- 
Marc Glisse

[-- Attachment #2: Type: TEXT/PLAIN, Size: 8036 bytes --]

Index: fold-const.c
===================================================================
--- fold-const.c	(revision 202413)
+++ fold-const.c	(working copy)
@@ -16171,21 +16171,31 @@ tree_expr_nonzero_warnv_p (tree t, bool
     case MODIFY_EXPR:
     case BIND_EXPR:
       return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 1),
 					strict_overflow_p);
 
     case SAVE_EXPR:
       return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 0),
 					strict_overflow_p);
 
     case CALL_EXPR:
-      return alloca_call_p (t);
+      {
+	tree fn = CALL_EXPR_FN (t);
+	if (TREE_CODE (fn) != ADDR_EXPR) return false;
+	tree fndecl = TREE_OPERAND (fn, 0);
+	if (TREE_CODE (fndecl) != FUNCTION_DECL) return false;
+	if (flag_delete_null_pointer_checks && !flag_check_new
+	    && DECL_IS_OPERATOR_NEW (fndecl)
+	    && !TREE_NOTHROW (fndecl))
+	  return true;
+	return alloca_call_p (t);
+      }
 
     default:
       break;
     }
   return false;
 }
 
 /* Return true when T is an address and is known to be nonzero.
    Handle warnings about undefined signed overflow.  */
 
Index: testsuite/g++.dg/tree-ssa/pr19476-1.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-1.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-1.C	(working copy)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-ccp1" } */
+
+#include <new>
+
+int f(){
+  return 33 + (0 == new(std::nothrow) int);
+}
+int g(){
+  return 42 + (0 == new int[50]);
+}
+
+/* { dg-final { scan-tree-dump     "return 42" "ccp1" } } */
+/* { dg-final { scan-tree-dump-not "return 33" "ccp1" } } */
+/* { dg-final { cleanup-tree-dump "ccp1" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-1.C
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Index: testsuite/g++.dg/tree-ssa/pr19476-2.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-2.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-2.C	(working copy)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+#include <new>
+
+int f(){
+  int *p = new(std::nothrow) int;
+  return 33 + (0 == p);
+}
+int g(){
+  int *p = new int[50];
+  return 42 + (0 == p);
+}
+
+/* { dg-final { scan-tree-dump     "return 42" "optimized" } } */
+/* { dg-final { scan-tree-dump-not "return 33" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-2.C
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Index: testsuite/g++.dg/tree-ssa/pr19476-3.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-3.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-3.C	(working copy)
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fcheck-new -fdump-tree-optimized" } */
+
+#include <new>
+
+int g(){
+  return 42 + (0 == new int);
+}
+
+/* { dg-final { scan-tree-dump-not "return 42" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-3.C
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Index: testsuite/g++.dg/tree-ssa/pr19476-4.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-4.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-4.C	(working copy)
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fno-delete-null-pointer-checks -fdump-tree-optimized" } */
+
+#include <new>
+
+int g(){
+  return 42 + (0 == new int);
+}
+
+/* { dg-final { scan-tree-dump-not "return 42" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-4.C
___________________________________________________________________
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Index: tree-vrp.c
===================================================================
--- tree-vrp.c	(revision 202413)
+++ tree-vrp.c	(working copy)
@@ -1047,21 +1047,29 @@ gimple_assign_nonzero_warnv_p (gimple st
    *STRICT_OVERFLOW_P.*/
 
 static bool
 gimple_stmt_nonzero_warnv_p (gimple stmt, bool *strict_overflow_p)
 {
   switch (gimple_code (stmt))
     {
     case GIMPLE_ASSIGN:
       return gimple_assign_nonzero_warnv_p (stmt, strict_overflow_p);
     case GIMPLE_CALL:
-      return gimple_alloca_call_p (stmt);
+      {
+	tree fndecl = gimple_call_fndecl (stmt);
+	if (!fndecl) return false;
+	if (flag_delete_null_pointer_checks && !flag_check_new
+	    && DECL_IS_OPERATOR_NEW (fndecl)
+	    && !TREE_NOTHROW (fndecl))
+	  return true;
+	return gimple_alloca_call_p (stmt);
+      }
     default:
       gcc_unreachable ();
     }
 }
 
 /* Like tree_expr_nonzero_warnv_p, but this function uses value ranges
    obtained so far.  */
 
 static bool
 vrp_stmt_computes_nonzero (gimple stmt, bool *strict_overflow_p)
@@ -6486,21 +6494,22 @@ stmt_interesting_for_vrp (gimple stmt)
       tree lhs = gimple_get_lhs (stmt);
 
       /* In general, assignments with virtual operands are not useful
 	 for deriving ranges, with the obvious exception of calls to
 	 builtin functions.  */
       if (lhs && TREE_CODE (lhs) == SSA_NAME
 	  && (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
 	      || POINTER_TYPE_P (TREE_TYPE (lhs)))
 	  && ((is_gimple_call (stmt)
 	       && gimple_call_fndecl (stmt) != NULL_TREE
-	       && DECL_BUILT_IN (gimple_call_fndecl (stmt)))
+	       && (DECL_BUILT_IN (gimple_call_fndecl (stmt))
+		   || DECL_IS_OPERATOR_NEW (gimple_call_fndecl (stmt))))
 	      || !gimple_vuse (stmt)))
 	return true;
     }
   else if (gimple_code (stmt) == GIMPLE_COND
 	   || gimple_code (stmt) == GIMPLE_SWITCH)
     return true;
 
   return false;
 }
 
@@ -7407,30 +7416,21 @@ vrp_visit_stmt (gimple stmt, edge *taken
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
       fprintf (dump_file, "\nVisiting statement:\n");
       print_gimple_stmt (dump_file, stmt, 0, dump_flags);
       fprintf (dump_file, "\n");
     }
 
   if (!stmt_interesting_for_vrp (stmt))
     gcc_assert (stmt_ends_bb_p (stmt));
   else if (is_gimple_assign (stmt) || is_gimple_call (stmt))
-    {
-      /* In general, assignments with virtual operands are not useful
-	 for deriving ranges, with the obvious exception of calls to
-	 builtin functions.  */
-      if ((is_gimple_call (stmt)
-	   && gimple_call_fndecl (stmt) != NULL_TREE
-	   && DECL_BUILT_IN (gimple_call_fndecl (stmt)))
-	  || !gimple_vuse (stmt))
-	return vrp_visit_assignment_or_call (stmt, output_p);
-    }
+    return vrp_visit_assignment_or_call (stmt, output_p);
   else if (gimple_code (stmt) == GIMPLE_COND)
     return vrp_visit_cond_stmt (stmt, taken_edge_p);
   else if (gimple_code (stmt) == GIMPLE_SWITCH)
     return vrp_visit_switch_stmt (stmt, taken_edge_p);
 
   /* All other statements produce nothing of interest for VRP, so mark
      their outputs varying and prevent further simulation.  */
   FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_DEF)
     set_value_range_to_varying (get_value_range (def));
 

Re: operator new returns nonzero

[Marc Glisse]

Nobody has expressed concern for a week, so it may be worth doing an 
official review ;-)

http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00676.html

On Mon, 9 Sep 2013, Marc Glisse wrote:

> I have now tested bootstrap+testsuite and there was no regression.
>
> 2013-09-07  Marc Glisse  <marc.glisse@inria.fr>
>
> 	PR c++/19476
> gcc/
> 	* fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
> 	* tree-vrp.c (gimple_stmt_nonzero_warnv_p, stmt_interesting_for_vrp):
> 	Likewise.
> 	(vrp_visit_stmt): Remove duplicated code.
>
> gcc/testsuite/
> 	* g++.dg/tree-ssa/pr19476-1.C: New file.
> 	* g++.dg/tree-ssa/pr19476-2.C: Likewise.
> 	* g++.dg/tree-ssa/pr19476-3.C: Likewise.
> 	* g++.dg/tree-ssa/pr19476-4.C: Likewise.

-- 
Marc Glisse

Re: operator new returns nonzero

[Marc Glisse]

It isn't a front-end patch, but it is still a C++ patch, maybe Jason will 
have comments? Anyone else?

On Mon, 16 Sep 2013, Marc Glisse wrote:

> Nobody has expressed concern for a week, so it may be worth doing an official 
> review ;-)
>
> http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00676.html
>
> On Mon, 9 Sep 2013, Marc Glisse wrote:
>
>> I have now tested bootstrap+testsuite and there was no regression.
>> 
>> 2013-09-07  Marc Glisse  <marc.glisse@inria.fr>
>>
>> 	PR c++/19476
>> gcc/
>> 	* fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
>> 	* tree-vrp.c (gimple_stmt_nonzero_warnv_p, stmt_interesting_for_vrp):
>> 	Likewise.
>> 	(vrp_visit_stmt): Remove duplicated code.
>> 
>> gcc/testsuite/
>> 	* g++.dg/tree-ssa/pr19476-1.C: New file.
>> 	* g++.dg/tree-ssa/pr19476-2.C: Likewise.
>> 	* g++.dg/tree-ssa/pr19476-3.C: Likewise.
>> 	* g++.dg/tree-ssa/pr19476-4.C: Likewise.

-- 
Marc Glisse

Re: operator new returns nonzero

[Jakub Jelinek]

On Mon, Sep 09, 2013 at 10:49:40PM +0200, Marc Glisse wrote:
> --- fold-const.c	(revision 202413)
> +++ fold-const.c	(working copy)
> @@ -16171,21 +16171,31 @@ tree_expr_nonzero_warnv_p (tree t, bool
>      case MODIFY_EXPR:
>      case BIND_EXPR:
>        return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 1),
>  					strict_overflow_p);
>  
>      case SAVE_EXPR:
>        return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 0),
>  					strict_overflow_p);
>  
>      case CALL_EXPR:
> -      return alloca_call_p (t);
> +      {
> +	tree fn = CALL_EXPR_FN (t);
> +	if (TREE_CODE (fn) != ADDR_EXPR) return false;
> +	tree fndecl = TREE_OPERAND (fn, 0);
> +	if (TREE_CODE (fndecl) != FUNCTION_DECL) return false;
> +	if (flag_delete_null_pointer_checks && !flag_check_new
> +	    && DECL_IS_OPERATOR_NEW (fndecl)
> +	    && !TREE_NOTHROW (fndecl))
> +	  return true;
> +	return alloca_call_p (t);

Not commenting on what this patch does, but how:
why don't you use tree fndecl = get_callee_fndecl (t);
if (fndecl && ...) return true; instead?  Perhaps alloca_call_p should use
it too.

	Jakub

Re: operator new returns nonzero

[Marc Glisse]

On Wed, 2 Oct 2013, Jakub Jelinek wrote:

> On Mon, Sep 09, 2013 at 10:49:40PM +0200, Marc Glisse wrote:
>> --- fold-const.c	(revision 202413)
>> +++ fold-const.c	(working copy)
>> @@ -16171,21 +16171,31 @@ tree_expr_nonzero_warnv_p (tree t, bool
>>      case MODIFY_EXPR:
>>      case BIND_EXPR:
>>        return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 1),
>>  					strict_overflow_p);
>>
>>      case SAVE_EXPR:
>>        return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 0),
>>  					strict_overflow_p);
>>
>>      case CALL_EXPR:
>> -      return alloca_call_p (t);
>> +      {
>> +	tree fn = CALL_EXPR_FN (t);
>> +	if (TREE_CODE (fn) != ADDR_EXPR) return false;
>> +	tree fndecl = TREE_OPERAND (fn, 0);
>> +	if (TREE_CODE (fndecl) != FUNCTION_DECL) return false;
>> +	if (flag_delete_null_pointer_checks && !flag_check_new
>> +	    && DECL_IS_OPERATOR_NEW (fndecl)
>> +	    && !TREE_NOTHROW (fndecl))
>> +	  return true;
>> +	return alloca_call_p (t);
>
> Not commenting on what this patch does, but how:
> why don't you use tree fndecl = get_callee_fndecl (t);
> if (fndecl && ...) return true; instead?

Because I copied the code from alloca_call_p ;-)
get_callee_fndecl does look better indeed.

> Perhaps alloca_call_p should use it too.

Thanks, I'll prepare a new patch.

-- 
Marc Glisse

Re: operator new returns nonzero

[Marc Glisse]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 592 bytes --]

New version after Jakub's comment, bootstrap and testsuite on x86_64.

2013-10-03  Marc Glisse  <marc.glisse@inria.fr>

 	PR c++/19476
gcc/
 	* calls.c (alloca_call_p): Use get_callee_fndecl.
 	* fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
 	* tree-vrp.c (gimple_stmt_nonzero_warnv_p, stmt_interesting_for_vrp):
 	Likewise.
 	(vrp_visit_stmt): Remove duplicated code.

gcc/testsuite/
 	* g++.dg/tree-ssa/pr19476-1.C: New file.
 	* g++.dg/tree-ssa/pr19476-2.C: Likewise.
 	* g++.dg/tree-ssa/pr19476-3.C: Likewise.
 	* g++.dg/tree-ssa/pr19476-4.C: Likewise.


-- 
Marc Glisse

[-- Attachment #2: Type: TEXT/PLAIN, Size: 8891 bytes --]

Index: calls.c
===================================================================
--- calls.c	(revision 203101)
+++ calls.c	(working copy)
@@ -628,25 +628,24 @@ gimple_alloca_call_p (const_gimple stmt)
     return true;
 
   return false;
 }
 
 /* Return true when exp contains alloca call.  */
 
 bool
 alloca_call_p (const_tree exp)
 {
+  tree fndecl;
   if (TREE_CODE (exp) == CALL_EXPR
-      && TREE_CODE (CALL_EXPR_FN (exp)) == ADDR_EXPR
-      && (TREE_CODE (TREE_OPERAND (CALL_EXPR_FN (exp), 0)) == FUNCTION_DECL)
-      && (special_function_p (TREE_OPERAND (CALL_EXPR_FN (exp), 0), 0)
-	  & ECF_MAY_BE_ALLOCA))
+      && (fndecl = get_callee_fndecl (exp))
+      && (special_function_p (fndecl, 0) & ECF_MAY_BE_ALLOCA))
     return true;
   return false;
 }
 
 /* Return TRUE if FNDECL is either a TM builtin or a TM cloned
    function.  Return FALSE otherwise.  */
 
 static bool
 is_tm_builtin (const_tree fndecl)
 {
Index: fold-const.c
===================================================================
--- fold-const.c	(revision 203101)
+++ fold-const.c	(working copy)
@@ -16215,21 +16215,29 @@ tree_expr_nonzero_warnv_p (tree t, bool
     case MODIFY_EXPR:
     case BIND_EXPR:
       return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 1),
 					strict_overflow_p);
 
     case SAVE_EXPR:
       return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 0),
 					strict_overflow_p);
 
     case CALL_EXPR:
-      return alloca_call_p (t);
+      {
+	tree fndecl = get_callee_fndecl (t);
+	if (!fndecl) return false;
+	if (flag_delete_null_pointer_checks && !flag_check_new
+	    && DECL_IS_OPERATOR_NEW (fndecl)
+	    && !TREE_NOTHROW (fndecl))
+	  return true;
+	return alloca_call_p (t);
+      }
 
     default:
       break;
     }
   return false;
 }
 
 /* Return true when T is an address and is known to be nonzero.
    Handle warnings about undefined signed overflow.  */
 
Index: testsuite/g++.dg/tree-ssa/pr19476-1.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-1.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-1.C	(working copy)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-ccp1" } */
+
+#include <new>
+
+int f(){
+  return 33 + (0 == new(std::nothrow) int);
+}
+int g(){
+  return 42 + (0 == new int[50]);
+}
+
+/* { dg-final { scan-tree-dump     "return 42" "ccp1" } } */
+/* { dg-final { scan-tree-dump-not "return 33" "ccp1" } } */
+/* { dg-final { cleanup-tree-dump "ccp1" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-1.C
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Index: testsuite/g++.dg/tree-ssa/pr19476-2.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-2.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-2.C	(working copy)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+#include <new>
+
+int f(){
+  int *p = new(std::nothrow) int;
+  return 33 + (0 == p);
+}
+int g(){
+  int *p = new int[50];
+  return 42 + (0 == p);
+}
+
+/* { dg-final { scan-tree-dump     "return 42" "optimized" } } */
+/* { dg-final { scan-tree-dump-not "return 33" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-2.C
___________________________________________________________________
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Index: testsuite/g++.dg/tree-ssa/pr19476-3.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-3.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-3.C	(working copy)
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fcheck-new -fdump-tree-optimized" } */
+
+#include <new>
+
+int g(){
+  return 42 + (0 == new int);
+}
+
+/* { dg-final { scan-tree-dump-not "return 42" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-3.C
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Index: testsuite/g++.dg/tree-ssa/pr19476-4.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-4.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-4.C	(working copy)
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fno-delete-null-pointer-checks -fdump-tree-optimized" } */
+
+#include <new>
+
+int g(){
+  return 42 + (0 == new int);
+}
+
+/* { dg-final { scan-tree-dump-not "return 42" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-4.C
___________________________________________________________________
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Index: tree-vrp.c
===================================================================
--- tree-vrp.c	(revision 203101)
+++ tree-vrp.c	(working copy)
@@ -1045,21 +1045,29 @@ gimple_assign_nonzero_warnv_p (gimple st
    *STRICT_OVERFLOW_P.*/
 
 static bool
 gimple_stmt_nonzero_warnv_p (gimple stmt, bool *strict_overflow_p)
 {
   switch (gimple_code (stmt))
     {
     case GIMPLE_ASSIGN:
       return gimple_assign_nonzero_warnv_p (stmt, strict_overflow_p);
     case GIMPLE_CALL:
-      return gimple_alloca_call_p (stmt);
+      {
+	tree fndecl = gimple_call_fndecl (stmt);
+	if (!fndecl) return false;
+	if (flag_delete_null_pointer_checks && !flag_check_new
+	    && DECL_IS_OPERATOR_NEW (fndecl)
+	    && !TREE_NOTHROW (fndecl))
+	  return true;
+	return gimple_alloca_call_p (stmt);
+      }
     default:
       gcc_unreachable ();
     }
 }
 
 /* Like tree_expr_nonzero_warnv_p, but this function uses value ranges
    obtained so far.  */
 
 static bool
 vrp_stmt_computes_nonzero (gimple stmt, bool *strict_overflow_p)
@@ -6484,21 +6492,22 @@ stmt_interesting_for_vrp (gimple stmt)
       tree lhs = gimple_get_lhs (stmt);
 
       /* In general, assignments with virtual operands are not useful
 	 for deriving ranges, with the obvious exception of calls to
 	 builtin functions.  */
       if (lhs && TREE_CODE (lhs) == SSA_NAME
 	  && (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
 	      || POINTER_TYPE_P (TREE_TYPE (lhs)))
 	  && ((is_gimple_call (stmt)
 	       && gimple_call_fndecl (stmt) != NULL_TREE
-	       && DECL_BUILT_IN (gimple_call_fndecl (stmt)))
+	       && (DECL_BUILT_IN (gimple_call_fndecl (stmt))
+		   || DECL_IS_OPERATOR_NEW (gimple_call_fndecl (stmt))))
 	      || !gimple_vuse (stmt)))
 	return true;
     }
   else if (gimple_code (stmt) == GIMPLE_COND
 	   || gimple_code (stmt) == GIMPLE_SWITCH)
     return true;
 
   return false;
 }
 
@@ -7405,30 +7414,21 @@ vrp_visit_stmt (gimple stmt, edge *taken
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
       fprintf (dump_file, "\nVisiting statement:\n");
       print_gimple_stmt (dump_file, stmt, 0, dump_flags);
       fprintf (dump_file, "\n");
     }
 
   if (!stmt_interesting_for_vrp (stmt))
     gcc_assert (stmt_ends_bb_p (stmt));
   else if (is_gimple_assign (stmt) || is_gimple_call (stmt))
-    {
-      /* In general, assignments with virtual operands are not useful
-	 for deriving ranges, with the obvious exception of calls to
-	 builtin functions.  */
-      if ((is_gimple_call (stmt)
-	   && gimple_call_fndecl (stmt) != NULL_TREE
-	   && DECL_BUILT_IN (gimple_call_fndecl (stmt)))
-	  || !gimple_vuse (stmt))
-	return vrp_visit_assignment_or_call (stmt, output_p);
-    }
+    return vrp_visit_assignment_or_call (stmt, output_p);
   else if (gimple_code (stmt) == GIMPLE_COND)
     return vrp_visit_cond_stmt (stmt, taken_edge_p);
   else if (gimple_code (stmt) == GIMPLE_SWITCH)
     return vrp_visit_switch_stmt (stmt, taken_edge_p);
 
   /* All other statements produce nothing of interest for VRP, so mark
      their outputs varying and prevent further simulation.  */
   FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_DEF)
     set_value_range_to_varying (get_value_range (def));
 

Re: operator new returns nonzero

[Jason Merrill]

On 10/02/2013 08:33 AM, Marc Glisse wrote:
> +	if (flag_delete_null_pointer_checks && !flag_check_new

You can't use flag_check_new in language-independent code without moving 
it from c.opt to common.opt.

Jason

Re: operator new returns nonzero

[Marc Glisse]

On Wed, 2 Oct 2013, Jason Merrill wrote:

> On 10/02/2013 08:33 AM, Marc Glisse wrote:
>> +	if (flag_delete_null_pointer_checks && !flag_check_new
>
> You can't use flag_check_new in language-independent code without moving it 
> from c.opt to common.opt.

Thanks, that makes sense and I'll do that, but I am surprised that the 
fortran and java compilers built without complaining.

-- 
Marc Glisse

Re: operator new returns nonzero

[Jakub Jelinek]

On Wed, Oct 02, 2013 at 04:12:24PM +0300, Marc Glisse wrote:
> On Wed, 2 Oct 2013, Jason Merrill wrote:
> 
> >On 10/02/2013 08:33 AM, Marc Glisse wrote:
> >>+	if (flag_delete_null_pointer_checks && !flag_check_new
> >
> >You can't use flag_check_new in language-independent code without
> >moving it from c.opt to common.opt.
> 
> Thanks, that makes sense and I'll do that, but I am surprised that
> the fortran and java compilers built without complaining.

I think the macros are gathered for all the FEs configured, and as
the C FE is mandatory I think you can't end up with it not being
defined.

	Jakub

Re: operator new returns nonzero

[Marc Glisse]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 959 bytes --]

On Wed, 2 Oct 2013, Jason Merrill wrote:

> On 10/02/2013 08:33 AM, Marc Glisse wrote:
>> +	if (flag_delete_null_pointer_checks && !flag_check_new
>
> You can't use flag_check_new in language-independent code without moving it 
> from c.opt to common.opt.

New version, tested with bootstrap+testsuite on x86_64:

2013-10-03  Marc Glisse  <marc.glisse@inria.fr>

 	PR c++/19476
gcc/c-family/
 	* c.opt (fcheck-new): Move to common.opt.

gcc/
 	* common.opt (fcheck-new): Moved from c.opt. Make it 'Common'.
 	* calls.c (alloca_call_p): Use get_callee_fndecl.
 	* fold-const.c (tree_expr_nonzero_warnv_p): Handle operator new.
 	* tree-vrp.c (gimple_stmt_nonzero_warnv_p, stmt_interesting_for_vrp):
 	Likewise.
 	(vrp_visit_stmt): Remove duplicated code.

gcc/testsuite/
 	* g++.dg/tree-ssa/pr19476-1.C: New file.
 	* g++.dg/tree-ssa/pr19476-2.C: Likewise.
 	* g++.dg/tree-ssa/pr19476-3.C: Likewise.
 	* g++.dg/tree-ssa/pr19476-4.C: Likewise.


-- 
Marc Glisse

[-- Attachment #2: Type: TEXT/PLAIN, Size: 10698 bytes --]

Index: c-family/c.opt
===================================================================
--- c-family/c.opt	(revision 203101)
+++ c-family/c.opt	(working copy)
@@ -848,24 +848,20 @@ fbuiltin-
 C ObjC C++ ObjC++ Joined
 
 fcanonical-system-headers
 C ObjC C++ ObjC++
 Where shorter, use canonicalized paths to systems headers.
 
 fcilkplus
 C ObjC C++ ObjC++ LTO Report Var(flag_enable_cilkplus) Init(0)
 Enable Cilk Plus
 
-fcheck-new
-C++ ObjC++ Var(flag_check_new)
-Check the return value of new
-
 fcond-mismatch
 C ObjC C++ ObjC++
 Allow the arguments of the '?' operator to have different types
 
 fconserve-space
 C++ ObjC++ Var(flag_conserve_space)
 Does nothing.  Preserved for backward compatibility.
 
 fconstant-string-class=
 ObjC ObjC++ Joined MissingArgError(no class name specified with %qs)
Index: calls.c
===================================================================
--- calls.c	(revision 203101)
+++ calls.c	(working copy)
@@ -628,25 +628,24 @@ gimple_alloca_call_p (const_gimple stmt)
     return true;
 
   return false;
 }
 
 /* Return true when exp contains alloca call.  */
 
 bool
 alloca_call_p (const_tree exp)
 {
+  tree fndecl;
   if (TREE_CODE (exp) == CALL_EXPR
-      && TREE_CODE (CALL_EXPR_FN (exp)) == ADDR_EXPR
-      && (TREE_CODE (TREE_OPERAND (CALL_EXPR_FN (exp), 0)) == FUNCTION_DECL)
-      && (special_function_p (TREE_OPERAND (CALL_EXPR_FN (exp), 0), 0)
-	  & ECF_MAY_BE_ALLOCA))
+      && (fndecl = get_callee_fndecl (exp))
+      && (special_function_p (fndecl, 0) & ECF_MAY_BE_ALLOCA))
     return true;
   return false;
 }
 
 /* Return TRUE if FNDECL is either a TM builtin or a TM cloned
    function.  Return FALSE otherwise.  */
 
 static bool
 is_tm_builtin (const_tree fndecl)
 {
Index: common.opt
===================================================================
--- common.opt	(revision 203101)
+++ common.opt	(working copy)
@@ -906,20 +906,24 @@ Common Joined RejectNegative Var(common_
 ; be saved across function calls, if that produces overall better code.
 ; Optional now, so people can test it.
 fcaller-saves
 Common Report Var(flag_caller_saves) Optimization
 Save registers around function calls
 
 fcheck-data-deps
 Common Report Var(flag_check_data_deps)
 Compare the results of several data dependence analyzers.
 
+fcheck-new
+Common Var(flag_check_new)
+Check the return value of new in C++
+
 fcombine-stack-adjustments
 Common Report Var(flag_combine_stack_adjustments) Optimization
 Looks for opportunities to reduce stack adjustments and stack references.
 
 fcommon
 Common Report Var(flag_no_common,0) Optimization
 Do not put uninitialized globals in the common section
 
 fcompare-debug
 Driver
Index: fold-const.c
===================================================================
--- fold-const.c	(revision 203101)
+++ fold-const.c	(working copy)
@@ -16215,21 +16215,29 @@ tree_expr_nonzero_warnv_p (tree t, bool
     case MODIFY_EXPR:
     case BIND_EXPR:
       return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 1),
 					strict_overflow_p);
 
     case SAVE_EXPR:
       return tree_expr_nonzero_warnv_p (TREE_OPERAND (t, 0),
 					strict_overflow_p);
 
     case CALL_EXPR:
-      return alloca_call_p (t);
+      {
+	tree fndecl = get_callee_fndecl (t);
+	if (!fndecl) return false;
+	if (flag_delete_null_pointer_checks && !flag_check_new
+	    && DECL_IS_OPERATOR_NEW (fndecl)
+	    && !TREE_NOTHROW (fndecl))
+	  return true;
+	return alloca_call_p (t);
+      }
 
     default:
       break;
     }
   return false;
 }
 
 /* Return true when T is an address and is known to be nonzero.
    Handle warnings about undefined signed overflow.  */
 
Index: testsuite/g++.dg/tree-ssa/pr19476-1.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-1.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-1.C	(working copy)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-ccp1" } */
+
+#include <new>
+
+int f(){
+  return 33 + (0 == new(std::nothrow) int);
+}
+int g(){
+  return 42 + (0 == new int[50]);
+}
+
+/* { dg-final { scan-tree-dump     "return 42" "ccp1" } } */
+/* { dg-final { scan-tree-dump-not "return 33" "ccp1" } } */
+/* { dg-final { cleanup-tree-dump "ccp1" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-1.C
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Index: testsuite/g++.dg/tree-ssa/pr19476-2.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-2.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-2.C	(working copy)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+#include <new>
+
+int f(){
+  int *p = new(std::nothrow) int;
+  return 33 + (0 == p);
+}
+int g(){
+  int *p = new int[50];
+  return 42 + (0 == p);
+}
+
+/* { dg-final { scan-tree-dump     "return 42" "optimized" } } */
+/* { dg-final { scan-tree-dump-not "return 33" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-2.C
___________________________________________________________________
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Index: testsuite/g++.dg/tree-ssa/pr19476-3.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-3.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-3.C	(working copy)
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fcheck-new -fdump-tree-optimized" } */
+
+#include <new>
+
+int g(){
+  return 42 + (0 == new int);
+}
+
+/* { dg-final { scan-tree-dump-not "return 42" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-3.C
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Index: testsuite/g++.dg/tree-ssa/pr19476-4.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr19476-4.C	(revision 0)
+++ testsuite/g++.dg/tree-ssa/pr19476-4.C	(working copy)
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fno-delete-null-pointer-checks -fdump-tree-optimized" } */
+
+#include <new>
+
+int g(){
+  return 42 + (0 == new int);
+}
+
+/* { dg-final { scan-tree-dump-not "return 42" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */

Property changes on: testsuite/g++.dg/tree-ssa/pr19476-4.C
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
Added: svn:keywords
## -0,0 +1 ##
+Author Date Id Revision URL
\ No newline at end of property
Index: tree-vrp.c
===================================================================
--- tree-vrp.c	(revision 203101)
+++ tree-vrp.c	(working copy)
@@ -1045,21 +1045,29 @@ gimple_assign_nonzero_warnv_p (gimple st
    *STRICT_OVERFLOW_P.*/
 
 static bool
 gimple_stmt_nonzero_warnv_p (gimple stmt, bool *strict_overflow_p)
 {
   switch (gimple_code (stmt))
     {
     case GIMPLE_ASSIGN:
       return gimple_assign_nonzero_warnv_p (stmt, strict_overflow_p);
     case GIMPLE_CALL:
-      return gimple_alloca_call_p (stmt);
+      {
+	tree fndecl = gimple_call_fndecl (stmt);
+	if (!fndecl) return false;
+	if (flag_delete_null_pointer_checks && !flag_check_new
+	    && DECL_IS_OPERATOR_NEW (fndecl)
+	    && !TREE_NOTHROW (fndecl))
+	  return true;
+	return gimple_alloca_call_p (stmt);
+      }
     default:
       gcc_unreachable ();
     }
 }
 
 /* Like tree_expr_nonzero_warnv_p, but this function uses value ranges
    obtained so far.  */
 
 static bool
 vrp_stmt_computes_nonzero (gimple stmt, bool *strict_overflow_p)
@@ -6484,21 +6492,22 @@ stmt_interesting_for_vrp (gimple stmt)
       tree lhs = gimple_get_lhs (stmt);
 
       /* In general, assignments with virtual operands are not useful
 	 for deriving ranges, with the obvious exception of calls to
 	 builtin functions.  */
       if (lhs && TREE_CODE (lhs) == SSA_NAME
 	  && (INTEGRAL_TYPE_P (TREE_TYPE (lhs))
 	      || POINTER_TYPE_P (TREE_TYPE (lhs)))
 	  && ((is_gimple_call (stmt)
 	       && gimple_call_fndecl (stmt) != NULL_TREE
-	       && DECL_BUILT_IN (gimple_call_fndecl (stmt)))
+	       && (DECL_BUILT_IN (gimple_call_fndecl (stmt))
+		   || DECL_IS_OPERATOR_NEW (gimple_call_fndecl (stmt))))
 	      || !gimple_vuse (stmt)))
 	return true;
     }
   else if (gimple_code (stmt) == GIMPLE_COND
 	   || gimple_code (stmt) == GIMPLE_SWITCH)
     return true;
 
   return false;
 }
 
@@ -7405,30 +7414,21 @@ vrp_visit_stmt (gimple stmt, edge *taken
   if (dump_file && (dump_flags & TDF_DETAILS))
     {
       fprintf (dump_file, "\nVisiting statement:\n");
       print_gimple_stmt (dump_file, stmt, 0, dump_flags);
       fprintf (dump_file, "\n");
     }
 
   if (!stmt_interesting_for_vrp (stmt))
     gcc_assert (stmt_ends_bb_p (stmt));
   else if (is_gimple_assign (stmt) || is_gimple_call (stmt))
-    {
-      /* In general, assignments with virtual operands are not useful
-	 for deriving ranges, with the obvious exception of calls to
-	 builtin functions.  */
-      if ((is_gimple_call (stmt)
-	   && gimple_call_fndecl (stmt) != NULL_TREE
-	   && DECL_BUILT_IN (gimple_call_fndecl (stmt)))
-	  || !gimple_vuse (stmt))
-	return vrp_visit_assignment_or_call (stmt, output_p);
-    }
+    return vrp_visit_assignment_or_call (stmt, output_p);
   else if (gimple_code (stmt) == GIMPLE_COND)
     return vrp_visit_cond_stmt (stmt, taken_edge_p);
   else if (gimple_code (stmt) == GIMPLE_SWITCH)
     return vrp_visit_switch_stmt (stmt, taken_edge_p);
 
   /* All other statements produce nothing of interest for VRP, so mark
      their outputs varying and prevent further simulation.  */
   FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_DEF)
     set_value_range_to_varying (get_value_range (def));
 

Re: operator new returns nonzero

[Jason Merrill]

OK.

Jason