On Mon, 26 Feb 2018, Jakub Jelinek wrote:
> Hi!
>
> On Sat, Feb 17, 2018 at 10:08:48AM +0100, Jakub Jelinek wrote:
> > Note, this doesn't work, as G++ <= 4.2 rejects:
> > template <typename value_type>
> > struct S
> > {
> > char __attribute__ ((__aligned__ (__alignof__ (value_type)))) buf[sizeof
> > (value_type)];
> > };
> >
> > S<int> s;
> >
> > with:
> > error: requested alignment is not a constant
> >
> > The following works though, and has been successfully bootstrapped/regtested
> > on x86_64-linux and i686-linux. It will fail if we use hash_table on
> > overaligned value_type type with alignment > 64, but that is not completely
> > unlikely on any target I'm aware of. It uses __alignof__ which should be
> > supported by the system GCC's we support for which
> > BROKEN_VALUE_INITIALIZATION is true (i.e. >= 3.4 and <= 4.3), non-GCC
> > compilers shouldn't get here due to GCC_VERSION != 0 unless they pretend to
> > be GCC compatible.
> >
> > Bootstrapped with system gcc34/g++34 on x86_64-linux, ok for trunk?
> >
> > 2018-02-17 Jakub Jelinek <ja...@redhat.com>
> >
> > PR bootstrap/84405
> > * vec.h (vec_default_construct): For BROKEN_VALUE_INITIALIZATION use
> > memset and value initialization afterwards.
> > * hash-table.h (hash_table<Descriptor, Allocator>::empty_slow): For
> > BROKEN_VALUE_INITIALIZATION use placement new into a previously
> > cleared appropriately aligned buffer.
>
> I'd like to ping this patch, or if it isn't acceptable, at least the vec.h
> part of it which isn't really more complex than what we have right now.
So BROKEN_VALUE_INITIALIZATION doesn't really mean it's "broken" but
that is has certain behavior that doesn't invalidate the clearing
done earlier? That said, I wonder if first doing the regular
construction and memset afterwards in case of
BROKEN_VALUE_INITIALIZATION wouldn't be more reliable and "easier"?
> > --- gcc/vec.h.jj 2018-02-16 19:39:03.637058918 +0100
> > +++ gcc/vec.h 2018-02-16 23:51:55.389582056 +0100
> > @@ -490,12 +490,11 @@ template <typename T>
> > inline void
> > vec_default_construct (T *dst, unsigned n)
> > {
> > -#ifndef BROKEN_VALUE_INITIALIZATION
> > - for ( ; n; ++dst, --n)
> > - ::new (static_cast<void*>(dst)) T ();
> > -#else
> > +#ifdef BROKEN_VALUE_INITIALIZATION
> > memset (dst, '\0', sizeof (T) * n);
> > #endif
> > + for ( ; n; ++dst, --n)
> > + ::new (static_cast<void*>(dst)) T ();
> > }
> >
> > /* Copy-construct N elements in DST from *SRC. */
> > --- gcc/hash-table.h.jj 2018-02-16 19:39:03.660058934 +0100
> > +++ gcc/hash-table.h 2018-02-17 00:01:24.207723925 +0100
> > @@ -808,7 +808,18 @@ hash_table<Descriptor, Allocator>::empty
> > for ( ; size; ++entries, --size)
> > *entries = value_type ();
> > #else
> > - memset (entries, 0, size * sizeof (value_type));
> > + /* To workaround older GCC versions which don't handle value
> > + initialization right, use a placement new into previously
> > + cleared buffer. */
> > + char buf[2 * sizeof (value_type) + 64];
> > + gcc_assert (__alignof__ (value_type) <= sizeof (value_type) + 64);
> > + char *q = (buf + sizeof (value_type) + 64
> > + - ((uintptr_t) buf % __alignof__ (value_type)));
> > + memset (q, '\0', sizeof (value_type));
> > + value_type *p = ::new (q) value_type ();
> > + for ( ; size; ++entries, --size)
> > + *entries = *p;
> > + p->~value_type ();
> > #endif
> > }
> > m_n_deleted = 0;
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
