Re: [PATCH] Remove also 2nd argument for unused delete operator (PR tree-optimization/91270).

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Jul 30, 2019 at 3:39 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 7/30/19 3:09 PM, Marc Glisse wrote:
> > On Tue, 30 Jul 2019, Martin Liška wrote:
> >
> >> On 7/30/19 1:35 PM, Marc Glisse wrote:
> >>> +          /* Some delete operators have size as 2nd argument.  */
> >>>
> >>> Some delete operators have 3 arguments. From cp/decl.c:
> >>>
> >>>             /* operator delete (void *, size_t, align_val_t); */
> >>>
> >>
> >> Yep, I know. The patch I installed expects at least 2 arguments:
> >>
> >> +                 /* Some delete operators have size as 2nd argument.  */
> >> +                 if (is_delete_operator && gimple_call_num_args (stmt) >= 2)
> >
> > True, I guess I am a bit confused why the second argument (which could be either size or alignment) needs special handling (mark_operand_necessary) while the third one does not (it is usually a constant).
>
> Ah, that's bad, both of them need a care:
>
> diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
> index bec13cd5930..80d5f5c30f7 100644
> --- a/gcc/tree-ssa-dce.c
> +++ b/gcc/tree-ssa-dce.c
> @@ -824,13 +824,16 @@ propagate_necessity (bool aggressive)
>                            || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_CALLOC))
>                       || DECL_IS_REPLACEABLE_OPERATOR_NEW_P (def_callee)))
>                 {
> -                 /* Some delete operators have size as 2nd argument.  */
> +                 /* Delete operators can have alignment and (or) size as next
> +                    arguments.  When being a SSA_NAME, they must be marked
> +                    as necessary.  */
>                   if (is_delete_operator && gimple_call_num_args (stmt) >= 2)
> -                   {
> -                     tree size_argument = gimple_call_arg (stmt, 1);
> -                     if (TREE_CODE (size_argument) == SSA_NAME)
> -                       mark_operand_necessary (size_argument);
> -                   }
> +                   for (unsigned i = 1; i < gimple_call_num_args (stmt); i++)
> +                     {
> +                       tree arg = gimple_call_arg (stmt, i);
> +                       if (TREE_CODE (arg) == SSA_NAME)
> +                         mark_operand_necessary (arg);
> +                     }
>
>                   continue;
>                 }

Pre-approved.

> >
> > I tried to experiment to understand, but it is complicated because including <new> disables the optimization:
> >
> > #include <new>
> > void fn1() {
> >     char*p=new char;
> >     delete p;
> > }
> >
> > This ICEs with -O -std=c++17:
> >
> > int a = 64;
> > std::align_val_t b{64};
> > void fn1() {
> >   void *s = operator new(a,b);
> >   operator delete(s,8+*(unsigned long*)s,b);
> > }
> >
> >
>
> I can't see it on current master. Can you?
>
> Martin
>