Re: Make assemble_real generate canonical CONST_INTs

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Sep 17, 2019 at 4:33 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> assemble_real used GEN_INT to create integers directly from the
> longs returned by real_to_target.  assemble_integer then went on
> to interpret the const_ints as though they had the mode corresponding
> to the accompanying size parameter:
>
>       imode = mode_for_size (size * BITS_PER_UNIT, mclass, 0).require ();
>
>       for (i = 0; i < size; i += subsize)
>         {
>           rtx partial = simplify_subreg (omode, x, imode, i);
>
> But in the assemble_real case, X might not be canonical for IMODE.
>
> If the interface to assemble_integer is supposed to allow outputting
> (say) the low 4 bytes of a DImode integer, then the simplify_subreg
> above is wrong.  But if the number of bytes passed to assemble_integer
> is supposed to be the number of bytes that the integer actually contains,
> assemble_real is wrong.
>
> This patch takes the latter interpretation and makes assemble_real
> generate const_ints that are canonical for the number of bytes passed.
>
> The flip_storage_order handling assumes that each long is a full
> SImode, which e.g. excludes BITS_PER_UNIT != 8 and float formats
> whose memory size is not a multiple of 32 bits (which includes
> HFmode at least).  The patch therefore leaves that code alone.
> If interpreting each integer as SImode is correct, the const_ints
> that it generates are also correct.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  Also tested
> by making sure that there were no new errors from a range of
> cross-built targets.  OK to install?
>
> Richard
>
>
> 2019-09-17  Richard Sandiford  <richard.sandiford@arm.com>
>
> gcc/
>         * varasm.c (assemble_real): Generate canonical const_ints.
>
> Index: gcc/varasm.c
> ===================================================================
> --- gcc/varasm.c        2019-09-05 08:49:30.829739618 +0100
> +++ gcc/varasm.c        2019-09-17 15:30:10.400740515 +0100
> @@ -2873,25 +2873,27 @@ assemble_real (REAL_VALUE_TYPE d, scalar
>    real_to_target (data, &d, mode);
>
>    /* Put out the first word with the specified alignment.  */
> +  unsigned int chunk_nunits = MIN (nunits, units_per);
>    if (reverse)
>      elt = flip_storage_order (SImode, gen_int_mode (data[nelts - 1], SImode));
>    else
> -    elt = GEN_INT (data[0]);
> -  assemble_integer (elt, MIN (nunits, units_per), align, 1);
> -  nunits -= units_per;
> +    elt = GEN_INT (sext_hwi (data[0], chunk_nunits * BITS_PER_UNIT));

why the appearant difference between the storage-order flipping
variant using gen_int_mode vs. the GEN_INT with sext_hwi?
Can't we use gen_int_mode in the non-flipping path and be done with that?

> +  assemble_integer (elt, chunk_nunits, align, 1);
> +  nunits -= chunk_nunits;
>
>    /* Subsequent words need only 32-bit alignment.  */
>    align = min_align (align, 32);
>
>    for (int i = 1; i < nelts; i++)
>      {
> +      chunk_nunits = MIN (nunits, units_per);
>        if (reverse)
>         elt = flip_storage_order (SImode,
>                                   gen_int_mode (data[nelts - 1 - i], SImode));
>        else
> -       elt = GEN_INT (data[i]);
> -      assemble_integer (elt, MIN (nunits, units_per), align, 1);
> -      nunits -= units_per;
> +       elt = GEN_INT (sext_hwi (data[i], chunk_nunits * BITS_PER_UNIT));
> +      assemble_integer (elt, chunk_nunits, align, 1);
> +      nunits -= chunk_nunits;
>      }
>  }
>