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)