On Fri, 24 Feb 2023 at 10:24, Jakub Jelinek <ja...@redhat.com> wrote:
>
> On Fri, Feb 24, 2023 at 11:02:07AM +0100, Jakub Jelinek via Gcc-patches wrote:
> > Maybe this would work, vl_relative even could be vl_embed.
> > Because vl_embed I believe is used in two spots, part of
> > auto_vec where it is followed by m_data and on heap or GGC
> > allocated memory where vec<..., vl_embed> is followed by
> > further storage for the vector.
>
> So roughtly something like below? Except I get weird crashes with it
> in the gen* tools. And we'd need to adjust the gdb python hooks
> which also use m_vecdata.
>
> --- gcc/vec.h.jj 2023-01-02 09:32:32.177143804 +0100
> +++ gcc/vec.h 2023-02-24 11:19:37.900157177 +0100
> @@ -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 (T *) (this + 1); }
> + const T *address (void) const { return (const T *) (this + 1); }
> T *begin () { return address (); }
> const T *begin () const { return address (); }
> T *end () { return address () + length (); }
> @@ -629,10 +629,9 @@ public:
> friend struct va_gc_atomic;
> friend struct va_heap;
>
> - /* FIXME - These fields should be private, but we need to cater to
> + /* FIXME - This field 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) alignas (vec_prefix) vec_prefix m_vecpfx;
> };
>
>
> @@ -879,7 +878,7 @@ inline const T &
> vec<T, A, vl_embed>::operator[] (unsigned ix) const
> {
> gcc_checking_assert (ix < m_vecpfx.m_num);
> - return m_vecdata[ix];
> + return address ()[ix];
> }
>
> template<typename T, typename A>
> @@ -887,7 +886,7 @@ inline T &
> vec<T, A, vl_embed>::operator[] (unsigned ix)
> {
> gcc_checking_assert (ix < m_vecpfx.m_num);
> - return m_vecdata[ix];
> + return address ()[ix];
> }
>
>
> @@ -929,7 +928,7 @@ vec<T, A, vl_embed>::iterate (unsigned i
> {
> if (ix < m_vecpfx.m_num)
> {
> - *ptr = m_vecdata[ix];
> + *ptr = address ()[ix];
> return true;
> }
> else
> @@ -955,7 +954,7 @@ vec<T, A, vl_embed>::iterate (unsigned i
> {
> if (ix < m_vecpfx.m_num)
> {
> - *ptr = CONST_CAST (T *, &m_vecdata[ix]);
> + *ptr = CONST_CAST (T *, &address ()[ix]);
> return true;
> }
> else
> @@ -978,7 +977,7 @@ vec<T, A, vl_embed>::copy (ALONE_MEM_STA
> {
> vec_alloc (new_vec, len PASS_MEM_STAT);
> new_vec->embedded_init (len, len);
> - vec_copy_construct (new_vec->address (), m_vecdata, len);
> + vec_copy_construct (new_vec->address (), address (), len);
> }
> return new_vec;
> }
> @@ -1018,7 +1017,7 @@ inline T *
> vec<T, A, vl_embed>::quick_push (const T &obj)
> {
> gcc_checking_assert (space (1));
> - T *slot = &m_vecdata[m_vecpfx.m_num++];
> + T *slot = &address ()[m_vecpfx.m_num++];
> *slot = obj;
> return slot;
> }
> @@ -1031,7 +1030,7 @@ inline T &
> vec<T, A, vl_embed>::pop (void)
> {
> gcc_checking_assert (length () > 0);
> - return m_vecdata[--m_vecpfx.m_num];
> + return address ()[--m_vecpfx.m_num];
> }
>
>
> @@ -1056,7 +1055,7 @@ vec<T, A, vl_embed>::quick_insert (unsig
> {
> gcc_checking_assert (length () < allocated ());
> gcc_checking_assert (ix <= length ());
> - T *slot = &m_vecdata[ix];
> + T *slot = &address ()[ix];
> memmove (slot + 1, slot, (m_vecpfx.m_num++ - ix) * sizeof (T));
> *slot = obj;
> }
> @@ -1071,7 +1070,7 @@ inline void
> vec<T, A, vl_embed>::ordered_remove (unsigned ix)
> {
> gcc_checking_assert (ix < length ());
> - T *slot = &m_vecdata[ix];
> + T *slot = &address ()[ix];
> memmove (slot, slot + 1, (--m_vecpfx.m_num - ix) * sizeof (T));
> }
>
> @@ -1118,7 +1117,7 @@ inline void
> vec<T, A, vl_embed>::unordered_remove (unsigned ix)
> {
> gcc_checking_assert (ix < length ());
> - m_vecdata[ix] = m_vecdata[--m_vecpfx.m_num];
> + address ()[ix] = address ()[--m_vecpfx.m_num];
> }
>
>
> @@ -1130,7 +1129,7 @@ inline void
> vec<T, A, vl_embed>::block_remove (unsigned ix, unsigned len)
> {
> gcc_checking_assert (ix + len <= length ());
> - T *slot = &m_vecdata[ix];
> + T *slot = &address ()[ix];
> m_vecpfx.m_num -= len;
> memmove (slot, slot + len, (m_vecpfx.m_num - ix) * sizeof (T));
> }
> @@ -1309,7 +1308,7 @@ vec<T, A, vl_embed>::embedded_size (unsi
> vec, vec_embedded>::type vec_stdlayout;
> static_assert (sizeof (vec_stdlayout) == sizeof (vec), "");
> static_assert (alignof (vec_stdlayout) == alignof (vec), "");
> - return offsetof (vec_stdlayout, m_vecdata) + alloc * sizeof (T);
> + return sizeof (vec_stdlayout) + alloc * sizeof (T);
> }
>
>
> @@ -1476,10 +1475,10 @@ public:
> { return m_vec ? m_vec->length () : 0; }
>
> T *address (void)
> - { return m_vec ? m_vec->m_vecdata : NULL; }
> + { return m_vec ? m_vec->address () : NULL; }
>
> const T *address (void) const
> - { return m_vec ? m_vec->m_vecdata : NULL; }
> + { return m_vec ? m_vec->address () : NULL; }
>
> T *begin () { return address (); }
> const T *begin () const { return address (); }
> @@ -1584,7 +1583,7 @@ public:
>
> private:
> vec<T, va_heap, vl_embed> m_auto;
> - T m_data[MAX (N - 1, 1)];
> + unsigned char m_data[MAX (N - 1, 1)];
This needs to be alignas(T) unsigned char m_data[sizeof(T) * N];
The alignas might be redundant (because the member before it is
aligned as T) but we don't want the N-1 now that this contains the
entire storage, there isn't an element in the m_auto vec before it.
