On Wed, Jun 17, 2015 at 4:44 PM, Martin Liška <mli...@suse.cz> wrote:
> On 06/17/2015 01:29 PM, Richard Biener wrote:
>> On Wed, Jun 17, 2015 at 1:22 PM, Jakub Jelinek <ja...@redhat.com> wrote:
>>> On Wed, Jun 17, 2015 at 11:13:58AM +0200, Martin Jambor 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?)
>>>
>>> IMHO if you want to put placement new into allocate, then you probably need
>>> two different methods, one that will return you just void * to the
>>> allocated memory chunk that you should use
>>> inline void *operator new (size_t) { return pool.allocate (); } on,
>>> and another method that will use that and additionally invoke a placement
>>> new on it and return you the constructed pointer, which you'd use elsewhere.
>>> Otherwise you'd construct the object twice...
>>
>> Note that in the case of cselib_val a
>>
>> new cselib_val;
>>
>> will already invoke the constructor. I think it's just too much
>> (partly) C++-ification here.
>> Either all alloc-pool users have to work that way or none.
>>
>> Richard.
>>
>>> Jakub
>
> Hello.
>
> You are right that we should call ::new just for classes that have
> m_ignore_type_size == false.
> I've come up with following patch, that I tested slightly:
>
> diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h
> index 1785df5..7da5f7a 100644
> --- a/gcc/alloc-pool.h
> +++ b/gcc/alloc-pool.h
> @@ -412,8 +412,16 @@ pool_allocator<T>::allocate ()
> #endif
> VALGRIND_DISCARD (VALGRIND_MAKE_MEM_UNDEFINED (header, size));
>
> + T *ptr = (T *)header;
> +
> /* Call default constructor. */
> - return (T *)(header);
> + if (!m_ignore_type_size)
> + {
> + memset (header + sizeof (T), 0, m_extra_size);
> + return ::new (ptr) T;
> + }
> + else
> + return ptr;
> }
>
> /* Puts PTR back on POOL's free list. */
>
> Would it be suitable?
Suitable with the memset removed, yes.
Thanks,
Richard.
> Martin
