[Bug c++/97151] GCC fails to optimize away uselessly allocated arrays (C++)

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97151

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #6)
> (In reply to Marat Radchenko from comment #4)
> > (In reply to Martin Liška from comment #3)
> > > It's caused by what was mentioned as 2) which transforms to:
> > > 
> > >   _6 = operator new [] (40);
> > >   __builtin_memset (_6, 0, 40);
> > >   operator delete [] (_6);
> > > 
> > > So there's a use of _6 and we probably can't remove it as memset can
> > > potentially access NULL pointer if operator new [] fails.
> > 
> > 1. If operator new[] fails, there would be an exception instead of NULL, no?
> > 
> > 2. It doesn't matter, you may guard accessing array with NULL check and
> > still observe described behavior.
> > 
> > What puzzles me the most is why removal of delete[] allows it to optimize.
> 
> Very likely delete[] is keeping the memset itself live.  In particular
> early CDDCE can elide the loop without the delete[] but with the delete[]
> we see
> 
> Marking useful stmt: operator delete [] (_7);
> 
> so that's a no-go there.  Using malloc/free we manage to delete the loop
> early
> because its argument is not considered necessary.  It looks like points-to
> analysis fails to fully handle new/delete and causes the allocated storage
> to point to escaped memory:
> 
> _7 = &HEAP(10)
> 
> new[] handled OK
> 
> ESCAPED = _7
> 
> delete[] not handled.  One "easy" way would be for the C++ FE to annotate
> delete[] with fnspec ".w" to mark the argument as not escaping.  A hackish
> patch in PTA also fixes it:
> 
> diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
> index 44fe52e0f65..f676bf91e95 100644
> --- a/gcc/tree-ssa-structalias.c
> +++ b/gcc/tree-ssa-structalias.c
> @@ -4857,6 +4857,8 @@ find_func_aliases_for_call (struct function *fn, gcall
> *t)
>          point for reachable memory of their arguments.  */
>        else if (flags & (ECF_PURE|ECF_LOOPING_CONST_OR_PURE))
>         handle_pure_call (t, &rhsc);
> +      else if (fndecl && DECL_IS_REPLACEABLE_OPERATOR_DELETE_P (fndecl))
> +       ;
>        else
>         handle_rhs_call (t, &rhsc);
>        if (gimple_call_lhs (t))

passes bootstrap & regtest, also improving g++.dg/cpp1y/new1.C as of

PASS: g++.dg/cpp1y/new1.C  -std=gnu++98 (test for excess errors)
g++.dg/cpp1y/new1.C  -std=gnu++98 : pattern found 6 times
FAIL: g++.dg/cpp1y/new1.C  -std=gnu++98  scan-tree-dump-times cddce1 "Deleting
: operator delete" 5
g++.dg/cpp1y/new1.C  -std=gnu++98 : pattern found 8 times
FAIL: g++.dg/cpp1y/new1.C  -std=gnu++98  scan-tree-dump-times cddce1 "Deleting
: _\\d+ = operator new" 7

note there are 10 functions in new1.C and there's no XFAIL so it's not clear
whether the currently not optimized cases are to be not optimized for
correctness or whether those are just missed optimizations...

The patch might be even OK in case DECL_IS_REPLACEABLE_OPERATOR_DELETE_P
implies that there is no return value and no argument escapes (IIRC there's
only ever a single argument to operator delete).  Note the patch implies
that the storage is not written to, but I suppose its contents become
undefined after 'delete'.