diff mbox

operator new returns nonzero

Message ID alpine.DEB.2.02.1309072256440.4641@stedding.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse Sept. 7, 2013, 9 p.m. UTC
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.

Comments

Richard Biener Sept. 9, 2013, 9:06 a.m. UTC | #1
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));
>
>
Marc Glisse Sept. 9, 2013, 9:19 a.m. UTC | #2
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...)
Gabriel Dos Reis Sept. 9, 2013, 12:41 p.m. UTC | #3
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));
>>
>>
Gabriel Dos Reis Sept. 9, 2013, 12:43 p.m. UTC | #4
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
Mike Stump Sept. 9, 2013, 4:16 p.m. UTC | #5
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.
Mike Stump Sept. 9, 2013, 4:26 p.m. UTC | #6
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.
Gabriel Dos Reis Sept. 9, 2013, 5:25 p.m. UTC | #7
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
Mike Stump Sept. 9, 2013, 5:54 p.m. UTC | #8
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.
Gabriel Dos Reis Sept. 9, 2013, 7:43 p.m. UTC | #9
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
diff mbox

Patch

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