Patchwork operator new returns nonzero

login
register
mail settings
Submitter Marc Glisse
Date Sept. 9, 2013, 8:49 p.m.
Message ID <alpine.DEB.2.02.1309092247150.14523@stedding.saclay.inria.fr>
Download mbox | patch
Permalink /patch/273710/
State New
Headers show

Comments

Marc Glisse - Sept. 9, 2013, 8:49 p.m.
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 - Sept. 16, 2013, 9:56 p.m.
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 - Oct. 2, 2013, 6:59 a.m.
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.
Jakub Jelinek - Oct. 2, 2013, 7:06 a.m.
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
Marc Glisse - Oct. 2, 2013, 7:18 a.m.
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.

Patch

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));