[PATCH] Add __attribute__((malloc) to allocator and remove unused code

[Marc Glisse]

On Thu, 17 May 2018, Richard Biener wrote:

> On Thu, May 17, 2018 at 1:14 PM Marc Glisse <marc.glisse@inria.fr> wrote:
>
>> On Thu, 17 May 2018, Jonathan Wakely wrote:
>
>>> On 17/05/18 12:54 +0200, Marc Glisse wrote:
>>>> On Mon, 14 May 2018, Jonathan Wakely wrote:
>>>>
>>>>> As discussed at https://gcc.gnu.org/ml/libstdc++/2018-01/msg00073.html
>>>>> we can simplify the allocator function for valarray memory. I also
>>>>> noticed that the _Array(size_t) constructor is never used.
>>>>>
>>>>>     * include/bits/valarray_array.h (__valarray_get_memory): Remove.
>>>>>     (__valarray_get_storage): Call operator new directly. Remove
> ignored
>>>>>     top-level restrict qualifier and add malloc attribute instead.
>>>>
>>>> I am trying to understand the point of adding this attribute. The
> function
>>>> is just
>>>>
>>>> { return static_cast<_Tp*>(operator new(__n * sizeof(_Tp))); }
>>>>
>>>> The idea is that it isn't safe (? see PR 23383) to mark operator new
> with
>>>> the attribute, but it is safe for this particular use?
>>>
>>> I'd forgotten about that (I was assuming the compiler doesn't need to
>>> be told about the properties of operator new, because they're defined
>>> by the language). We can remove the attribute.
>
>> I am not necessarily asking to remove it. I don't have a good
>> understanding of what would break if we marked operator new with the
>> attribute, so I have no idea if those reasons also apply for this use in
>> valarray.
>
>>>> When optimizing, I certainly hope this trivial function gets inlined,
> and
>>>> then the attribute is lost (should the inliner add 'restrict' when
> inlining
>>>> a function with attribute malloc?) and all that matters is operator
> new.
>
>> If we determine that using the attribute here but not on operator new is
>> the right choice, then I believe we need some middle-end tweaks so it
>> isn't ignored.
>
> We don't have a good way to do this.  Your suggestion of adding restrict
> would work if it were not that only function-scope restrict uses are later
> handled...

This seems extremely similar to the issue of inlining functions with 
restrict arguments.

I have written a PR, but it is probably not worth submitting.

-- 
Marc Glisse