Re: [PATCH] tree-optimization/97151 - improve PTA for C++ operator delete

2020-10-02 Thread Richard Biener
On Thu, 1 Oct 2020, Jason Merrill wrote:

> On 10/1/20 5:26 AM, Richard Biener wrote:
> > On Wed, 30 Sep 2020, Jason Merrill wrote:
> > 
> >> On 9/28/20 3:09 PM, Jason Merrill wrote:
> >>> On 9/28/20 3:56 AM, Richard Biener wrote:
>  On Fri, 25 Sep 2020, Jason Merrill wrote:
> 
> > On 9/25/20 2:30 AM, Richard Biener wrote:
> >> On Thu, 24 Sep 2020, Jason Merrill wrote:
> >>
> >>> On 9/24/20 3:43 AM, Richard Biener wrote:
>  On Wed, 23 Sep 2020, Jason Merrill wrote:
> 
> > On 9/23/20 2:42 PM, Richard Biener wrote:
> >> On September 23, 2020 7:53:18 PM GMT+02:00, Jason Merrill
> >> 
> >> wrote:
> >>> On 9/23/20 4:14 AM, Richard Biener wrote:
>  C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
>  does not cause the deleted object to be escaped.? It also has no
>  other interesting side-effects for PTA so skip it like we do
>  for BUILT_IN_FREE.
> >>>
> >>> Hmm, this is true of the default implementation, but since the
> >>> function
> >>>
> >>> is replaceable, we don't know what a user definition might do with
> >>> the
> >>> pointer.
> >>
> >> But can the object still be 'used' after delete? Can delete fail /
> >> throw?
> >>
> >> What guarantee does the predicate give us?
> >
> > The deallocation function is called as part of a delete expression
> > in
> > order
> > to
> > release the storage for an object, ending its lifetime (if it was
> > not
> > ended
> > by
> > a destructor), so no, the object can't be used afterward.
> 
>  OK, but the delete operator can access the object contents if there
>  wasn't a destructor ...
> >>>
> > A deallocation function that throws has undefined behavior.
> 
>  OK, so it seems the 'replaceable' operators are the global ones
>  (for user-defined/class-specific placement variants I see arbitrary
>  extra arguments that we'd possibly need to handle).
> 
>  I'm happy to revert but I'd like to have a testcase that FAILs
>  with the patch ;)
> 
>  Now, the following aborts:
> 
>  struct X {
>   static struct X saved;
>   int *p;
>   X() { __builtin_memcpy (this, , sizeof (X)); }
>  };
>  void operator delete (void *p)
>  {
>   __builtin_memcpy (::saved, p, sizeof (X));
>  }
>  int main()
>  {
>   int y = 1;
>   X *p = new X;
>   p->p = 
>   delete p;
>   X *q = new X;
>   *(q->p) = 2;
>   if (y != 2)
>  ?? __builtin_abort ();
>  }
> 
>  and I could fix this by not making *p but what *p points to escape.
>  The testcase is of course maximally awkward, but hey ... ;)
> 
>  Now this would all be moot if operator delete may not access
>  the object (or if the object contents are undefined at that point).
> 
>  Oh, and the testcase segfaults when compiled with GCC 10 because
>  there we elide the new X / delete p pair ... which is invalid then?
>  Hmm, we emit
> 
>   MEM[(struct X *)_8] ={v} {CLOBBER};
>   operator delete (_8, 8);
> 
>  so the object contents are undefined _before_ calling delete
>  even when I do not have a DTOR?? That is, the above,
>  w/o -fno-lifetime-dse, makes the PTA patch OK for the testcase.
> >>>
> >>> Yes, all classes have a destructor, even if it's trivial, so the
> >>> object's
> >>> lifetime definitely ends before the call to operator delete. This is
> >>> less
> >>> clear for scalar objects, but treating them similarly would be
> >>> consistent
> >>> with
> >>> other recent changes, so I think it's fine for us to assume that
> >>> scalar
> >>> objects are also invalidated before the call to operator delete.  But
> >>> of
> >>> course this doesn't apply to explicit calls to operator delete outside
> >>> of a
> >>> delete expression.
> >>
> >> OK, so change the testcase main slightly to
> >>
> >> int main()
> >> {
> >> ??? int y = 1;
> >> ??? X *p = new X;
> >> ??? p->p = 
> >> ??? ::operator delete(p);
> >> ??? X *q = new X;
> >> ??? *(q->p) = 2;
> >> ??? if (y != 2)
> >> ? __builtin_abort ();
> >> }
> >>
> >> in this case the lifetime of *p does not end before calling
> >> ::operator delete() and delete can stash the object contents
> >> somewhere before ending its lifetime.? For the very same reason
> >> we may not elide a new/delete pair 

Re: [PATCH] tree-optimization/97151 - improve PTA for C++ operator delete

2020-10-01 Thread Jason Merrill via Gcc-patches

On 10/1/20 5:26 AM, Richard Biener wrote:

On Wed, 30 Sep 2020, Jason Merrill wrote:


On 9/28/20 3:09 PM, Jason Merrill wrote:

On 9/28/20 3:56 AM, Richard Biener wrote:

On Fri, 25 Sep 2020, Jason Merrill wrote:


On 9/25/20 2:30 AM, Richard Biener wrote:

On Thu, 24 Sep 2020, Jason Merrill wrote:


On 9/24/20 3:43 AM, Richard Biener wrote:

On Wed, 23 Sep 2020, Jason Merrill wrote:


On 9/23/20 2:42 PM, Richard Biener wrote:

On September 23, 2020 7:53:18 PM GMT+02:00, Jason Merrill

wrote:

On 9/23/20 4:14 AM, Richard Biener wrote:

C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
does not cause the deleted object to be escaped.? It also has no
other interesting side-effects for PTA so skip it like we do
for BUILT_IN_FREE.


Hmm, this is true of the default implementation, but since the
function

is replaceable, we don't know what a user definition might do with
the
pointer.


But can the object still be 'used' after delete? Can delete fail /
throw?

What guarantee does the predicate give us?


The deallocation function is called as part of a delete expression in
order
to
release the storage for an object, ending its lifetime (if it was not
ended
by
a destructor), so no, the object can't be used afterward.


OK, but the delete operator can access the object contents if there
wasn't a destructor ...



A deallocation function that throws has undefined behavior.


OK, so it seems the 'replaceable' operators are the global ones
(for user-defined/class-specific placement variants I see arbitrary
extra arguments that we'd possibly need to handle).

I'm happy to revert but I'd like to have a testcase that FAILs
with the patch ;)

Now, the following aborts:

struct X {
 static struct X saved;
 int *p;
 X() { __builtin_memcpy (this, , sizeof (X)); }
};
void operator delete (void *p)
{
 __builtin_memcpy (::saved, p, sizeof (X));
}
int main()
{
 int y = 1;
 X *p = new X;
 p->p = 
 delete p;
 X *q = new X;
 *(q->p) = 2;
 if (y != 2)
?? __builtin_abort ();
}

and I could fix this by not making *p but what *p points to escape.
The testcase is of course maximally awkward, but hey ... ;)

Now this would all be moot if operator delete may not access
the object (or if the object contents are undefined at that point).

Oh, and the testcase segfaults when compiled with GCC 10 because
there we elide the new X / delete p pair ... which is invalid then?
Hmm, we emit

 MEM[(struct X *)_8] ={v} {CLOBBER};
 operator delete (_8, 8);

so the object contents are undefined _before_ calling delete
even when I do not have a DTOR?? That is, the above,
w/o -fno-lifetime-dse, makes the PTA patch OK for the testcase.


Yes, all classes have a destructor, even if it's trivial, so the
object's
lifetime definitely ends before the call to operator delete. This is
less
clear for scalar objects, but treating them similarly would be
consistent
with
other recent changes, so I think it's fine for us to assume that scalar
objects are also invalidated before the call to operator delete.  But of
course this doesn't apply to explicit calls to operator delete outside
of a
delete expression.


OK, so change the testcase main slightly to

int main()
{
??? int y = 1;
??? X *p = new X;
??? p->p = 
??? ::operator delete(p);
??? X *q = new X;
??? *(q->p) = 2;
??? if (y != 2)
? __builtin_abort ();
}

in this case the lifetime of *p does not end before calling
::operator delete() and delete can stash the object contents
somewhere before ending its lifetime.? For the very same reason
we may not elide a new/delete pair like in

int main()
{
??? int *p = new int;
??? *p = 1;
??? ::operator delete (p);
}


Correct; the permission to elide new/delete pairs are for the expressions,
not
the functions.


which we before the change did not do only because calling
operator delete made p escape.? Unfortunately points-to analysis
cannot really reconstruct whether delete was called as part of
a delete expression or directly (and thus whether object lifetime
ended already), neither can DCE.? So I guess we need to mark
the operator delete call in some way to make those transforms
safe.? At least currently any operator delete call makes the
alias guarantee of a operator new call moot by forcing the object
to be aliased with all global and escaped memory ...

Looks like there are some unallocated flags for CALL_EXPR we could
pick but I wonder if we can recycle protected_flag which is

 CALL_FROM_THUNK_P and
 CALL_ALLOCA_FOR_VAR_P in
 CALL_EXPR

for calls to DECL_IS_OPERATOR_{NEW,DELETE}_P, thus whether
we have CALL_FROM_THUNK_P for those operators.? Guess picking
a new flag is safer.


We won't ever call those operators from a thunk, so it should be OK to
reuse
it.


But, does it seem correct that we need to distinguish
delete expressions from plain calls to operator delete?


A reason for that distinction came up in the context of omitting
new/delete
pairs: we want to 

Re: [PATCH] tree-optimization/97151 - improve PTA for C++ operator delete

2020-10-01 Thread Richard Biener
On Wed, 30 Sep 2020, Jason Merrill wrote:

> On 9/28/20 3:09 PM, Jason Merrill wrote:
> > On 9/28/20 3:56 AM, Richard Biener wrote:
> >> On Fri, 25 Sep 2020, Jason Merrill wrote:
> >>
> >>> On 9/25/20 2:30 AM, Richard Biener wrote:
>  On Thu, 24 Sep 2020, Jason Merrill wrote:
> 
> > On 9/24/20 3:43 AM, Richard Biener wrote:
> >> On Wed, 23 Sep 2020, Jason Merrill wrote:
> >>
> >>> On 9/23/20 2:42 PM, Richard Biener wrote:
>  On September 23, 2020 7:53:18 PM GMT+02:00, Jason Merrill
>  
>  wrote:
> > On 9/23/20 4:14 AM, Richard Biener wrote:
> >> C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
> >> does not cause the deleted object to be escaped.? It also has no
> >> other interesting side-effects for PTA so skip it like we do
> >> for BUILT_IN_FREE.
> >
> > Hmm, this is true of the default implementation, but since the
> > function
> >
> > is replaceable, we don't know what a user definition might do with
> > the
> > pointer.
> 
>  But can the object still be 'used' after delete? Can delete fail /
>  throw?
> 
>  What guarantee does the predicate give us?
> >>>
> >>> The deallocation function is called as part of a delete expression in
> >>> order
> >>> to
> >>> release the storage for an object, ending its lifetime (if it was not
> >>> ended
> >>> by
> >>> a destructor), so no, the object can't be used afterward.
> >>
> >> OK, but the delete operator can access the object contents if there
> >> wasn't a destructor ...
> >
> >>> A deallocation function that throws has undefined behavior.
> >>
> >> OK, so it seems the 'replaceable' operators are the global ones
> >> (for user-defined/class-specific placement variants I see arbitrary
> >> extra arguments that we'd possibly need to handle).
> >>
> >> I'm happy to revert but I'd like to have a testcase that FAILs
> >> with the patch ;)
> >>
> >> Now, the following aborts:
> >>
> >> struct X {
> >>  static struct X saved;
> >>  int *p;
> >>  X() { __builtin_memcpy (this, , sizeof (X)); }
> >> };
> >> void operator delete (void *p)
> >> {
> >>  __builtin_memcpy (::saved, p, sizeof (X));
> >> }
> >> int main()
> >> {
> >>  int y = 1;
> >>  X *p = new X;
> >>  p->p = 
> >>  delete p;
> >>  X *q = new X;
> >>  *(q->p) = 2;
> >>  if (y != 2)
> >> ?? __builtin_abort ();
> >> }
> >>
> >> and I could fix this by not making *p but what *p points to escape.
> >> The testcase is of course maximally awkward, but hey ... ;)
> >>
> >> Now this would all be moot if operator delete may not access
> >> the object (or if the object contents are undefined at that point).
> >>
> >> Oh, and the testcase segfaults when compiled with GCC 10 because
> >> there we elide the new X / delete p pair ... which is invalid then?
> >> Hmm, we emit
> >>
> >>  MEM[(struct X *)_8] ={v} {CLOBBER};
> >>  operator delete (_8, 8);
> >>
> >> so the object contents are undefined _before_ calling delete
> >> even when I do not have a DTOR?? That is, the above,
> >> w/o -fno-lifetime-dse, makes the PTA patch OK for the testcase.
> >
> > Yes, all classes have a destructor, even if it's trivial, so the
> > object's
> > lifetime definitely ends before the call to operator delete. This is
> > less
> > clear for scalar objects, but treating them similarly would be
> > consistent
> > with
> > other recent changes, so I think it's fine for us to assume that scalar
> > objects are also invalidated before the call to operator delete.  But of
> > course this doesn't apply to explicit calls to operator delete outside
> > of a
> > delete expression.
> 
>  OK, so change the testcase main slightly to
> 
>  int main()
>  {
>  ??? int y = 1;
>  ??? X *p = new X;
>  ??? p->p = 
>  ??? ::operator delete(p);
>  ??? X *q = new X;
>  ??? *(q->p) = 2;
>  ??? if (y != 2)
>  ? __builtin_abort ();
>  }
> 
>  in this case the lifetime of *p does not end before calling
>  ::operator delete() and delete can stash the object contents
>  somewhere before ending its lifetime.? For the very same reason
>  we may not elide a new/delete pair like in
> 
>  int main()
>  {
>  ??? int *p = new int;
>  ??? *p = 1;
>  ??? ::operator delete (p);
>  }
> >>>
> >>> Correct; the permission to elide new/delete pairs are for the expressions,
> >>> not
> >>> the functions.
> >>>
>  which we before the change did not do only because calling
>  operator delete made p escape.? Unfortunately points-to 

Re: [PATCH] tree-optimization/97151 - improve PTA for C++ operator delete

2020-09-30 Thread Jason Merrill via Gcc-patches

On 9/28/20 3:09 PM, Jason Merrill wrote:

On 9/28/20 3:56 AM, Richard Biener wrote:

On Fri, 25 Sep 2020, Jason Merrill wrote:


On 9/25/20 2:30 AM, Richard Biener wrote:

On Thu, 24 Sep 2020, Jason Merrill wrote:


On 9/24/20 3:43 AM, Richard Biener wrote:

On Wed, 23 Sep 2020, Jason Merrill wrote:


On 9/23/20 2:42 PM, Richard Biener wrote:

On September 23, 2020 7:53:18 PM GMT+02:00, Jason Merrill

wrote:

On 9/23/20 4:14 AM, Richard Biener wrote:

C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
does not cause the deleted object to be escaped.  It also has no
other interesting side-effects for PTA so skip it like we do
for BUILT_IN_FREE.


Hmm, this is true of the default implementation, but since the 
function


is replaceable, we don't know what a user definition might do 
with the

pointer.


But can the object still be 'used' after delete? Can delete fail /
throw?

What guarantee does the predicate give us?


The deallocation function is called as part of a delete 
expression in

order
to
release the storage for an object, ending its lifetime (if it was 
not

ended
by
a destructor), so no, the object can't be used afterward.


OK, but the delete operator can access the object contents if there
wasn't a destructor ...



A deallocation function that throws has undefined behavior.


OK, so it seems the 'replaceable' operators are the global ones
(for user-defined/class-specific placement variants I see arbitrary
extra arguments that we'd possibly need to handle).

I'm happy to revert but I'd like to have a testcase that FAILs
with the patch ;)

Now, the following aborts:

struct X {
 static struct X saved;
 int *p;
 X() { __builtin_memcpy (this, , sizeof (X)); }
};
void operator delete (void *p)
{
 __builtin_memcpy (::saved, p, sizeof (X));
}
int main()
{
 int y = 1;
 X *p = new X;
 p->p = 
 delete p;
 X *q = new X;
 *(q->p) = 2;
 if (y != 2)
   __builtin_abort ();
}

and I could fix this by not making *p but what *p points to escape.
The testcase is of course maximally awkward, but hey ... ;)

Now this would all be moot if operator delete may not access
the object (or if the object contents are undefined at that point).

Oh, and the testcase segfaults when compiled with GCC 10 because
there we elide the new X / delete p pair ... which is invalid then?
Hmm, we emit

 MEM[(struct X *)_8] ={v} {CLOBBER};
 operator delete (_8, 8);

so the object contents are undefined _before_ calling delete
even when I do not have a DTOR?  That is, the above,
w/o -fno-lifetime-dse, makes the PTA patch OK for the testcase.


Yes, all classes have a destructor, even if it's trivial, so the 
object's
lifetime definitely ends before the call to operator delete. This 
is less
clear for scalar objects, but treating them similarly would be 
consistent

with
other recent changes, so I think it's fine for us to assume that 
scalar
objects are also invalidated before the call to operator delete.  
But of
course this doesn't apply to explicit calls to operator delete 
outside of a

delete expression.


OK, so change the testcase main slightly to

int main()
{
    int y = 1;
    X *p = new X;
    p->p = 
    ::operator delete(p);
    X *q = new X;
    *(q->p) = 2;
    if (y != 2)
  __builtin_abort ();
}

in this case the lifetime of *p does not end before calling
::operator delete() and delete can stash the object contents
somewhere before ending its lifetime.  For the very same reason
we may not elide a new/delete pair like in

int main()
{
    int *p = new int;
    *p = 1;
    ::operator delete (p);
}


Correct; the permission to elide new/delete pairs are for the 
expressions, not

the functions.


which we before the change did not do only because calling
operator delete made p escape.  Unfortunately points-to analysis
cannot really reconstruct whether delete was called as part of
a delete expression or directly (and thus whether object lifetime
ended already), neither can DCE.  So I guess we need to mark
the operator delete call in some way to make those transforms
safe.  At least currently any operator delete call makes the
alias guarantee of a operator new call moot by forcing the object
to be aliased with all global and escaped memory ...

Looks like there are some unallocated flags for CALL_EXPR we could
pick but I wonder if we can recycle protected_flag which is

 CALL_FROM_THUNK_P and
 CALL_ALLOCA_FOR_VAR_P in
 CALL_EXPR

for calls to DECL_IS_OPERATOR_{NEW,DELETE}_P, thus whether
we have CALL_FROM_THUNK_P for those operators.  Guess picking
a new flag is safer.


We won't ever call those operators from a thunk, so it should be OK 
to reuse

it.


But, does it seem correct that we need to distinguish
delete expressions from plain calls to operator delete?


A reason for that distinction came up in the context of omitting 
new/delete
pairs: we want to consider the operator first called by the new or 
delete

Re: [PATCH] tree-optimization/97151 - improve PTA for C++ operator delete

2020-09-28 Thread Jason Merrill via Gcc-patches

On 9/28/20 3:56 AM, Richard Biener wrote:

On Fri, 25 Sep 2020, Jason Merrill wrote:


On 9/25/20 2:30 AM, Richard Biener wrote:

On Thu, 24 Sep 2020, Jason Merrill wrote:


On 9/24/20 3:43 AM, Richard Biener wrote:

On Wed, 23 Sep 2020, Jason Merrill wrote:


On 9/23/20 2:42 PM, Richard Biener wrote:

On September 23, 2020 7:53:18 PM GMT+02:00, Jason Merrill

wrote:

On 9/23/20 4:14 AM, Richard Biener wrote:

C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
does not cause the deleted object to be escaped.  It also has no
other interesting side-effects for PTA so skip it like we do
for BUILT_IN_FREE.


Hmm, this is true of the default implementation, but since the function

is replaceable, we don't know what a user definition might do with the
pointer.


But can the object still be 'used' after delete? Can delete fail /
throw?

What guarantee does the predicate give us?


The deallocation function is called as part of a delete expression in
order
to
release the storage for an object, ending its lifetime (if it was not
ended
by
a destructor), so no, the object can't be used afterward.


OK, but the delete operator can access the object contents if there
wasn't a destructor ...



A deallocation function that throws has undefined behavior.


OK, so it seems the 'replaceable' operators are the global ones
(for user-defined/class-specific placement variants I see arbitrary
extra arguments that we'd possibly need to handle).

I'm happy to revert but I'd like to have a testcase that FAILs
with the patch ;)

Now, the following aborts:

struct X {
 static struct X saved;
 int *p;
 X() { __builtin_memcpy (this, , sizeof (X)); }
};
void operator delete (void *p)
{
 __builtin_memcpy (::saved, p, sizeof (X));
}
int main()
{
 int y = 1;
 X *p = new X;
 p->p = 
 delete p;
 X *q = new X;
 *(q->p) = 2;
 if (y != 2)
   __builtin_abort ();
}

and I could fix this by not making *p but what *p points to escape.
The testcase is of course maximally awkward, but hey ... ;)

Now this would all be moot if operator delete may not access
the object (or if the object contents are undefined at that point).

Oh, and the testcase segfaults when compiled with GCC 10 because
there we elide the new X / delete p pair ... which is invalid then?
Hmm, we emit

 MEM[(struct X *)_8] ={v} {CLOBBER};
 operator delete (_8, 8);

so the object contents are undefined _before_ calling delete
even when I do not have a DTOR?  That is, the above,
w/o -fno-lifetime-dse, makes the PTA patch OK for the testcase.


Yes, all classes have a destructor, even if it's trivial, so the object's
lifetime definitely ends before the call to operator delete. This is less
clear for scalar objects, but treating them similarly would be consistent
with
other recent changes, so I think it's fine for us to assume that scalar
objects are also invalidated before the call to operator delete.  But of
course this doesn't apply to explicit calls to operator delete outside of a
delete expression.


OK, so change the testcase main slightly to

int main()
{
int y = 1;
X *p = new X;
p->p = 
::operator delete(p);
X *q = new X;
*(q->p) = 2;
if (y != 2)
  __builtin_abort ();
}

in this case the lifetime of *p does not end before calling
::operator delete() and delete can stash the object contents
somewhere before ending its lifetime.  For the very same reason
we may not elide a new/delete pair like in

int main()
{
int *p = new int;
*p = 1;
::operator delete (p);
}


Correct; the permission to elide new/delete pairs are for the expressions, not
the functions.


which we before the change did not do only because calling
operator delete made p escape.  Unfortunately points-to analysis
cannot really reconstruct whether delete was called as part of
a delete expression or directly (and thus whether object lifetime
ended already), neither can DCE.  So I guess we need to mark
the operator delete call in some way to make those transforms
safe.  At least currently any operator delete call makes the
alias guarantee of a operator new call moot by forcing the object
to be aliased with all global and escaped memory ...

Looks like there are some unallocated flags for CALL_EXPR we could
pick but I wonder if we can recycle protected_flag which is

 CALL_FROM_THUNK_P and
 CALL_ALLOCA_FOR_VAR_P in
 CALL_EXPR

for calls to DECL_IS_OPERATOR_{NEW,DELETE}_P, thus whether
we have CALL_FROM_THUNK_P for those operators.  Guess picking
a new flag is safer.


We won't ever call those operators from a thunk, so it should be OK to reuse
it.


But, does it seem correct that we need to distinguish
delete expressions from plain calls to operator delete?


A reason for that distinction came up in the context of omitting new/delete
pairs: we want to consider the operator first called by the new or delete
expression, not a call from that first operator to another operator 

Re: [PATCH] tree-optimization/97151 - improve PTA for C++ operator delete

2020-09-28 Thread Richard Biener
On Fri, 25 Sep 2020, Jason Merrill wrote:

> On 9/25/20 2:30 AM, Richard Biener wrote:
> > On Thu, 24 Sep 2020, Jason Merrill wrote:
> > 
> >> On 9/24/20 3:43 AM, Richard Biener wrote:
> >>> On Wed, 23 Sep 2020, Jason Merrill wrote:
> >>>
>  On 9/23/20 2:42 PM, Richard Biener wrote:
> > On September 23, 2020 7:53:18 PM GMT+02:00, Jason Merrill
> > 
> > wrote:
> >> On 9/23/20 4:14 AM, Richard Biener wrote:
> >>> C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
> >>> does not cause the deleted object to be escaped.  It also has no
> >>> other interesting side-effects for PTA so skip it like we do
> >>> for BUILT_IN_FREE.
> >>
> >> Hmm, this is true of the default implementation, but since the function
> >>
> >> is replaceable, we don't know what a user definition might do with the
> >> pointer.
> >
> > But can the object still be 'used' after delete? Can delete fail /
> > throw?
> >
> > What guarantee does the predicate give us?
> 
>  The deallocation function is called as part of a delete expression in
>  order
>  to
>  release the storage for an object, ending its lifetime (if it was not
>  ended
>  by
>  a destructor), so no, the object can't be used afterward.
> >>>
> >>> OK, but the delete operator can access the object contents if there
> >>> wasn't a destructor ...
> >>
>  A deallocation function that throws has undefined behavior.
> >>>
> >>> OK, so it seems the 'replaceable' operators are the global ones
> >>> (for user-defined/class-specific placement variants I see arbitrary
> >>> extra arguments that we'd possibly need to handle).
> >>>
> >>> I'm happy to revert but I'd like to have a testcase that FAILs
> >>> with the patch ;)
> >>>
> >>> Now, the following aborts:
> >>>
> >>> struct X {
> >>> static struct X saved;
> >>> int *p;
> >>> X() { __builtin_memcpy (this, , sizeof (X)); }
> >>> };
> >>> void operator delete (void *p)
> >>> {
> >>> __builtin_memcpy (::saved, p, sizeof (X));
> >>> }
> >>> int main()
> >>> {
> >>> int y = 1;
> >>> X *p = new X;
> >>> p->p = 
> >>> delete p;
> >>> X *q = new X;
> >>> *(q->p) = 2;
> >>> if (y != 2)
> >>>   __builtin_abort ();
> >>> }
> >>>
> >>> and I could fix this by not making *p but what *p points to escape.
> >>> The testcase is of course maximally awkward, but hey ... ;)
> >>>
> >>> Now this would all be moot if operator delete may not access
> >>> the object (or if the object contents are undefined at that point).
> >>>
> >>> Oh, and the testcase segfaults when compiled with GCC 10 because
> >>> there we elide the new X / delete p pair ... which is invalid then?
> >>> Hmm, we emit
> >>>
> >>> MEM[(struct X *)_8] ={v} {CLOBBER};
> >>> operator delete (_8, 8);
> >>>
> >>> so the object contents are undefined _before_ calling delete
> >>> even when I do not have a DTOR?  That is, the above,
> >>> w/o -fno-lifetime-dse, makes the PTA patch OK for the testcase.
> >>
> >> Yes, all classes have a destructor, even if it's trivial, so the object's
> >> lifetime definitely ends before the call to operator delete. This is less
> >> clear for scalar objects, but treating them similarly would be consistent
> >> with
> >> other recent changes, so I think it's fine for us to assume that scalar
> >> objects are also invalidated before the call to operator delete.  But of
> >> course this doesn't apply to explicit calls to operator delete outside of a
> >> delete expression.
> > 
> > OK, so change the testcase main slightly to
> > 
> > int main()
> > {
> >int y = 1;
> >X *p = new X;
> >p->p = 
> >::operator delete(p);
> >X *q = new X;
> >*(q->p) = 2;
> >if (y != 2)
> >  __builtin_abort ();
> > }
> > 
> > in this case the lifetime of *p does not end before calling
> > ::operator delete() and delete can stash the object contents
> > somewhere before ending its lifetime.  For the very same reason
> > we may not elide a new/delete pair like in
> > 
> > int main()
> > {
> >int *p = new int;
> >*p = 1;
> >::operator delete (p);
> > }
> 
> Correct; the permission to elide new/delete pairs are for the expressions, not
> the functions.
> 
> > which we before the change did not do only because calling
> > operator delete made p escape.  Unfortunately points-to analysis
> > cannot really reconstruct whether delete was called as part of
> > a delete expression or directly (and thus whether object lifetime
> > ended already), neither can DCE.  So I guess we need to mark
> > the operator delete call in some way to make those transforms
> > safe.  At least currently any operator delete call makes the
> > alias guarantee of a operator new call moot by forcing the object
> > to be aliased with all global and escaped memory ...
> > 
> > Looks like there are some unallocated flags for CALL_EXPR we could
> > pick but I wonder if we can recycle protected_flag 

Re: [PATCH] tree-optimization/97151 - improve PTA for C++ operator delete

2020-09-25 Thread Jason Merrill via Gcc-patches

On 9/25/20 2:30 AM, Richard Biener wrote:

On Thu, 24 Sep 2020, Jason Merrill wrote:


On 9/24/20 3:43 AM, Richard Biener wrote:

On Wed, 23 Sep 2020, Jason Merrill wrote:


On 9/23/20 2:42 PM, Richard Biener wrote:

On September 23, 2020 7:53:18 PM GMT+02:00, Jason Merrill

wrote:

On 9/23/20 4:14 AM, Richard Biener wrote:

C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
does not cause the deleted object to be escaped.  It also has no
other interesting side-effects for PTA so skip it like we do
for BUILT_IN_FREE.


Hmm, this is true of the default implementation, but since the function

is replaceable, we don't know what a user definition might do with the
pointer.


But can the object still be 'used' after delete? Can delete fail / throw?

What guarantee does the predicate give us?


The deallocation function is called as part of a delete expression in order
to
release the storage for an object, ending its lifetime (if it was not ended
by
a destructor), so no, the object can't be used afterward.


OK, but the delete operator can access the object contents if there
wasn't a destructor ...



A deallocation function that throws has undefined behavior.


OK, so it seems the 'replaceable' operators are the global ones
(for user-defined/class-specific placement variants I see arbitrary
extra arguments that we'd possibly need to handle).

I'm happy to revert but I'd like to have a testcase that FAILs
with the patch ;)

Now, the following aborts:

struct X {
static struct X saved;
int *p;
X() { __builtin_memcpy (this, , sizeof (X)); }
};
void operator delete (void *p)
{
__builtin_memcpy (::saved, p, sizeof (X));
}
int main()
{
int y = 1;
X *p = new X;
p->p = 
delete p;
X *q = new X;
*(q->p) = 2;
if (y != 2)
  __builtin_abort ();
}

and I could fix this by not making *p but what *p points to escape.
The testcase is of course maximally awkward, but hey ... ;)

Now this would all be moot if operator delete may not access
the object (or if the object contents are undefined at that point).

Oh, and the testcase segfaults when compiled with GCC 10 because
there we elide the new X / delete p pair ... which is invalid then?
Hmm, we emit

MEM[(struct X *)_8] ={v} {CLOBBER};
operator delete (_8, 8);

so the object contents are undefined _before_ calling delete
even when I do not have a DTOR?  That is, the above,
w/o -fno-lifetime-dse, makes the PTA patch OK for the testcase.


Yes, all classes have a destructor, even if it's trivial, so the object's
lifetime definitely ends before the call to operator delete. This is less
clear for scalar objects, but treating them similarly would be consistent with
other recent changes, so I think it's fine for us to assume that scalar
objects are also invalidated before the call to operator delete.  But of
course this doesn't apply to explicit calls to operator delete outside of a
delete expression.


OK, so change the testcase main slightly to

int main()
{
   int y = 1;
   X *p = new X;
   p->p = 
   ::operator delete(p);
   X *q = new X;
   *(q->p) = 2;
   if (y != 2)
 __builtin_abort ();
}

in this case the lifetime of *p does not end before calling
::operator delete() and delete can stash the object contents
somewhere before ending its lifetime.  For the very same reason
we may not elide a new/delete pair like in

int main()
{
   int *p = new int;
   *p = 1;
   ::operator delete (p);
}


Correct; the permission to elide new/delete pairs are for the 
expressions, not the functions.



which we before the change did not do only because calling
operator delete made p escape.  Unfortunately points-to analysis
cannot really reconstruct whether delete was called as part of
a delete expression or directly (and thus whether object lifetime
ended already), neither can DCE.  So I guess we need to mark
the operator delete call in some way to make those transforms
safe.  At least currently any operator delete call makes the
alias guarantee of a operator new call moot by forcing the object
to be aliased with all global and escaped memory ...

Looks like there are some unallocated flags for CALL_EXPR we could
pick but I wonder if we can recycle protected_flag which is

CALL_FROM_THUNK_P and
CALL_ALLOCA_FOR_VAR_P in
CALL_EXPR

for calls to DECL_IS_OPERATOR_{NEW,DELETE}_P, thus whether
we have CALL_FROM_THUNK_P for those operators.  Guess picking
a new flag is safer.


We won't ever call those operators from a thunk, so it should be OK to 
reuse it.



But, does it seem correct that we need to distinguish
delete expressions from plain calls to operator delete?


A reason for that distinction came up in the context of omitting 
new/delete pairs: we want to consider the operator first called by the 
new or delete expression, not a call from that first operator to another 
operator new/delete and exposed by inlining.


https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543404.html


In this 

Re: [PATCH] tree-optimization/97151 - improve PTA for C++ operator delete

2020-09-25 Thread Richard Biener
On Thu, 24 Sep 2020, Jason Merrill wrote:

> On 9/24/20 3:43 AM, Richard Biener wrote:
> > On Wed, 23 Sep 2020, Jason Merrill wrote:
> > 
> >> On 9/23/20 2:42 PM, Richard Biener wrote:
> >>> On September 23, 2020 7:53:18 PM GMT+02:00, Jason Merrill
> >>> 
> >>> wrote:
>  On 9/23/20 4:14 AM, Richard Biener wrote:
> > C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
> > does not cause the deleted object to be escaped.  It also has no
> > other interesting side-effects for PTA so skip it like we do
> > for BUILT_IN_FREE.
> 
>  Hmm, this is true of the default implementation, but since the function
> 
>  is replaceable, we don't know what a user definition might do with the
>  pointer.
> >>>
> >>> But can the object still be 'used' after delete? Can delete fail / throw?
> >>>
> >>> What guarantee does the predicate give us?
> >>
> >> The deallocation function is called as part of a delete expression in order
> >> to
> >> release the storage for an object, ending its lifetime (if it was not ended
> >> by
> >> a destructor), so no, the object can't be used afterward.
> > 
> > OK, but the delete operator can access the object contents if there
> > wasn't a destructor ...
> 
> >> A deallocation function that throws has undefined behavior.
> > 
> > OK, so it seems the 'replaceable' operators are the global ones
> > (for user-defined/class-specific placement variants I see arbitrary
> > extra arguments that we'd possibly need to handle).
> > 
> > I'm happy to revert but I'd like to have a testcase that FAILs
> > with the patch ;)
> > 
> > Now, the following aborts:
> > 
> > struct X {
> >static struct X saved;
> >int *p;
> >X() { __builtin_memcpy (this, , sizeof (X)); }
> > };
> > void operator delete (void *p)
> > {
> >__builtin_memcpy (::saved, p, sizeof (X));
> > }
> > int main()
> > {
> >int y = 1;
> >X *p = new X;
> >p->p = 
> >delete p;
> >X *q = new X;
> >*(q->p) = 2;
> >if (y != 2)
> >  __builtin_abort ();
> > }
> > 
> > and I could fix this by not making *p but what *p points to escape.
> > The testcase is of course maximally awkward, but hey ... ;)
> > 
> > Now this would all be moot if operator delete may not access
> > the object (or if the object contents are undefined at that point).
> > 
> > Oh, and the testcase segfaults when compiled with GCC 10 because
> > there we elide the new X / delete p pair ... which is invalid then?
> > Hmm, we emit
> > 
> >MEM[(struct X *)_8] ={v} {CLOBBER};
> >operator delete (_8, 8);
> > 
> > so the object contents are undefined _before_ calling delete
> > even when I do not have a DTOR?  That is, the above,
> > w/o -fno-lifetime-dse, makes the PTA patch OK for the testcase.
> 
> Yes, all classes have a destructor, even if it's trivial, so the object's
> lifetime definitely ends before the call to operator delete. This is less
> clear for scalar objects, but treating them similarly would be consistent with
> other recent changes, so I think it's fine for us to assume that scalar
> objects are also invalidated before the call to operator delete.  But of
> course this doesn't apply to explicit calls to operator delete outside of a
> delete expression.

OK, so change the testcase main slightly to

int main()
{
  int y = 1;
  X *p = new X;
  p->p = 
  ::operator delete(p);
  X *q = new X;
  *(q->p) = 2;
  if (y != 2)
__builtin_abort ();
}

in this case the lifetime of *p does not end before calling
::operator delete() and delete can stash the object contents
somewhere before ending its lifetime.  For the very same reason
we may not elide a new/delete pair like in

int main()
{
  int *p = new int;
  *p = 1;
  ::operator delete (p);
}

which we before the change did not do only because calling
operator delete made p escape.  Unfortunately points-to analysis
cannot really reconstruct whether delete was called as part of
a delete expression or directly (and thus whether object lifetime
ended already), neither can DCE.  So I guess we need to mark
the operator delete call in some way to make those transforms
safe.  At least currently any operator delete call makes the
alias guarantee of a operator new call moot by forcing the object
to be aliased with all global and escaped memory ...

Looks like there are some unallocated flags for CALL_EXPR we could
pick but I wonder if we can recycle protected_flag which is

   CALL_FROM_THUNK_P and
   CALL_ALLOCA_FOR_VAR_P in
   CALL_EXPR

for calls to DECL_IS_OPERATOR_{NEW,DELETE}_P, thus whether
we have CALL_FROM_THUNK_P for those operators.  Guess picking
a new flag is safer.

But, does it seem correct that we need to distinguish
delete expressions from plain calls to operator delete?
In this context I also wonder about non-replaceable operator delete,
specifically operator delete in classes - are there any semantic
differences between those or why did we choose to only mark
the replaceable ones?

Thanks,

Re: [PATCH] tree-optimization/97151 - improve PTA for C++ operator delete

2020-09-24 Thread Jason Merrill via Gcc-patches

On 9/24/20 3:43 AM, Richard Biener wrote:

On Wed, 23 Sep 2020, Jason Merrill wrote:


On 9/23/20 2:42 PM, Richard Biener wrote:

On September 23, 2020 7:53:18 PM GMT+02:00, Jason Merrill 
wrote:

On 9/23/20 4:14 AM, Richard Biener wrote:

C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
does not cause the deleted object to be escaped.  It also has no
other interesting side-effects for PTA so skip it like we do
for BUILT_IN_FREE.


Hmm, this is true of the default implementation, but since the function

is replaceable, we don't know what a user definition might do with the
pointer.


But can the object still be 'used' after delete? Can delete fail / throw?

What guarantee does the predicate give us?


The deallocation function is called as part of a delete expression in order to
release the storage for an object, ending its lifetime (if it was not ended by
a destructor), so no, the object can't be used afterward.


OK, but the delete operator can access the object contents if there
wasn't a destructor ...



A deallocation function that throws has undefined behavior.


OK, so it seems the 'replaceable' operators are the global ones
(for user-defined/class-specific placement variants I see arbitrary
extra arguments that we'd possibly need to handle).

I'm happy to revert but I'd like to have a testcase that FAILs
with the patch ;)

Now, the following aborts:

struct X {
   static struct X saved;
   int *p;
   X() { __builtin_memcpy (this, , sizeof (X)); }
};
void operator delete (void *p)
{
   __builtin_memcpy (::saved, p, sizeof (X));
}
int main()
{
   int y = 1;
   X *p = new X;
   p->p = 
   delete p;
   X *q = new X;
   *(q->p) = 2;
   if (y != 2)
 __builtin_abort ();
}

and I could fix this by not making *p but what *p points to escape.
The testcase is of course maximally awkward, but hey ... ;)

Now this would all be moot if operator delete may not access
the object (or if the object contents are undefined at that point).

Oh, and the testcase segfaults when compiled with GCC 10 because
there we elide the new X / delete p pair ... which is invalid then?
Hmm, we emit

   MEM[(struct X *)_8] ={v} {CLOBBER};
   operator delete (_8, 8);

so the object contents are undefined _before_ calling delete
even when I do not have a DTOR?  That is, the above,
w/o -fno-lifetime-dse, makes the PTA patch OK for the testcase.


Yes, all classes have a destructor, even if it's trivial, so the 
object's lifetime definitely ends before the call to operator delete. 
This is less clear for scalar objects, but treating them similarly would 
be consistent with other recent changes, so I think it's fine for us to 
assume that scalar objects are also invalidated before the call to 
operator delete.  But of course this doesn't apply to explicit calls to 
operator delete outside of a delete expression.



Richard.


Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

Richard.

2020-09-23  Richard Biener  

  PR tree-optimization/97151
  * tree-ssa-structalias.c (find_func_aliases_for_call):
  DECL_IS_REPLACEABLE_OPERATOR_DELETE_P has no effect on
  arguments.

* g++.dg/cpp1y/new1.C: Adjust for two more handled transforms.
---
gcc/testsuite/g++.dg/cpp1y/new1.C | 4 ++--
gcc/tree-ssa-structalias.c| 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/g++.dg/cpp1y/new1.C

b/gcc/testsuite/g++.dg/cpp1y/new1.C

index aa5f647d535..fec0088cb40 100644
--- a/gcc/testsuite/g++.dg/cpp1y/new1.C
+++ b/gcc/testsuite/g++.dg/cpp1y/new1.C
@@ -69,5 +69,5 @@ test_unused() {
  delete p;
}

-/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 5

"cddce1"} } */

-/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator

new" 7 "cddce1"} } */

+/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 6

"cddce1"} } */

+/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator

new" 8 "cddce1"} } */

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, );
+  else if (fndecl && DECL_IS_REPLACEABLE_OPERATOR_DELETE_P

(fndecl))

+   ;
   else
 handle_rhs_call (t, );
  if (gimple_call_lhs (t))












Re: [PATCH] tree-optimization/97151 - improve PTA for C++ operator delete

2020-09-24 Thread Richard Biener
On Wed, 23 Sep 2020, Jason Merrill wrote:

> On 9/23/20 2:42 PM, Richard Biener wrote:
> > On September 23, 2020 7:53:18 PM GMT+02:00, Jason Merrill 
> > wrote:
> >> On 9/23/20 4:14 AM, Richard Biener wrote:
> >>> C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
> >>> does not cause the deleted object to be escaped.  It also has no
> >>> other interesting side-effects for PTA so skip it like we do
> >>> for BUILT_IN_FREE.
> >>
> >> Hmm, this is true of the default implementation, but since the function
> >>
> >> is replaceable, we don't know what a user definition might do with the
> >> pointer.
> > 
> > But can the object still be 'used' after delete? Can delete fail / throw?
> > 
> > What guarantee does the predicate give us?
> 
> The deallocation function is called as part of a delete expression in order to
> release the storage for an object, ending its lifetime (if it was not ended by
> a destructor), so no, the object can't be used afterward.

OK, but the delete operator can access the object contents if there
wasn't a destructor ...

> A deallocation function that throws has undefined behavior.

OK, so it seems the 'replaceable' operators are the global ones
(for user-defined/class-specific placement variants I see arbitrary
extra arguments that we'd possibly need to handle).

I'm happy to revert but I'd like to have a testcase that FAILs
with the patch ;)

Now, the following aborts:

struct X {
  static struct X saved;
  int *p;
  X() { __builtin_memcpy (this, , sizeof (X)); }
};
void operator delete (void *p)
{
  __builtin_memcpy (::saved, p, sizeof (X));
}
int main()
{
  int y = 1;
  X *p = new X;
  p->p = 
  delete p;
  X *q = new X;
  *(q->p) = 2;
  if (y != 2)
__builtin_abort ();
}

and I could fix this by not making *p but what *p points to escape.
The testcase is of course maximally awkward, but hey ... ;)

Now this would all be moot if operator delete may not access
the object (or if the object contents are undefined at that point).

Oh, and the testcase segfaults when compiled with GCC 10 because
there we elide the new X / delete p pair ... which is invalid then?
Hmm, we emit

  MEM[(struct X *)_8] ={v} {CLOBBER};
  operator delete (_8, 8);

so the object contents are undefined _before_ calling delete
even when I do not have a DTOR?  That is, the above,
w/o -fno-lifetime-dse, makes the PTA patch OK for the testcase.

Richard.

> >>> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
> >>>
> >>> Richard.
> >>>
> >>> 2020-09-23  Richard Biener  
> >>>
> >>>  PR tree-optimization/97151
> >>>  * tree-ssa-structalias.c (find_func_aliases_for_call):
> >>>  DECL_IS_REPLACEABLE_OPERATOR_DELETE_P has no effect on
> >>>  arguments.
> >>>
> >>>   * g++.dg/cpp1y/new1.C: Adjust for two more handled transforms.
> >>> ---
> >>>gcc/testsuite/g++.dg/cpp1y/new1.C | 4 ++--
> >>>gcc/tree-ssa-structalias.c| 2 ++
> >>>2 files changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/gcc/testsuite/g++.dg/cpp1y/new1.C
> >> b/gcc/testsuite/g++.dg/cpp1y/new1.C
> >>> index aa5f647d535..fec0088cb40 100644
> >>> --- a/gcc/testsuite/g++.dg/cpp1y/new1.C
> >>> +++ b/gcc/testsuite/g++.dg/cpp1y/new1.C
> >>> @@ -69,5 +69,5 @@ test_unused() {
> >>>  delete p;
> >>>}
> >>>
> >>> -/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 5
> >> "cddce1"} } */
> >>> -/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator
> >> new" 7 "cddce1"} } */
> >>> +/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 6
> >> "cddce1"} } */
> >>> +/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator
> >> new" 8 "cddce1"} } */
> >>> 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, );
> >>> +  else if (fndecl && DECL_IS_REPLACEABLE_OPERATOR_DELETE_P
> >> (fndecl))
> >>> + ;
> >>>   else
> >>> handle_rhs_call (t, );
> >>>  if (gimple_call_lhs (t))
> >>>
> > 
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend


Re: [PATCH] tree-optimization/97151 - improve PTA for C++ operator delete

2020-09-23 Thread Jason Merrill via Gcc-patches

On 9/23/20 2:42 PM, Richard Biener wrote:

On September 23, 2020 7:53:18 PM GMT+02:00, Jason Merrill  
wrote:

On 9/23/20 4:14 AM, Richard Biener wrote:

C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
does not cause the deleted object to be escaped.  It also has no
other interesting side-effects for PTA so skip it like we do
for BUILT_IN_FREE.


Hmm, this is true of the default implementation, but since the function

is replaceable, we don't know what a user definition might do with the
pointer.


But can the object still be 'used' after delete? Can delete fail / throw?

What guarantee does the predicate give us?


The deallocation function is called as part of a delete expression in 
order to release the storage for an object, ending its lifetime (if it 
was not ended by a destructor), so no, the object can't be used afterward.


A deallocation function that throws has undefined behavior.


Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

Richard.

2020-09-23  Richard Biener  

PR tree-optimization/97151
* tree-ssa-structalias.c (find_func_aliases_for_call):
DECL_IS_REPLACEABLE_OPERATOR_DELETE_P has no effect on
arguments.

* g++.dg/cpp1y/new1.C: Adjust for two more handled transforms.
---
   gcc/testsuite/g++.dg/cpp1y/new1.C | 4 ++--
   gcc/tree-ssa-structalias.c| 2 ++
   2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/g++.dg/cpp1y/new1.C

b/gcc/testsuite/g++.dg/cpp1y/new1.C

index aa5f647d535..fec0088cb40 100644
--- a/gcc/testsuite/g++.dg/cpp1y/new1.C
+++ b/gcc/testsuite/g++.dg/cpp1y/new1.C
@@ -69,5 +69,5 @@ test_unused() {
 delete p;
   }
   
-/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 5

"cddce1"} } */

-/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator

new" 7 "cddce1"} } */

+/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 6

"cddce1"} } */

+/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator

new" 8 "cddce1"} } */

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, );
+  else if (fndecl && DECL_IS_REPLACEABLE_OPERATOR_DELETE_P

(fndecl))

+   ;
 else
handle_rhs_call (t, );
 if (gimple_call_lhs (t))







Re: [PATCH] tree-optimization/97151 - improve PTA for C++ operator delete

2020-09-23 Thread Richard Biener
On September 23, 2020 7:53:18 PM GMT+02:00, Jason Merrill  
wrote:
>On 9/23/20 4:14 AM, Richard Biener wrote:
>> C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
>> does not cause the deleted object to be escaped.  It also has no
>> other interesting side-effects for PTA so skip it like we do
>> for BUILT_IN_FREE.
>
>Hmm, this is true of the default implementation, but since the function
>
>is replaceable, we don't know what a user definition might do with the 
>pointer.

But can the object still be 'used' after delete? Can delete fail / throw?

What guarantee does the predicate give us? 

>> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
>> 
>> Richard.
>> 
>> 2020-09-23  Richard Biener  
>> 
>>  PR tree-optimization/97151
>>  * tree-ssa-structalias.c (find_func_aliases_for_call):
>>  DECL_IS_REPLACEABLE_OPERATOR_DELETE_P has no effect on
>>  arguments.
>> 
>>  * g++.dg/cpp1y/new1.C: Adjust for two more handled transforms.
>> ---
>>   gcc/testsuite/g++.dg/cpp1y/new1.C | 4 ++--
>>   gcc/tree-ssa-structalias.c| 2 ++
>>   2 files changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/gcc/testsuite/g++.dg/cpp1y/new1.C
>b/gcc/testsuite/g++.dg/cpp1y/new1.C
>> index aa5f647d535..fec0088cb40 100644
>> --- a/gcc/testsuite/g++.dg/cpp1y/new1.C
>> +++ b/gcc/testsuite/g++.dg/cpp1y/new1.C
>> @@ -69,5 +69,5 @@ test_unused() {
>> delete p;
>>   }
>>   
>> -/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 5
>"cddce1"} } */
>> -/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator
>new" 7 "cddce1"} } */
>> +/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 6
>"cddce1"} } */
>> +/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator
>new" 8 "cddce1"} } */
>> 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, );
>> +  else if (fndecl && DECL_IS_REPLACEABLE_OPERATOR_DELETE_P
>(fndecl))
>> +;
>> else
>>  handle_rhs_call (t, );
>> if (gimple_call_lhs (t))
>> 



Re: [PATCH] tree-optimization/97151 - improve PTA for C++ operator delete

2020-09-23 Thread Jason Merrill via Gcc-patches

On 9/23/20 4:14 AM, Richard Biener wrote:

C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
does not cause the deleted object to be escaped.  It also has no
other interesting side-effects for PTA so skip it like we do
for BUILT_IN_FREE.


Hmm, this is true of the default implementation, but since the function 
is replaceable, we don't know what a user definition might do with the 
pointer.



Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

Richard.

2020-09-23  Richard Biener  

PR tree-optimization/97151
* tree-ssa-structalias.c (find_func_aliases_for_call):
DECL_IS_REPLACEABLE_OPERATOR_DELETE_P has no effect on
arguments.

* g++.dg/cpp1y/new1.C: Adjust for two more handled transforms.
---
  gcc/testsuite/g++.dg/cpp1y/new1.C | 4 ++--
  gcc/tree-ssa-structalias.c| 2 ++
  2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/g++.dg/cpp1y/new1.C 
b/gcc/testsuite/g++.dg/cpp1y/new1.C
index aa5f647d535..fec0088cb40 100644
--- a/gcc/testsuite/g++.dg/cpp1y/new1.C
+++ b/gcc/testsuite/g++.dg/cpp1y/new1.C
@@ -69,5 +69,5 @@ test_unused() {
delete p;
  }
  
-/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 5 "cddce1"} } */

-/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator new" 7 
"cddce1"} } */
+/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 6 "cddce1"} 
} */
+/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator new" 8 
"cddce1"} } */
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, );
+  else if (fndecl && DECL_IS_REPLACEABLE_OPERATOR_DELETE_P (fndecl))
+   ;
else
handle_rhs_call (t, );
if (gimple_call_lhs (t))





[PATCH] tree-optimization/97151 - improve PTA for C++ operator delete

2020-09-23 Thread Richard Biener
C++ operator delete, when DECL_IS_REPLACEABLE_OPERATOR_DELETE_P,
does not cause the deleted object to be escaped.  It also has no
other interesting side-effects for PTA so skip it like we do
for BUILT_IN_FREE.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

Richard.

2020-09-23  Richard Biener  

PR tree-optimization/97151
* tree-ssa-structalias.c (find_func_aliases_for_call):
DECL_IS_REPLACEABLE_OPERATOR_DELETE_P has no effect on
arguments.

* g++.dg/cpp1y/new1.C: Adjust for two more handled transforms.
---
 gcc/testsuite/g++.dg/cpp1y/new1.C | 4 ++--
 gcc/tree-ssa-structalias.c| 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/g++.dg/cpp1y/new1.C 
b/gcc/testsuite/g++.dg/cpp1y/new1.C
index aa5f647d535..fec0088cb40 100644
--- a/gcc/testsuite/g++.dg/cpp1y/new1.C
+++ b/gcc/testsuite/g++.dg/cpp1y/new1.C
@@ -69,5 +69,5 @@ test_unused() {
   delete p;
 }
 
-/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 5 "cddce1"} 
} */
-/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator new" 7 
"cddce1"} } */
+/* { dg-final { scan-tree-dump-times "Deleting : operator delete" 6 "cddce1"} 
} */
+/* { dg-final { scan-tree-dump-times "Deleting : _\\d+ = operator new" 8 
"cddce1"} } */
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, );
+  else if (fndecl && DECL_IS_REPLACEABLE_OPERATOR_DELETE_P (fndecl))
+   ;
   else
handle_rhs_call (t, );
   if (gimple_call_lhs (t))
-- 
2.26.2