Re: [PATCH] Add SPECIAL_FLOAT_MODE to enable adding IEEE 128-bit floating point to PowerPC

[Richard Sandiford <rdsandiford at googlemail dot com>]

Jeff Law <law@redhat.com> writes:
> On 05/05/2015 05:54 PM, Michael Meissner wrote:
>> This patch contains the machine independent changes that I will need to add
>> IEEE 128-bit floating point to the GCC compiler for PowerPC.
>>
>> It adds a new mode defintion macro: SPECIAL_FLOAT_MODE that is similar to
>> FLOAT_MODE.  The difference is the conversion system will skip
>> SPECIAL_FLOAT_MODE types when it is looking for a larger type.
>>
>> The problem is the PowerPC will have 2 128-bit floating point types, one using
>> the IBM double-double format (a pair of doubles to give the user more mantissa
>> bits, but no greater exponent range), and IEEE 128-bit floating point.
>> I don't
>> want DFmode to automatically convert to KFmode (IEEE 128-bit floating point),
>> but to TFmode (IBM double-double).
>>
>> With these patches applied, in the places where we are setting up types, we
>> include the special floating point types, but for the normal
>> GET_MODE_WIDER_MODE usage we do not want to use the special floating point
>> mode.
>>
>> I found some of the GET_MODE_WIDER_MODE loops needed to add a check
>> for running
>> out of modes if the mode type was special.  For those loops, I added a
>> test for
>> mode not being VOIDmode.
>>
>>
>> 2015-05-05  Michael Meissner  <meissner@linux.vnet.ibm.com>
>>
>> 	* machmode.h (GET_MODE_WIDER_MODE_SPECIAL): New macro to get the
>> 	wider modes that are normally skipped by default.
>>
>> 	* rtlanal.c (init_num_sign_bit_copies_in_rep): In going from
>> 	narrow to wider modes, check whether we receive VOIDmode, since
>> 	special floating point types are not listed in the normal widening
>> 	tables.  When doing initializations, go through all modes in a
>> 	class, even special modes, that are normally skipped by default
>> 	widening.
>> 	* cse.c (cse_insn): Likewise.
>> 	* expr.c (init_expr_target): Likewise.
>> 	(compress_float_constant): Likewise.
>> 	* dse.c (find_shift_sequence): Likewise.
>> 	* emit-rtl.c (init_derived_machine_modes): Likewise.
>> 	(init_emit_once): Likewise.
>> 	* combine.c (simplify_comparison): Likewise.
>>
>> 	* machmode.def (SPECIAL_FLOAT_MODE): New type of floating point
>> 	that is special, and is not automatically part of the normal
>> 	widening rules.
>>
>> 	* genmodes.c (struct mode_data): Add special field.
>> 	(blank_mode): Initialize special.
>> 	(complete_mode): Complex and vector types inherit the special mode
>> 	class.
>> 	(FLOAT_MODE): Add special field for floating point to sort special
>> 	nodes higher than normal nodes for the same size.  The intention
>> 	is to allow __float128 on PowerPC (KFmode) to be higher than long
>> 	double (TFmode), so that automatic widening uses the long double
>> 	type.
>> 	(FRACTIONAL_FLOAT_MODE): Likewise.
>> 	(SPECIAL_FLOAT_MODE): Likewise.
>> 	(FLOAT_MODE_INTERNAL): Likewise.
>> 	(make_float_mode): Likewise.
>> 	(emit_mode_wider): Likewise.
> So my worry here is that folks writing these loops to iterate over modes 
> are going to easily miss the != VOIDmode terminator, or not know when to 
> use GET_MODE_WIDER_SPECIAL.
>
> We can certainly go with the patch as-is since you've done the work to 
> sort out which GET_MODE_WIDER to use and added the appropriate 
> termination checks.   But do we want to try to future proof this a 
> little and define two iterators for folks to use rather than write the 
> loops by hand every time and probably getting it wrong -- and wrong in 
> such a way that it only breaks on PPC, forcing someone to regularly be 
> fixing this stuff?

Yeah.  I might be wrong, but I don't think the emit-rtl.c code
that uses GET_MODE_WIDER_MODE_SPECIAL actually cares about one thing
being wider than another.  It just wants to iterate over _all_ modes
of a particular class.  So maybe we could just convert them to iterate
over everything between MIN_MODE_foo and MAX_MODE_foo (already defined
in insn-modes.h).  Agree proper iterators would be even nicer though :-)

Thanks,
Richard