Hi,
On Tue, Jun 16, 2015 at 05:31:57PM +0200, Martin Liska wrote:
> On 06/16/2015 04:02 PM, Richard Biener wrote:
> > On Tue, Jun 16, 2015 at 3:38 PM, Martin Liška <mli...@suse.cz> wrote:
...
> >> Do you mean Richard following changes:
> >>
> >> alloc-pool.h (allocate):
> >> ...
> >> + /* Placement new contructor. */
> >> + inline void *operator new (size_t, elt_loc_list *&ptr)
> >> + {
> >> + return ptr;
> >> + }
> >
> > That should be there with including <new>
> >
> >> and e.g. cselib.h:
> >>
> >> struct cselib_val
> >> {
> >> /* Pool allocation new operator. */
> >> inline void *operator new (size_t)
> >> {
> >> return pool.allocate ();
> >> }
> >>
> >> /* Placement new contructor. */
> >> inline void *operator new (size_t, char *&ptr)
> >> {
> >> return ptr;
> >> }
> >
> > Yes, though I wonder whether cselib_val should really have undefined
> > contents after
> > allocating it? (or does the pool allocator zero the memory?)
> >
> > Richard.
>
> Hio.
>
> I've added calling of placement new operators and memset a memory, look the
> patch
> works for me.
>
> If it's the right way, I'll write Changelog and run testsuite.
So do I understand this thread correctly that any clas that overrides
its new operator to allocate from a pool also has to override its
placement new operator (to just return the argument)? (I'm thinking
of doing the same in the HSA branch.) If so, I think it would warrant
at least a simple comment before the pool allocator class in
alloc-pool.h.
> From d60bc8fa02161df64ddbb6bdb35c733af5e073c6 Mon Sep 17 00:00:00 2001
> From: mliska <mli...@suse.cz>
> Date: Tue, 16 Jun 2015 17:28:27 +0200
> Subject: [PATCH] Add placement new for classes in pool allocator.
>
> ---
> gcc/alloc-pool.h | 10 +++++++++-
> gcc/asan.c | 6 ++++++
> gcc/cselib.c | 6 ++++++
> gcc/cselib.h | 15 +++++++++++++++
> gcc/dse.c | 30 ++++++++++++++++++++++++++++++
> gcc/et-forest.c | 6 ++++++
> gcc/et-forest.h | 8 ++++++++
> gcc/ira-color.c | 6 ++++++
> gcc/lra-int.h | 18 ++++++++++++++++++
> gcc/regcprop.c | 6 ++++++
> gcc/tree-sra.c | 12 ++++++++++++
> gcc/var-tracking.c | 21 +++++++++++++++++++++
> 12 files changed, 143 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
> index 1785df5..237ece3 100644
> --- a/gcc/alloc-pool.h
> +++ b/gcc/alloc-pool.h
> @@ -413,7 +413,15 @@ pool_allocator<T>::allocate ()
> VALGRIND_DISCARD (VALGRIND_MAKE_MEM_UNDEFINED (header, size));
>
> /* Call default constructor. */
> - return (T *)(header);
> + T *ptr = (T *)header;
> +
> + if (!m_ignore_type_size)
> + {
> + memset (header, 0, sizeof (T));
> + return new (ptr) T ();
> + }
> + else
> + return ptr;
> }
Interesting, does this mean that the allocator will clear the memory
for all types? I thought the initialization of data should be left to
the class itself and its constructor. On the other hand, I suppose
that memset(this, 0, sizeof(*this)) is a very bad idea in C++, so the
simplicity of this might actually justify the overhead in cases where
we don't need the zeroing.
Thanks,
Martin
