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

[Richard Biener]

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...

Richard.

> --
> Marc Glisse