On Fri, 24 Feb 2023, Jonathan Wakely wrote:
> On Fri, 24 Feb 2023 at 11:52, Jakub Jelinek <[email protected]> wrote:
> >
> > On Fri, Feb 24, 2023 at 12:44:44PM +0100, Richard Biener wrote:
> > > --- a/gcc/vec.h
> > > +++ b/gcc/vec.h
> > > @@ -586,8 +586,8 @@ public:
> > > unsigned allocated (void) const { return m_vecpfx.m_alloc; }
> > > unsigned length (void) const { return m_vecpfx.m_num; }
> > > bool is_empty (void) const { return m_vecpfx.m_num == 0; }
> > > - T *address (void) { return m_vecdata; }
> > > - const T *address (void) const { return m_vecdata; }
> > > + T *address (void) { return reinterpret_cast <T *> (this + 1); }
> > > + const T *address (void) const { return reinterpret_cast <const T *>
> > > (this + 1); }
> >
> > This is now too long.
Fixed.
> > > T *begin () { return address (); }
> > > const T *begin () const { return address (); }
> > > T *end () { return address () + length (); }
> > > @@ -631,8 +631,7 @@ public:
> > >
> > > /* FIXME - These fields should be private, but we need to cater to
> > > compilers that have stricter notions of PODness for types. */
> > > - vec_prefix m_vecpfx;
> > > - T m_vecdata[1];
> > > + alignas (T) vec_prefix m_vecpfx;
> >
> > The comment needs adjustment and down't we need
> > alignas (T) alignas (vec_prefix) ?
>
> Yes. If alignas(T) is less than the natural alignment then this will
> be an error. We want it to be the larger of the two alignments, so we
> need to specify both.
OK, changed to specify both and adjusted the comment, also noting why
we do this - it simplifies address (), otherwise we'd have to round up
to an aligned address.
> >
> > > @@ -1588,7 +1587,7 @@ public:
> > >
> > > private:
> > > vec<T, va_heap, vl_embed> m_auto;
> > > - T m_data[MAX (N - 1, 1)];
> > > + alignas(T) unsigned char m_data[sizeof (T) * N];
> > > };
> >
> > I still believe you don't need alignas(T) here (and space before (T) ).
I was worried that with auto_vec<__int128> we get tail-padding in m_auto
re-used, but since this isn't inheritance we're probably safe. So
removed give that m_auto is aligned to T.
> > Also, I think it needs to be MAX (N, 2) instead of N, because auto_vec
> > ctors use MAX (N, 2). We could also change all those to MAX (N, 1)
> > now, but it can't be N because m_data[sizeof (T) * 0] is invalid in
> > standard C.
I've removed the MAX (N, 2) now, I think that N == 0 cannot happen
because we have a specialization covering that. So we know N is
at least 1.
> > Anyway, I wonder if you get the -Werror=stringop-overflow= errors during
> > bootstrap that I got with my version or not.
Yes, I get this as well, not sure how to suppress it. I guess there's
no standard way to get at the address after some object without going
through uintptr obfuscation - and obviously we do not want to have
that (and if we optimize it away that doesn't help the diagnostic ...)
Richard.
