Re: [PATCH], PR target/85358, Fix __ibm128 being converter to __float128 on PowerPC ISA 3.0 (power9)
On Tue, Apr 17, 2018 at 12:51:17PM -0400, Michael Meissner wrote: > On Mon, Apr 16, 2018 at 02:53:35PM -0500, Segher Boessenkool wrote: > > Can't you just set up things such that GET_MODE_WIDER_TYPE does not > > return ieee128 for ibm128? With maybe a new hook yes, if that is > > necessary. > > To me both are semantically the same. Both involve adding a new hook, and > whether you change the macro or the calls, is a mattery of syntax. As I > recall, when I first started doing __float128, there were one or two places > that wants to do wider types, but there you want to do it, even if the hook > says not to do automatic conversions. You also don't want to outlaw explicit > conversions. > > I can rewrite it if the global/release maintainers would prefer. It should be a much smaller patch that way. And yes, you need some middle end maintainer's ack. Segher
Re: [PATCH], PR target/85358, Fix __ibm128 being converter to __float128 on PowerPC ISA 3.0 (power9)
On Mon, Apr 16, 2018 at 02:53:35PM -0500, Segher Boessenkool wrote: > Hi! > > On Mon, Apr 16, 2018 at 01:41:29PM -0400, Michael Meissner wrote: > > As I was working on PR target/85075 (to flesh some bugs with IEEE 128-bit > > support on the PowerPC, particularly with switching the default of long > > double), I noticed that for explicit IBM extended double, the compiler was > > converting the __ibm128 type to an IEEE 128-bit type, because those types > > had > > hardware support in ISA 3.0 (power9). > > > > Unfortunately, IBM extended double (a pair of doubles meant to give you more > > mantissa precision but not more expoenent range), is not completely > > representable in IEEE 128-bit floating point. There are likely some exposed > > corner cases involved, and the bottom bits might not be the same. > > > > While I would prefer IBM extended double to disappear entirely, I do think > > we > > need to deal with it for a few versions still to come. > > There are many subtargets that still need it, and nothing to change > that has been planned. Just to be clear, IBM extended double is fine, as long as you don't try to have both IBM extended double and IEEE 128-bit support in the SAME program (and of course, you can live with the various problems IBM extended double has). The main problem on the last 3 years has been that at its core, GCC believes there should be only one floating point type for a given size. > > I tried to come up with machine dependent ways to prevent the automatic > > widening from occuring, but I couldn't get anything to work reliably. This > > patch adds a new target hook that says whether the automatic widening > > between > > two modes should be allowed. The default hook says to allow all of the > > default > > widenings to occur, while the PowerPC override says IBM extended double does > > not widen to IEEE 128-bit and vice versa. > > > > Given we are in stage4 right now, it is not the time to add new hooks, but > > here > > is the patch. If it doesn't go into GCC 8, is it acceptable for being put > > early into GCC 9 with a backport before 8.2 comes out? > > > > I have tested this patch using bootstrap builds on a power8 little system, a > > power7 big endian system (both 32-bit and 64-bit), and an x86_64 bit system > > with no regressions. > > Can't you just set up things such that GET_MODE_WIDER_TYPE does not > return ieee128 for ibm128? With maybe a new hook yes, if that is > necessary. To me both are semantically the same. Both involve adding a new hook, and whether you change the macro or the calls, is a mattery of syntax. As I recall, when I first started doing __float128, there were one or two places that wants to do wider types, but there you want to do it, even if the hook says not to do automatic conversions. You also don't want to outlaw explicit conversions. I can rewrite it if the global/release maintainers would prefer. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Re: [PATCH], PR target/85358, Fix __ibm128 being converter to __float128 on PowerPC ISA 3.0 (power9)
Hi!
On Mon, Apr 16, 2018 at 01:41:29PM -0400, Michael Meissner wrote:
> As I was working on PR target/85075 (to flesh some bugs with IEEE 128-bit
> support on the PowerPC, particularly with switching the default of long
> double), I noticed that for explicit IBM extended double, the compiler was
> converting the __ibm128 type to an IEEE 128-bit type, because those types had
> hardware support in ISA 3.0 (power9).
>
> Unfortunately, IBM extended double (a pair of doubles meant to give you more
> mantissa precision but not more expoenent range), is not completely
> representable in IEEE 128-bit floating point. There are likely some exposed
> corner cases involved, and the bottom bits might not be the same.
>
> While I would prefer IBM extended double to disappear entirely, I do think we
> need to deal with it for a few versions still to come.
There are many subtargets that still need it, and nothing to change
that has been planned.
> I tried to come up with machine dependent ways to prevent the automatic
> widening from occuring, but I couldn't get anything to work reliably. This
> patch adds a new target hook that says whether the automatic widening between
> two modes should be allowed. The default hook says to allow all of the
> default
> widenings to occur, while the PowerPC override says IBM extended double does
> not widen to IEEE 128-bit and vice versa.
>
> Given we are in stage4 right now, it is not the time to add new hooks, but
> here
> is the patch. If it doesn't go into GCC 8, is it acceptable for being put
> early into GCC 9 with a backport before 8.2 comes out?
>
> I have tested this patch using bootstrap builds on a power8 little system, a
> power7 big endian system (both 32-bit and 64-bit), and an x86_64 bit system
> with no regressions.
Can't you just set up things such that GET_MODE_WIDER_TYPE does not
return ieee128 for ibm128? With maybe a new hook yes, if that is
necessary.
> --- gcc/target.def(revision 259376)
> +++ gcc/target.def(working copy)
> @@ -3486,6 +3486,13 @@ If this hook allows @code{val} to have a
> hook_bool_mode_uhwi_false)
>
> DEFHOOK
> +(default_widening_p,
> + "Return true if GCC can automatically widen from @var{from_mode} to\n\
> +@var{to_mode}. Conversions are unsigned if @var{unsigned_p} is true.",
> + bool, (machine_mode, machine_mode, bool),
> + hook_bool_mode_mode_bool_true)
This should have those names (from_mode, to_mode, unsigned_p) in the
prototype then, for the generated documentation to make sense.
Segher
