public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Martin Liška" <mliska@suse.cz>
To: Nathan Sidwell <nathan@acm.org>, Jan Hubicka <hubicka@ucw.cz>
Cc: Richard Biener <richard.guenther@gmail.com>,
	GCC Patches <gcc-patches@gcc.gnu.org>,
	Marc Glisse <marc.glisse@inria.fr>,
	Jason Merrill <jason@redhat.com>,
	Jonathan Wakely <jwakely.gcc@gmail.com>
Subject: Re: [PATCH] Check DECL_CONTEXT of new/delete operators.
Date: Tue, 7 Apr 2020 12:46:49 +0200	[thread overview]
Message-ID: <3e0769d3-df87-4c4e-e435-9ceb59faf475@suse.cz> (raw)
In-Reply-To: <e856b407-df7e-4459-c1d7-2665810996fe@acm.org>

On 4/6/20 2:45 PM, Nathan Sidwell wrote:
> On 4/6/20 4:34 AM, Martin Liška wrote:
> 
>>
>> May I please ping Jason, Nathan and Jonathan who can help us here?
> 
> On IRC Martin clarified the question as essentially 'how do you pair up operator new and operator delete calls?' (so you may delete them if the object is not used).
> 
> I am not sure you're permitted to remove those calls in general.  All I can find is [expr.new]/12
> 'An implementation is allowed to omit a call to a replaceable global allocation function (17.6.2.1, 17.6.2.2). When it does so, the storage is instead provided by the implementation or provided by extending the allocation of another new-expression.'
> 
> But, I suspect the optimization is safe, in that either no one counts objects by their allocation, or if they do, they don't actually care that the std-conforming number of allocations happen.
> 
> The both operator new and operator delete are looked up in the same manner.  The std does not require a 'matching pair' be found.  but it is extremely poor form for a class to declare exactly one of operator {new,delete}.

Hi.

Thank you for the information.

> 
> The following is well formed:
> 
> struct PoorForm {
>    void *operator new (size_t s) {count++; return ::operator new (s)};
>    static unsigned count;
> };
> 
> Have you considered throwing ctors?
> 
> struct Obj {
>    Obj (); // might throw
> };
> 
> 'obj = new Obj (); ... delete obj;' sequence expand to something like ...
> 
> // obj = new Obj ();
> void *p = ::operator new (sizeof (Obj));
> try {
>    Obj::ctor(p);
> }
> catch (...) // cleanup code
> {
>    ::operator delete (p); // #1
>    throw;
> }
> 
> obj = (Obj*)p;
> 
> .... user code
> 
> // delete obj;
> Obj::dtor (obj);
> ::operator delete (obj); // #2
> 
> calls 1 & 2 matchup to the operator new call

Looking at the throwing ctors:

$ cat new-delete2.C
#include <stdio.h>

struct A
{
   __attribute__((always_inline)) A(int x)
   {
     if (x == 123)
       throw x;
   }
};

int main(int argc, char **argv) {
   A *a = new A (argc);
   delete a;

   return 0;
}

$ g++-9 new-delete2.C -O2 -c -fdump-tree-optimized=/dev/stdout
...
   <bb 2> [local count: 1073741824]:
   _3 = operator new (1);
   if (argc_4(D) == 123)
     goto <bb 3>; [0.00%]
   else
     goto <bb 4>; [100.00%]

   <bb 3> [count: 0]:
   _8 = __cxa_allocate_exception (4);
   MEM[(int *)_8] = 123;
   __cxa_throw (_8, &_ZTIi, 0B);

   <bb 4> [local count: 1073741824]:
   operator delete (_3, 1);
   return 0;
...

As seen cddce can still optimize out
   _3 = operator new (1);
...
   operator delete (_3, 1);

Martin

> 
> Notice that para I quoted allows one to coalesce allocations using the global operator new/deletes.  The rules are pretty much as you can guess -- one lifetime must be entirely within the other.  If inner one's ctor throws, the exception path must destroy the outer.
> 
> does that help?
> 
> nathan
> 


  parent reply	other threads:[~2020-04-07 10:46 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-30  8:40 Martin Liška
2020-03-30  8:53 ` Richard Biener
2020-03-31 12:29   ` Jan Hubicka
2020-03-31 12:38     ` Martin Liška
2020-04-03 15:26       ` Jan Hubicka
2020-04-03 15:42         ` Jan Hubicka
2020-04-04 11:53           ` Jan Hubicka
2020-04-06  9:27             ` Richard Biener
2020-04-06 15:10               ` Jason Merrill
2020-04-06  8:34         ` Martin Liška
2020-04-06 12:45           ` Nathan Sidwell
2020-04-07  8:26             ` Jonathan Wakely
2020-04-07  9:29               ` Richard Biener
2020-04-07  9:49                 ` Jan Hubicka
2020-04-07 10:22                   ` Richard Biener
2020-04-07 10:42                     ` Martin Liška
2020-04-07 11:41                 ` Jonathan Wakely
2020-04-07 10:46             ` Martin Liška [this message]
2020-04-07 11:29             ` Jonathan Wakely
2020-04-07 11:40               ` Richard Biener
2020-04-07 11:46                 ` Jonathan Wakely
2020-04-07 11:57                   ` Richard Biener
2020-04-07 15:00                     ` [PATCH] Allow new/delete operator deletion only for replaceable Martin Liška
2020-04-08  8:47                       ` Richard Biener
2020-04-08 13:20                         ` Jason Merrill
2020-04-08 13:32                           ` Jakub Jelinek
2020-04-08 13:34                             ` Jason Merrill
2020-04-08 15:16                               ` Martin Liška
2020-04-08 15:46                                 ` Jan Hubicka
2020-04-08 16:06                                   ` Jakub Jelinek
2020-04-09  5:05                                 ` Martin Liška
2020-04-09  6:45                                   ` Richard Biener
2020-04-09  6:59                                     ` Martin Liška
2020-04-09  7:21                                       ` Richard Biener
2020-04-09  7:55                                       ` Jakub Jelinek
2020-04-09  8:04                                     ` Marc Glisse
2020-04-09  8:13                                       ` Jonathan Wakely
2020-04-10  8:08                                         ` Martin Liška
2020-04-10  8:18                                           ` Jonathan Wakely
2020-04-10  8:29                                             ` Martin Liška
2020-04-10  9:17                                               ` Jakub Jelinek
2020-04-14  7:09                                                 ` Martin Liška
2020-04-14  7:11                                                   ` Martin Liška
2020-04-14  8:37                                                     ` Jakub Jelinek
2020-04-14 10:54                                                       ` Martin Liška
2020-04-17  7:05                                                         ` Jakub Jelinek
2020-04-17  8:12                                                           ` Jonathan Wakely
2020-04-10  8:37                                           ` Marc Glisse
2020-04-10  9:11                                             ` Iain Sandoe
2020-04-09 16:55                                   ` Jason Merrill
2020-04-07 15:16                     ` [PATCH] Check DECL_CONTEXT of new/delete operators Jonathan Wakely
2020-04-08  7:34                       ` Richard Biener
2020-04-08  8:11                         ` Jonathan Wakely
2020-04-07 14:11               ` Nathan Sidwell
2020-03-30  9:29 ` Marc Glisse

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3e0769d3-df87-4c4e-e435-9ceb59faf475@suse.cz \
    --to=mliska@suse.cz \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=jason@redhat.com \
    --cc=jwakely.gcc@gmail.com \
    --cc=marc.glisse@inria.fr \
    --cc=nathan@acm.org \
    --cc=richard.guenther@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).