On Fri, Nov 05, 2021 at 12:01:43PM -0500, will schmidt wrote:
> On Fri, 2021-11-05 at 00:04 -0400, Michael Meissner wrote:
> > Add new constant data structure.
> >
> > This patch provides the data structure and function to convert a
> > CONST_INT, CONST_DOUBLE, CONST_VECTOR, or VEC_DUPLICATE of a constant) to
> > an array of bytes, half-words, words, and double words that can be loaded
> > into a 128-bit vector register.
> >
> > The next patches will use this data structure to generate code that
> > generates load of the vector/floating point registers using the XXSPLTIDP,
> > XXSPLTIW, and LXVKQ instructions that were added in power10.
> >
> > 2021-11-05 Michael Meissner <meiss...@the-meissners.org>
> >
Whoops, it should be meiss...@linux.ibm.com.
> comment to be explicit on the structure name being copied to/from.
> (vec_const_128bit_type is easy to search for, vector or constant or
> structure are not as unique)
Yes, the original name was more generic (rs6000_const). Originally it could
potentially handle vector constants that were greater than 128-bits if we ever
have support for larger vectors. But I thought that extra generallity hindered
the code (since you had to check whether the size was exactly 128-bits, etc.).
So I made the data structure tailored to the problem at hand.
> > +
> > +/* Copy an floating point constant to the vector constant structure. */
> > +
>
> s/an/a/
Ok.
> > +static void
> > +constant_fp_to_128bit_vector (rtx op,
> > + machine_mode mode,
> > + size_t byte_num,
> > + vec_const_128bit_type *info)
> > +{
> > + unsigned bitsize = GET_MODE_BITSIZE (mode);
> > + unsigned num_words = bitsize / 32;
> > + const REAL_VALUE_TYPE *rtype = CONST_DOUBLE_REAL_VALUE (op);
> > + long real_words[VECTOR_128BIT_WORDS];
> > +
> > + /* Make sure we don't overflow the real_words array and that it is
> > + filled completely. */
> > + gcc_assert (num_words <= VECTOR_128BIT_WORDS && (bitsize % 32) == 0);
>
> Not clear to me on the potential to partially fill the real_words
> array.
At the moment we don't support a 16-bit floating point type in the compiler
(the Power10 has limited 16-bit floating point support, but we don't make a
special type for it). If/when we add the 16-bit floating point, we will
possibly need to revisit this.
> > +
> > + real_to_target (real_words, rtype, mode);
> > +
> > + /* Iterate over each 32-bit word in the floating point constant. The
> > + real_to_target function puts out words in endian fashion. We need
>
> Meaning host-endian fashion, or is that meant to be big-endian ?
Real_to_target puts out the 32-bit values in endian fashion. This data
structure wants to hold everything in big endian fashion to make checking
things simpler.
> Perhaps also rephrase or move the comment up to indicate that
> real_to_target will have placed or has already placed the words in
> <whatever> endian fashion.
> As stated I was expecting to see a call to real_to_target() below the
> comment.
Yes, I probably should move the real_to_target call after the comment.
> > +
> > + /* Possibly splat the constant to fill a vector size. */
>
>
> Suggest "Splat the constant to fill a vector size if ..."
Ok.
--
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meiss...@linux.ibm.com
