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)

Reply via email to