On Thu, Feb 23, 2023 at 03:02:01PM +0000, Richard Biener wrote:
> > > * vec.h (auto_vec<T, N>): Turn m_data storage into
> > > uninitialized unsigned char.
> >
> > Given that we actually never reference the m_data array anywhere,
> > it is just to reserve space, I think even the alignas(T) there is
> > useless. The point is that m_auto has as data members:
> > vec_prefix m_vecpfx;
> > T m_vecdata[1];
> > and we rely on it (admittedly -fstrict-flex-arrays{,=2,=3} or
> > -fsanitize=bound-sstrict incompatible) being treated as
> > flexible array member flowing into the m_data storage after it.
>
> Doesn't the array otherwise eventually overlap with tail padding
> in m_auto? Or does an array of T never produce tail padding?
The array can certainly overlap with tail padding in m_auto if any.
But whether m_data is aligned to alignof (T) or not doesn't change anything
on it.
m_vecpfx is struct { unsigned m_alloc : 31, m_using_auto_storage : 1, m_num; },
so I think there is on most arches tail padding if T has smaller alignment
than int, so typically char/short or structs with the same size/alignments.
If that happens, alignof (auto_vec_x.m_auto) will be alignof (int),
there can be 2 or 3 padding bytes, but because sizeof (auto_vec_x.m_auto)
is 3 * sizeof (int), m_data will have offset always aligned to alignof (T).
If alignof (T) >= alignof (int), then there won't be any tail padding
at the end of m_auto, there could be padding between m_vecpfx and
m_vecdata, sizeof (auto_vec_x.m_auto) will be a multiple of sizeof (T) and
so m_data will be again already properly aligned.
So, I think your patch is fine without alignas(T), the rest is just that
there is more work to do incrementally, even for the case you want to
deal with (the point 1) in particular).
> Yes, I'm not proposing to fix non-POD support. I want to make
> as-if-POD stuff like std::pair to work like it was intended.
>
> > Oh, and perhaps we should start marking such spots in GCC with
> > strict_flex_array attribute to make it clear where we rely on the
> > non-strict behavior.
>
> I think we never access the array directly as array, do we?
Sure, the attribute should go to m_vecdata array, not to m_data.
And to op array in gimple_statement_with_ops, operands array in
operands, ops array in tree_omp_clause, val in tree_int_cst,
str in tree_string, elts in tree_vector, a in tree_vec, elem in rtvec_def,
elem in hwivec_def, u.{fld,hwint} in rtx_def and various others, we
use this stuff everywhere. Also often use GTY length similarly to the
proposed element_count...
Jakub
