On Fri, Feb 24, 2023 at 10:30:00AM +0000, Jonathan Wakely wrote:
> 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];
unsigned char m_data[MAX (N, 2) * sizeof (T)];
if we want to preserve current behavior I think.
I've screwed up, when I was about to change this line, I've realized
I want to look at the embedded_size stuff first and then forgot
to update this. With the above line it builds, though I bet one will need
to compare the generated code etc. and test with GCC 4.8.
Though I guess we now have also an option to change auto_vec with N 0/1
to have 1 embedded element instead of 2, so that would also mean changing
all of MAX (N, 2) to MAX (N, 1) if we want.
--- gcc/vec.h.jj 2023-01-02 09:32:32.177143804 +0100
+++ gcc/vec.h 2023-02-24 11:54:49.859564268 +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, 2) * sizeof (T)];
};
/* auto_vec is a sub class of vec whose storage is released when it is
Jakub
