Re: [v3] Slightly improve operator new
Ping. On Tue, 15 Apr 2014, Marc Glisse wrote: Ping http://gcc.gnu.org/ml/gcc-patches/2014-03/msg00051.html On Sun, 2 Mar 2014, Marc Glisse wrote: Hello, inlining operator new (with LTO or otherwise), I noticed that it has a complicated implementation, which makes it hard to use this inlined code for optimizations. This patch does two things: 1) there are 2 calls to malloc, I am turning them into just one. At -Os, it does not change the generated code (RTL optimizers manage to merge the calls to malloc). At other levels (-O2, -O3, and especially with -g) it gives a smaller object file. And with just one malloc, some optimizations become much easier (see my recent calloc patch for instance). 2) malloc is predicted to return null 19 times out of 20 because of the loop (that didn't change with the patch), so I am adding __builtin_expect to let gcc optimize the fast path. Further discussion: a) I didn't add __builtin_expect for the test (sz == 0), it didn't change the generated code in my limited test. I was wondering if this test is necessary (new doesn't seem to ever call operator new(0)) or could be moved to operator new[] (new type[0] does call operator new[](0)), but since one can call operator new directly, it has to be protected indeed, so let's forget this point ;-) (too bad malloc is replacable, so we can't use the fact that glibc already does the right thing) b) I have a bit of trouble parsing the standard. Is the nothrow operator new supposed to call the regular operator new? In particular, if a user replaces only the throwing operator new, should the nothrow operator new automatically call that function? That's not what we are currently doing (and it would be a perf regression). Required behavior: Return a non-null pointer to suitably aligned storage (3.7.4), or else return a null pointer. This nothrow version of operator new returns a pointer obtained as if acquired from the (possibly replaced) ordinary version. This requirement is binding on a replacement version of this function. Default behavior: Calls operator new(size). If the call returns normally, returns the result of that call. Otherwise, returns a null pointer. Passes bootstrap+testsuite on x86_64-linux-gnu. Stage 1? 2014-03-03 Marc Glisse marc.gli...@inria.fr * libsupc++/new_op.cc: Factor the calls to malloc, use __builtin_expect. * libsupc++/new_opnt.cc: Likewise. -- Marc Glisse
Re: [v3] Slightly improve operator new
On 17/05/14 18:30 +0200, Marc Glisse wrote: Ping. On Tue, 15 Apr 2014, Marc Glisse wrote: Ping http://gcc.gnu.org/ml/gcc-patches/2014-03/msg00051.html The patch is OK for trunk - sorry for forgetting about it.
Re: [v3] Slightly improve operator new
Ping http://gcc.gnu.org/ml/gcc-patches/2014-03/msg00051.html On Sun, 2 Mar 2014, Marc Glisse wrote: Hello, inlining operator new (with LTO or otherwise), I noticed that it has a complicated implementation, which makes it hard to use this inlined code for optimizations. This patch does two things: 1) there are 2 calls to malloc, I am turning them into just one. At -Os, it does not change the generated code (RTL optimizers manage to merge the calls to malloc). At other levels (-O2, -O3, and especially with -g) it gives a smaller object file. And with just one malloc, some optimizations become much easier (see my recent calloc patch for instance). 2) malloc is predicted to return null 19 times out of 20 because of the loop (that didn't change with the patch), so I am adding __builtin_expect to let gcc optimize the fast path. Further discussion: a) I didn't add __builtin_expect for the test (sz == 0), it didn't change the generated code in my limited test. I was wondering if this test is necessary (new doesn't seem to ever call operator new(0)) or could be moved to operator new[] (new type[0] does call operator new[](0)), but since one can call operator new directly, it has to be protected indeed, so let's forget this point ;-) (too bad malloc is replacable, so we can't use the fact that glibc already does the right thing) b) I have a bit of trouble parsing the standard. Is the nothrow operator new supposed to call the regular operator new? In particular, if a user replaces only the throwing operator new, should the nothrow operator new automatically call that function? That's not what we are currently doing (and it would be a perf regression). Required behavior: Return a non-null pointer to suitably aligned storage (3.7.4), or else return a null pointer. This nothrow version of operator new returns a pointer obtained as if acquired from the (possibly replaced) ordinary version. This requirement is binding on a replacement version of this function. Default behavior: Calls operator new(size). If the call returns normally, returns the result of that call. Otherwise, returns a null pointer. Passes bootstrap+testsuite on x86_64-linux-gnu. Stage 1? 2014-03-03 Marc Glisse marc.gli...@inria.fr * libsupc++/new_op.cc: Factor the calls to malloc, use __builtin_expect. * libsupc++/new_opnt.cc: Likewise. -- Marc Glisse
[v3] Slightly improve operator new
Hello,
inlining operator new (with LTO or otherwise), I noticed that it has a
complicated implementation, which makes it hard to use this inlined code
for optimizations. This patch does two things:
1) there are 2 calls to malloc, I am turning them into just one. At -Os,
it does not change the generated code (RTL optimizers manage to merge the
calls to malloc). At other levels (-O2, -O3, and especially with -g) it
gives a smaller object file. And with just one malloc, some optimizations
become much easier (see my recent calloc patch for instance).
2) malloc is predicted to return null 19 times out of 20 because of the
loop (that didn't change with the patch), so I am adding __builtin_expect
to let gcc optimize the fast path.
Further discussion:
a) I didn't add __builtin_expect for the test (sz == 0), it didn't change
the generated code in my limited test. I was wondering if this test is
necessary (new doesn't seem to ever call operator new(0)) or could be
moved to operator new[] (new type[0] does call operator new[](0)), but
since one can call operator new directly, it has to be protected indeed,
so let's forget this point ;-)
(too bad malloc is replacable, so we can't use the fact that glibc already
does the right thing)
b) I have a bit of trouble parsing the standard. Is the nothrow operator
new supposed to call the regular operator new? In particular, if a user
replaces only the throwing operator new, should the nothrow operator new
automatically call that function? That's not what we are currently doing
(and it would be a perf regression).
Required behavior: Return a non-null pointer to suitably aligned storage
(3.7.4), or else return a null pointer. This nothrow version of operator
new returns a pointer obtained as if acquired from the (possibly replaced)
ordinary version. This requirement is binding on a replacement version of
this function.
Default behavior: Calls operator new(size). If the call returns normally,
returns the result of that call. Otherwise, returns a null pointer.
Passes bootstrap+testsuite on x86_64-linux-gnu. Stage 1?
2014-03-03 Marc Glisse marc.gli...@inria.fr
* libsupc++/new_op.cc: Factor the calls to malloc, use __builtin_expect.
* libsupc++/new_opnt.cc: Likewise.
--
Marc GlisseIndex: libsupc++/new_op.cc
===
--- libsupc++/new_op.cc (revision 208255)
+++ libsupc++/new_op.cc (working copy)
@@ -39,22 +39,21 @@ extern C void *malloc (std::size_t);
#endif
_GLIBCXX_WEAK_DEFINITION void *
operator new (std::size_t sz) _GLIBCXX_THROW (std::bad_alloc)
{
void *p;
/* malloc (0) is unpredictable; avoid it. */
if (sz == 0)
sz = 1;
- p = (void *) malloc (sz);
- while (p == 0)
+
+ while (__builtin_expect ((p = malloc (sz)) == 0, false))
{
new_handler handler = std::get_new_handler ();
if (! handler)
_GLIBCXX_THROW_OR_ABORT(bad_alloc());
handler ();
- p = (void *) malloc (sz);
}
return p;
}
Index: libsupc++/new_opnt.cc
===
--- libsupc++/new_opnt.cc (revision 208255)
+++ libsupc++/new_opnt.cc (working copy)
@@ -32,30 +32,28 @@ using std::bad_alloc;
extern C void *malloc (std::size_t);
_GLIBCXX_WEAK_DEFINITION void *
operator new (std::size_t sz, const std::nothrow_t) _GLIBCXX_USE_NOEXCEPT
{
void *p;
/* malloc (0) is unpredictable; avoid it. */
if (sz == 0)
sz = 1;
- p = (void *) malloc (sz);
- while (p == 0)
+
+ while (__builtin_expect ((p = malloc (sz)) == 0, false))
{
new_handler handler = std::get_new_handler ();
if (! handler)
return 0;
__try
{
handler ();
}
__catch(const bad_alloc)
{
return 0;
}
-
- p = (void *) malloc (sz);
}
return p;
}
