On Fri, May 3, 2024 at 2:24 AM Aldy Hernandez <al...@redhat.com> wrote:
>
> Sparc requires strict alignment and is choking on the byte vector in
> Value_Range. Is this the right approach, or is there a more canonical
> way of forcing alignment?
I think the suggestion was to change over to use an union and use the
types directly in the union (anonymous unions and unions containing
non-PODs are part of C++11).
That is:
union {
int_range_max int_range;
frange fload_range;
unsupported_range un_range;
};
...
m_vrange = new (&int_range) int_range_max ();
...
Also the canonical way of forcing alignment in C++ is to use aliagnas
as my patch in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114912
did.
Also I suspect the alignment is not word alignment but rather the
alignment of HOST_WIDE_INT which is not always the same as the
alignment of the pointer but bigger and that is why it is failing on
sparc (32bit rather than 64bit).
Thanks,
Andrew Pinski
>
> If this is correct, OK for trunk?
>
> gcc/ChangeLog:
>
> * value-range.h (class Value_Range): Use a union.
> ---
> gcc/value-range.h | 24 +++++++++++++++---------
> 1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/value-range.h b/gcc/value-range.h
> index 934eec9e386..31af7888018 100644
> --- a/gcc/value-range.h
> +++ b/gcc/value-range.h
> @@ -740,9 +740,14 @@ private:
> void init (const vrange &);
>
> vrange *m_vrange;
> - // The buffer must be at least the size of the largest range.
> - static_assert (sizeof (int_range_max) > sizeof (frange), "");
> - char m_buffer[sizeof (int_range_max)];
> + union {
> + // The buffer must be at least the size of the largest range, and
> + // be aligned on a word boundary for strict alignment targets
> + // such as sparc.
> + static_assert (sizeof (int_range_max) > sizeof (frange), "");
> + char m_buffer[sizeof (int_range_max)];
> + void *align;
> + } u;
> };
>
> // The default constructor is uninitialized and must be initialized
> @@ -816,11 +821,11 @@ Value_Range::init (tree type)
> gcc_checking_assert (TYPE_P (type));
>
> if (irange::supports_p (type))
> - m_vrange = new (&m_buffer) int_range_max ();
> + m_vrange = new (&u.m_buffer) int_range_max ();
> else if (frange::supports_p (type))
> - m_vrange = new (&m_buffer) frange ();
> + m_vrange = new (&u.m_buffer) frange ();
> else
> - m_vrange = new (&m_buffer) unsupported_range ();
> + m_vrange = new (&u.m_buffer) unsupported_range ();
> }
>
> // Initialize object with a copy of R.
> @@ -829,11 +834,12 @@ inline void
> Value_Range::init (const vrange &r)
> {
> if (is_a <irange> (r))
> - m_vrange = new (&m_buffer) int_range_max (as_a <irange> (r));
> + m_vrange = new (&u.m_buffer) int_range_max (as_a <irange> (r));
> else if (is_a <frange> (r))
> - m_vrange = new (&m_buffer) frange (as_a <frange> (r));
> + m_vrange = new (&u.m_buffer) frange (as_a <frange> (r));
> else
> - m_vrange = new (&m_buffer) unsupported_range (as_a <unsupported_range>
> (r));
> + m_vrange
> + = new (&u.m_buffer) unsupported_range (as_a <unsupported_range> (r));
> }
>
> // Assignment operator. Copying incompatible types is allowed. That
> --
> 2.44.0
>
