On Thu, 5 Nov 2015, Martin Jambor wrote:
> Hi,
>
> we use C++ new operators based on alloc-pools a lot in the subsequent
> patches and realized that on the current trunk, such new operators
> would needlessly call the placement ::new operator within the allocate
> method of pool-alloc. Fixed below by providing a new allocation
> method which does not call placement new, which is only safe to use
> from within a new operator.
>
> The patch also fixes the slightly weird two parameter operator new
> (which we do not use in HSA backend) so that it does not do the same.
Why do you need to add the pointer variant then?
Also isn't the issue with allocate() that it does
return ::new (m_allocator.allocate ()) T ();
which 1) value-initializes and 2) doesn't even work with types like
struct T { T(int); };
thus types without a default constructor.
I think the allocator was poorly C++-ified without updating the
specification for the cases it is supposed to handle. And now
we have C++ uses that are not working because the allocator is
broken.
An incrementally better version (w/o fixing the issue with
types w/o default constructor) is
return ::new (m_allocator.allocate ()) T;
thus default-initialize which does no initialization for PODs (without
array members...) which is what the old pool allocator did.
To fix the new operator (how do you even call that? does it allow
specifying constructor args and thus work without a default constructor?)
it should indeed use an allocation method not performing the placement
new. But I'd call it allocate_raw rather than vallocate.
Thanks.
Richard.
> Thanks,
>
> Martin
>
>
> 2015-11-05 Martin Liska <mli...@suse.cz>
> Martin Jambor <mjam...@suse.cz>
>
> * alloc-pool.h (object_allocator::vallocate): New method.
> (operator new): Call vallocate instead of allocate.
> (operator new): New operator.
>
>
> diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
> index 0dc05cd..46b6550 100644
> --- a/gcc/alloc-pool.h
> +++ b/gcc/alloc-pool.h
> @@ -483,6 +483,12 @@ public:
> return ::new (m_allocator.allocate ()) T ();
> }
>
> + inline void *
> + vallocate () ATTRIBUTE_MALLOC
> + {
> + return m_allocator.allocate ();
> + }
> +
> inline void
> remove (T *object)
> {
> @@ -523,12 +529,19 @@ struct alloc_pool_descriptor
> };
>
> /* Helper for classes that do not provide default ctor. */
> -
> template <typename T>
> inline void *
> operator new (size_t, object_allocator<T> &a)
> {
> - return a.allocate ();
> + return a.vallocate ();
> +}
> +
> +/* Helper for classes that do not provide default ctor. */
> +template <typename T>
> +inline void *
> +operator new (size_t, object_allocator<T> *a)
> +{
> + return a->vallocate ();
> }
>
> /* Hashtable mapping alloc_pool names to descriptors. */
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
