On Thu, Feb 23, 2023 at 01:54:27PM +0100, Richard Biener wrote:
> The following avoids default-initializing auto_vec storage for
> non-POD T since that's not what the allocated storage fallback
> will do and it's also not expected for existing cases like
>
> auto_vec<std::pair<unsigned, unsigned>, 64> elts;
>
> which exist to optimize the allocation.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>
> It saves around 1kB of text size for cc1.
>
> OK for trunk?
>
> Thanks,
> Richard.
>
> * 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.
Though, this shows we still have work to do. Because
1) T m_vecdata[1]; still has element type T in m_auto and
so is actually constructed/destructed uselessly
2) the non-POD support is certainly incomplete; we have
vec_default_construct and vec_copy_construct and use that
for say grow_cleared etc. or vector copies, but don't have
anything that would invoke the destructors and don't invoke
vec_copy_construct (aka placement new) when we just push
stuff into the vector; so, I bet what we do is invalid
and kind of works for non-PODs which have assignment operator
which overwrites everything, doesn't use anything from the
overwritten object and has the same overall effect
as copy constructor, and only support trivial dtors
For full non-POD support, I bet we'd need to vec_copy_construct (..., 1)
on quick_push rather than invoke operator= there, we'd need
to destruct stuff on pop/truncate etc., and quick_grow would need
to be effectively the same as quick_grow_cleared for non-PODs.
As for 1), if even m_vecdata was
alignas(T) unsigned char m_vecdata[sizeof (T)];
we'll need casts in various spots and it will printing vectors
by hand in gdb when the .gdbinit printers don't work properly.
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.
> --- a/gcc/vec.h
> +++ b/gcc/vec.h
> @@ -1584,7 +1584,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) * MAX (N - 1, 1)];
> };
>
> /* auto_vec is a sub class of vec whose storage is released when it is
Jakub
