On Mon, 26 Feb 2018, Jakub Jelinek wrote:
> On Mon, Feb 26, 2018 at 01:15:45PM +0100, Richard Biener wrote:
> > > 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"?
>
> BROKEN_VALUE_INITIALIZATION means the value initialization is broken,
> where broken means that for certain cases (for 4.2.[0-3] more than others)
> it doesn't initialize the object or some subobjects at all.
> memset after wouldn't make any sense, that is equivalent to just memset.
> What the patch is meant to fix are cases where even the broken value
> initialization invokes a ctor (when T has user defined default ctor).
>
> There are several cases of
> vec<ipa_polymorphic_call_context> where ipa_polymorphic_call_context
> has user defined default ctor that doesn't clear the object, but instead
> sets a bunch of bool flags in it to true. If we do just memset when
> built with older system G++ we get different behavior.
>
> Even with the patch if BROKEN_VALUE_INITIALIZATION we won't handle e.g.
> cases where we'd have vec of a struct without user defined default ctor,
> but where one of the fields has ipa_polymorphic_call_context type or
> some other with non-trivial user defined default ctor that doesn't
> clear everything. We don't have anything like that in the codebase
> right now, and if we would, a workaround would be to provide user defined
> default ctor for such types if we want to put them into vec.
>
> Unlike the vec.h case, the hash-table.h is just for hypothetical case
> where we'd have a hash table with such types.
Ok, I see. So the vec.h change is ok but please put a comment there
reflecting the above.
IMHO we should wait for the hypothetical case to happen for hash-table.h.
I still hope we can stop caring about too old GCC in a few years ;)
Richard.
> > > > --- 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)
