Re: rfc: constant pool and floats

[Geoff Keating <geoffk at geoffk dot org>]

On 16/10/2004, at 6:30 PM, Aldy Hernandez wrote:

> Thanks for taking the time on this.
>
> > > > That's the problem.  Clearly, (reg:SF 120) is not equal to the
> > > > CONST_INT; it's equal to a SFmode CONST_DOUBLE.
> > >
> > > I played around kludging the REG_EQUAL to be equal to a CONST_DOUBLE.
> > > That didn't fix the problem because when we fill in the constant pool
> > > (force_const_mem) with the value, we fill it in with a 
> > > const_int/SFmode
> > > pair, which still causes the abort while dumping the constant pool.
> >
> > Where did the CONST_INT come from, if not the REG_EQUAL note?
>
> Ah, see below.
>
> > >  /* If VALUE is a floating-point mode, access it as an integer of the
> > >     corresponding size.  This can occur on a machine with 64 bit
> > > registers
> > >     that uses SFmode for float.  This can also occur for unaligned
> > > float
> > >     structure fields.  */
> > >  if (GET_MODE_CLASS (GET_MODE (value)) != MODE_INT
> > >      && GET_MODE_CLASS (GET_MODE (value)) != MODE_PARTIAL_INT)
> > >    value = gen_lowpart ((GET_MODE (value) == VOIDmode
> > > 			  ? word_mode : int_mode_for_mode (GET_MODE
> > > 			  (value))),
> > > 			 value);
> >
> > This code is not wrong, as such, but I'm a bit confused about what 
> > then
> > happens to 'value'.  The routine appears to return it, but there's no
> > documentation about what the routine is *supposed* to return, and 
> > every
> > use of it in the compiler appears to ignore the return value.
> >
> > Anyway, what you want to look for is the first point where SFmode and
> > CONST_INT get associated with each other.
>
> The code above sets 'value' to the const_int.  Further down we recurse 
> into
> store_bit_field:
>
>           /* Fetch that unit, store the bitfield in it, then store
>              the unit.  */
>           tempreg = copy_to_reg (op0);
> =>        store_bit_field (tempreg, bitsize, bitpos, fieldmode, value);
>
> Where:
> 	tempreg = (reg:SI 119)
> 	fieldmode = SFmode
> 	value = (const_int 1202908642)
>
> Store_bit_field eventually calls emit_move_insn with:
> 	dst = (subreg:SF (reg:SI 119) 0)
> 	src = (const_int xx)
>
> *This* is the first time we associate an SF with a const_int.

Aha!  So it should not do that.  I think the right thing to do is to 
pass the original VALUE here, but if that doesn't work, try passing the 
mode of VALUE instead of FIELDMODE (the mode of VALUE is the mode that 
got passed to gen_lowpart).

> I'm a bit confused here, so I'm not sure if this is related, but
> emit_move_insn calls rs6000_emit_move with the above 'dst' and 'src',
> which calls force_reg(SFmode, (const_int xx)), which is where we get
> the REG_EQUAL note:
>
> (insn 11 10 0 (set (reg:SF 120)
>         (mem/u/i:SF (lo_sum:SI (reg:SI 121)
>                 (symbol_ref/u:SI ("*.LC0") [flags 0x2])) [0 S4 A32])) 
> -1 (nil)
>     (expr_list:REG_EQUAL (const_int 1202908642 [0x47b2ede2])
>         (nil)))
>
> It looks like store_bit_field is at fault by calling the emit_move_insn
> with an incompatible source/dest.  Is (set (reg:SF) (const_int)) valid
> rtl?  David had mentioned it was, but now I'm not sure.

No, I don't think it's valid RTL; certainly, there is no reason for it 
to be.

David commented:

> The rs6000 movsf pattern allows that combination, if reg:SF is a GPR.

which it does, by using 'n', but I suspect that's just a workaround for 
some old bug (perhaps even this one).  It's been that way since at 
least 1997.

Attachment: smime.p7s
Description: S/MIME cryptographic signature