This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 05/06/2015 11:29 AM, Michael Meissner wrote:
On Wed, May 06, 2015 at 04:03:00PM +0100, Richard Sandiford wrote:
Jeff Law <law@redhat.com> writes:
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

If they miss the != VOIDmode, the program will hang since it will never exit
the loop (VOIDmode is the wider type for VOIDmode).
OK. And presumably if someone is adding a new loop over the modes they'll actually be testing that code on whatever target they're using. So it's unlikely they'll introduce an infinite loop on other targets by accident.



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 :-)

The code in emit-rtl.c that uses GET_MODE_WIDER_MODE_SPECIAL just is
initializing all of the types.  There are 2 functions:

init_derived_machine_modes -- this really doesn't need special, since it is
just looking at the INT types, looking to define byte_mode, word_mode, and
ptr_mode to appropriate integer types.

init_emit_once -- this is initializing all of the special constants (-1.0, 0.0,
1.0, etc.) for each of the modes.  The only one that really needs to use
_SPECIAL is the floating point loop.  When I was making the change months ago,
I just used _SPECIAL in all of the places, just in case we ever needed to have
an INT special type, or decimal float, etc.
I wonder if we're iterating over all the modes if we should just iterate to # modes rather than using the GET_WIDER stuff.

Perhaps it is simpler just to have a target hook that says whether a given
conversion can happen automatically (as opposed to explicitly if asked for by
the user).
Maybe. I may be over-thinking the issues here -- mostly I just want to make sure we don't introduce something folks get wrong easily and in turn break ports which depend on this stuff.

Jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]