On Tue, Sep 17, 2019 at 4:33 PM Richard Sandiford <richard.sandif...@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.sandif...@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; > } > } >