| Message ID | 20180522232416.GA20583@ibm-toto.the-meissners.org |
|---|---|
| State | New |
| Headers | show |
| Series | PR target/85358: Add target hook to prevent default widening | expand |
On Tue, 22 May 2018, Michael Meissner wrote: > I posted this patch at the end of GCC 8 as a RFC. Now that we are in GCC 9, I > would like to repose it. Sorry to spam some of you. It is unclear whom the > reviewers for things like target hooks and basic mode handling are. > > Here is the original patch. > https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00764.html > > PowerPC has 3 different 128-bit floating point types (KFmode, IFmode, and > TFmode). We are in the process of migrating long double from IBM extended > double to IEEE 128-bit floating point. > > * IFmode is IBM extended double (__ibm128) > * KFmode is IEEE 128-bit floating point (__float128, _Float128N) > * TFmode is whatever long double maps to > > If we are compiling for a power8 system (which does not have hardware IEEE > 128-bit floating point), the current system works because each of the 3 modes > do not have hardware support. > > If we are compiling for a power9 system and long double is IBM extended double, > again things are fine. > > However, if we compiling for power9 and we've flipped the default for long > double to be IEEE 128-bit floating point, then the code to support __ibm128 > breaks. The machine independent portions of the mode handling says oh, there > is hardware to support TFmode operations, lets widen the type to TFmode and do > those operations. However, converting IFmode to TFmode is not cheap, it has to > be done in a function call. > > This patch adds a new target hook, that if it is overriden, the backend can say > don't automatically widen this type to that type. The PowerPC port defines the > target hook so that it doesn't automatically convert IBM extended double to > IEEE 128-bit and vice versa. > > This patch goes through all of the places that calls GET_MODE_WIDER_MODE and > then calls the target hook. Now, the PowerPC only needs to block certain > floating point widenings. Several of the changes are to integer widenings, and > if desired, we could restrict the changes to just floating point types. > However, there might be other ports that need the flexibility for other types. > > I have tried various other approprches to fix this problem, and so far, I have > not been able to come up with a PowerPC back-end only solution that works. > > Alternatively, Segher has suggested that the call to the target hook be in > GET_MODE_WIDER_MODE and GET_MODE_2XWIDER_MODE (plus any places where we access > the mode_wider array direcly). > > I have built little endian PowerPC builds with this patch, and I have verified > that it does work. I have tested the same patch in April on a big endian > PowerPC system and x86_64 and it worked there also. > > Can I check in this patch as is (I will verify x86/PowerPC big endian still > works before checkin). Or would people prefer modifications to the patch? Just a question for clarification - _is_ KFmode strictly a wider mode than IFmode? That is, can it represent all values that IFmode can? On another note I question the expanders considering wider FP modes somewhat in general. So maybe the hook shouldn't be named default_widening_p but rather mode_covers_p ()? And we can avoid calling the hook for integer modes. That said, I wonder if the construction of mode_wider and friends should be (optionally) made more explicit in the modes .def file so powerpc could avoid any wider relation for IFmode. Thanks, Richard. > [gcc] > 2018-05-22 Michael Meissner <meissner@linux.vnet.ibm.com> > > PR target/85358 > * target.def (default_widening_p): New target hook to say whether > default widening between modes should be done. > * targhooks.h (hook_bool_mode_mode_bool_true): New declaration. > * targhooks.c (hook_bool_mode_mode_bool_true): New default target > hook. > * optabs.c (expand_binop): Before doing default widening, check > whether the backend allows the widening. > (expand_twoval_unop): Likewise. > (expand_twoval_binop): Likewise. > (widen_leading): Likewise. > (widen_bswap): Likewise. > (expand_unop): Likewise. > * cse.c (cse_insn): Likewise. > * combine.c (simplify_comparison): Likewise. > * var-tracking.c (prepare_call_arguments): Likewise. > * config/rs6000/rs6000.c (TARGET_DEFAULT_WIDENING_P): Define > target hook to prevent IBM extended double and IEEE 128-bit > floating point from being converted to each by default. > (rs6000_default_widening_p): Likewise. > * doc/tm.texi (TARGET_DEFAULT_WIDENING_P): Document the new > default widening hook. > * doc/tm.texi.in (TARGET_DEFAULT_WIDENING_P): Likewise. > > [gcc/testsuite] > 2018-05-22 Michael Meissner <meissner@linux.vnet.ibm.com> > > PR target/85358 > * gcc.target/powerpc/pr85358.c: New test to make sure __ibm128 > does not widen to __float128 on ISA 3.0 systems. > > In order to start the transition of PowerPC long double to IEEE 128-bit, we > will need this patch or a similar patch to be back ported to GCC 8.2. > >
On Wed, May 23, 2018 at 11:41:49AM +0200, Richard Biener wrote: > Just a question for clarification - _is_ KFmode strictly a wider mode > than IFmode? That is, can it represent all values that IFmode can? No. IFmode consists of a pair of doubles. I'm sure there are corner cases IFmode can represent something that KFmode can't. In addition, IFmode doesn't play to well with reseting rounding modes, etc. > On another note I question the expanders considering wider FP modes > somewhat in general. So maybe the hook shouldn't be named > default_widening_p but rather mode_covers_p ()? And we can avoid > calling the hook for integer modes. > > That said, I wonder if the construction of mode_wider and friends > should be (optionally) made more explicit in the modes .def file > so powerpc could avoid any wider relation for IFmode. This morning on the way to work, I was thinking that maybe the solution is to have an ALTERNATE_FLOAT_MODE, where it doesn't list the other float modes of the same size as canadates for widening. That way we could express it the .def file directly. One of the problems I've faced over the years is the assumption that there is only one type for a given size, and that isn't true for IF/KF/TFmode.
On May 23, 2018 7:53:01 PM GMT+02:00, Michael Meissner <meissner@linux.ibm.com> wrote: >On Wed, May 23, 2018 at 11:41:49AM +0200, Richard Biener wrote: >> Just a question for clarification - _is_ KFmode strictly a wider mode >> than IFmode? That is, can it represent all values that IFmode can? > >No. IFmode consists of a pair of doubles. I'm sure there are corner >cases >IFmode can represent something that KFmode can't. > >In addition, IFmode doesn't play to well with reseting rounding modes, >etc. > >> On another note I question the expanders considering wider FP modes >> somewhat in general. So maybe the hook shouldn't be named >> default_widening_p but rather mode_covers_p ()? And we can avoid >> calling the hook for integer modes. >> >> That said, I wonder if the construction of mode_wider and friends >> should be (optionally) made more explicit in the modes .def file >> so powerpc could avoid any wider relation for IFmode. > >This morning on the way to work, I was thinking that maybe the solution >is to >have an ALTERNATE_FLOAT_MODE, where it doesn't list the other float >modes of >the same size as canadates for widening. That way we could express it >the .def >file directly. Yeah. Not sure if we need sth other than FLOAT_MODE or if avoiding the chaining in wider mode and friends is enough. I guess one could also change IFmode to be a non-scalar, composite mode - a different 'kind' of complex mode maybe... >One of the problems I've faced over the years is the assumption that >there is >only one type for a given size, and that isn't true for IF/KF/TFmode.
On Wed, 23 May 2018, Michael Meissner wrote: > One of the problems I've faced over the years is the assumption that there is > only one type for a given size, and that isn't true for IF/KF/TFmode. I think that's a different problem. The problem here is that "wider" is only a partial ordering between floating-point modes; neither IFmode nor KFmode is wider than the other. (While TFmode has the same set of values as whichever of IFmode and KFmode it corresponds to. And though IFmode is in some sense wider than DFmode, it's probably not a good idea to treat it as such given that it doesn't have the IEEE semantics of DFmode.) As a separate issue, anything that tries to deduce a floating-point mode from a size should have had a mode in the first place (for example, the *_TYPE_SIZE target macros for floating-point types really ought to be some kind of target hook that returns the required mode rather than a size).
On Wed, 23 May 2018, Richard Biener wrote: > Yeah. Not sure if we need sth other than FLOAT_MODE or if avoiding the > chaining in wider mode and friends is enough. I guess one could also > change IFmode to be a non-scalar, composite mode - a different 'kind' of > complex mode maybe... I'm pretty sure you want FOR_EACH_MODE_IN_CLASS (mode, MODE_FLOAT) to include IFmode, as otherwise all such iterators now present or added in future would probably need to be changed to include the new type of mode as well.
Hi, On Wed, May 23, 2018 at 08:35:49PM +0000, Joseph Myers wrote: > On Wed, 23 May 2018, Michael Meissner wrote: > > > One of the problems I've faced over the years is the assumption that there is > > only one type for a given size, and that isn't true for IF/KF/TFmode. > > I think that's a different problem. > > The problem here is that "wider" is only a partial ordering between > floating-point modes; neither IFmode nor KFmode is wider than the other. > (While TFmode has the same set of values as whichever of IFmode and KFmode > it corresponds to. And though IFmode is in some sense wider than DFmode, > it's probably not a good idea to treat it as such given that it doesn't > have the IEEE semantics of DFmode.) Which semantics doesn't it have? (Other than the lack of non-default rounding modes, or is that what you meant?) Segher
On Wed, May 23, 2018 at 08:38:21PM +0000, Joseph Myers wrote: > On Wed, 23 May 2018, Richard Biener wrote: > > > Yeah. Not sure if we need sth other than FLOAT_MODE or if avoiding the > > chaining in wider mode and friends is enough. I guess one could also > > change IFmode to be a non-scalar, composite mode - a different 'kind' of > > complex mode maybe... > > I'm pretty sure you want FOR_EACH_MODE_IN_CLASS (mode, MODE_FLOAT) to > include IFmode, as otherwise all such iterators now present or added in > future would probably need to be changed to include the new type of mode > as well. Yes. I think trying to change it to a non-scalar composite mode would likely be much more work.
On Thu, 24 May 2018, Segher Boessenkool wrote: > Which semantics doesn't it have? (Other than the lack of non-default > rounding modes, or is that what you meant?) It has problems with rounding modes, exceptions and various operations involving large or small arguments or results. Some of these apply even in round-to-nearest and when the arguments are exactly representable as double (spurious overflow from (long double) DBL_MAX / 3.0L, for example), which makes it unambiguous that a widening from DFmode to IFmode is never appropriate. (Of course, code doing such a widening would be bad for other reasons anyway, since it should only be widening from a non-hardware mode to a hardware one, and IFmode is never a hardware mode.)
I redid the patch to make the target hook only apply for scalar float points, and I removed all of the integer only subcases. I have checked this on a little endian Power8 system, and verified that it bootstraps correctly and there are no regressions. I have just started an x86_64 build. Assuming that build has no regressions, can I check this into GCC 9? This bug appears in GCC 8, and I would like to back port this patch to GCC 8 as well before GCC 8.2 goes out. [gcc] 2018-05-25 Michael Meissner <meissner@linux.ibm.com> PR target/85358 * target.def (default_fp_widening_p): New target hook to automatic widening betwen two floating point modes. * optabs.c (expand_binop): Do not automatically widen a binary or unary scalar floating point op if the backend says that the widening should not occur. (expand_twoval_unop): Likewise. (expand_twoval_binop): Likewise. (expand_unop): Likewise. * config/rs6000/rs6000.c (TARGET_DEFAULT_FP_WIDENING_P): Define. (rs6000_default_fp_widening_p): New target hook to prevent automatic widening between IEEE 128-bit floating point and IBM extended double floating point. * doc/tm.texi (Target Hooks): Document new target hook default_fp_widening_p. * doc/tm.texi.in (Target Hooks): Likewise. [gcc/testsuite] 2018-05-25 Michael Meissner <meissner@linux.ibm.com> PR target/85358 * gcc.target/powerpc/pr85358.c: New test.
On May 25, 2018 8:49:47 PM GMT+02:00, Michael Meissner <meissner@linux.ibm.com> wrote: >I redid the patch to make the target hook only apply for scalar float >points, >and I removed all of the integer only subcases. > >I have checked this on a little endian Power8 system, and verified that >it >bootstraps correctly and there are no regressions. I have just started >an >x86_64 build. Assuming that build has no regressions, can I check this >into >GCC 9? This bug appears in GCC 8, and I would like to back port this >patch to >GCC 8 as well before GCC 8.2 goes out. What happens if you hack genmodes to not claim IFmode has any wider relationship with other modes? Richard. >[gcc] >2018-05-25 Michael Meissner <meissner@linux.ibm.com> > > PR target/85358 > * target.def (default_fp_widening_p): New target hook to automatic > widening betwen two floating point modes. > * optabs.c (expand_binop): Do not automatically widen a binary or > unary scalar floating point op if the backend says that the > widening should not occur. > (expand_twoval_unop): Likewise. > (expand_twoval_binop): Likewise. > (expand_unop): Likewise. > * config/rs6000/rs6000.c (TARGET_DEFAULT_FP_WIDENING_P): Define. > (rs6000_default_fp_widening_p): New target hook to prevent > automatic widening between IEEE 128-bit floating point and IBM > extended double floating point. > * doc/tm.texi (Target Hooks): Document new target hook > default_fp_widening_p. > * doc/tm.texi.in (Target Hooks): Likewise. > >[gcc/testsuite] >2018-05-25 Michael Meissner <meissner@linux.ibm.com> > > PR target/85358 > * gcc.target/powerpc/pr85358.c: New test.
On Fri, May 25, 2018 at 02:49:47PM -0400, Michael Meissner wrote: > * target.def (default_fp_widening_p): New target hook to automatic > widening betwen two floating point modes. "default" is a pretty bad name. The rs6000 parts are fine of course, if the rest is. Segher
On Sat, May 26, 2018 at 08:13:04AM +0200, Richard Biener wrote: > On May 25, 2018 8:49:47 PM GMT+02:00, Michael Meissner <meissner@linux.ibm.com> wrote: > >I redid the patch to make the target hook only apply for scalar float > >points, > >and I removed all of the integer only subcases. > > > >I have checked this on a little endian Power8 system, and verified that > >it > >bootstraps correctly and there are no regressions. I have just started > >an > >x86_64 build. Assuming that build has no regressions, can I check this > >into > >GCC 9? This bug appears in GCC 8, and I would like to back port this > >patch to > >GCC 8 as well before GCC 8.2 goes out. > > What happens if you hack genmodes to not claim IFmode has any wider relationship with other modes? It breaks the initialization of all of the types that initializes each of the types from smallest to widest using the wider tables. So you either need to have special types that are initialized without using the wider table, or you need two sets of wider tables, one used by the initialization sequences, and the other by normal binop/unop expansion. There may be other dependencies that I'm not aware of (such as it must be within the MIN_MODE_FLOAT and MAX_MODE_FLOAT range).
I'm wondering if there are other suggestions to make this patch acceptable. As I mentioned previously, the initialization process needs to go through all of the widening tables in order to initialize all FP types, so we can't just arbitrarily eliminate IFmode from the widening table. I could imagine having an alternative *_FLOAT_MODE that essentially marks which modes shouldn't be widened to/from and binop/unop could use. Or I could imagine making two widening tables, one for initialization, and one for binop/unop, or other possibilities.
On Fri, 1 Jun 2018, Michael Meissner wrote: > I'm wondering if there are other suggestions to make this patch acceptable. > > As I mentioned previously, the initialization process needs to go through all > of the widening tables in order to initialize all FP types, so we can't just > arbitrarily eliminate IFmode from the widening table. Initialization that's meant to cover all floating-point modes logically should not rely on everything being reachable by the "wider" relation. That is, I'd expect it to do something equivalent to FOR_EACH_MODE_IN_CLASS (mode, MODE_FLOAT) rather than something based on "wider". But if the initialization relies on some form of partial ordering (initializing some modes relies on other modes having been initialized first, in a way somehow related to the "wider" relation), it may be more complicated than that.
On Fri, 1 Jun 2018, Joseph Myers wrote: > On Fri, 1 Jun 2018, Michael Meissner wrote: > > > I'm wondering if there are other suggestions to make this patch acceptable. > > > > As I mentioned previously, the initialization process needs to go through all > > of the widening tables in order to initialize all FP types, so we can't just > > arbitrarily eliminate IFmode from the widening table. > > Initialization that's meant to cover all floating-point modes logically > should not rely on everything being reachable by the "wider" relation. > > That is, I'd expect it to do something equivalent to > FOR_EACH_MODE_IN_CLASS (mode, MODE_FLOAT) rather than something based on > "wider". The unfortunate thing is that all iterators are wrapped around the "wider" relationship - we do not have any other way to find related modes. So the concept that all modes in a class can be ordered after their width is baked in very deeply. That's IMHO something we need to eventually fix to avoid this kind of target-hook "hacks". Richard. > But if the initialization relies on some form of partial > ordering (initializing some modes relies on other modes having been > initialized first, in a way somehow related to the "wider" relation), it > may be more complicated than that.
On Mon, Jun 04, 2018 at 08:46:42AM +0200, Richard Biener wrote: > On Fri, 1 Jun 2018, Joseph Myers wrote: > > > On Fri, 1 Jun 2018, Michael Meissner wrote: > > > > > I'm wondering if there are other suggestions to make this patch acceptable. > > > > > > As I mentioned previously, the initialization process needs to go through all > > > of the widening tables in order to initialize all FP types, so we can't just > > > arbitrarily eliminate IFmode from the widening table. > > > > Initialization that's meant to cover all floating-point modes logically > > should not rely on everything being reachable by the "wider" relation. > > > > That is, I'd expect it to do something equivalent to > > FOR_EACH_MODE_IN_CLASS (mode, MODE_FLOAT) rather than something based on > > "wider". > > The unfortunate thing is that all iterators are wrapped around > the "wider" relationship - we do not have any other way to find > related modes. So the concept that all modes in a class can be > ordered after their width is baked in very deeply. That's IMHO > something we need to eventually fix to avoid this kind of > target-hook "hacks". > > Richard. Given the related modes are in sequential order now, I could imagine ways to do the initialization and iterators using those values.
On Fri, Jun 01, 2018 at 04:36:54PM -0400, Michael Meissner wrote: > As I mentioned previously, the initialization process needs to go through all > of the widening tables in order to initialize all FP types, so we can't just > arbitrarily eliminate IFmode from the widening table. Or change the initialisation process. There is no wider mode for IFmode, and IFmode can not be used as a wider mode for any other mode. > I could imagine having an alternative *_FLOAT_MODE that essentially marks which > modes shouldn't be widened to/from and binop/unop could use. Or I could > imagine making two widening tables, one for initialization, and one for > binop/unop, or other possibilities. For integer types there are the partial int types, maybe we want something similar for float? Segher
On June 5, 2018 6:50:22 PM GMT+02:00, Segher Boessenkool <segher@kernel.crashing.org> wrote: >On Fri, Jun 01, 2018 at 04:36:54PM -0400, Michael Meissner wrote: >> As I mentioned previously, the initialization process needs to go >through all >> of the widening tables in order to initialize all FP types, so we >can't just >> arbitrarily eliminate IFmode from the widening table. > >Or change the initialisation process. There is no wider mode for >IFmode, >and IFmode can not be used as a wider mode for any other mode. > >> I could imagine having an alternative *_FLOAT_MODE that essentially >marks which >> modes shouldn't be widened to/from and binop/unop could use. Or I >could >> imagine making two widening tables, one for initialization, and one >for >> binop/unop, or other possibilities. > >For integer types there are the partial int types, maybe we want >something >similar for float? Are there other targets with similar kind of (FP) modes or is ppc the only one with this 'legacy'? Also keep in mind that you intended to backport this... Why not simply make IBM and IEEE double mutually exclusive inside a TU? Richard. > >Segher
On Tue, 5 Jun 2018, Richard Biener wrote: > Are there other targets with similar kind of (FP) modes or is ppc the > only one with this 'legacy'? Also keep in mind that you intended to > backport this... Well, ia64 RFmode probably isn't a sensible mode to include in widening either (it's specifically disallowed for arithmetic operations). Before IRIX was obsoleted it used to use something like IBM long double as well (the support it used in fp-bit.[ch] still hasn't been removed).
This patch is a complete rework of the previous patch. Previously I used new
target hooks to provide IFmode (__ibm128) from being widened by default to
TFmode (long double) on power9 systems when long double is IEEE 128-bit.
This patch reorganizes the 3 128-bit floating point types, so that IFmode is
numerically higher than TFmode/KFmode. This means IFmode is considered the
widest type. Since we do not define arithmetic insns for IFmode, other than
negate/absolute value (that we define for the other types), we will not have
undesirable widening. I needed to change long double size so that lookup of
size would get the TFmode type and not the IFmode.
Since I reorganized the modes, the compiler now uses truncif{kf,tf}2 instead of
extend{tf,kf}if2. It turns out I had the argument modes backwards for trunc.
I have included a fix for this thinko.
I have built this on the following systems, and they bootstrapped and had no
regressions. Can I check this into the trunk, and after a burn-in period check
it into GCC 8.2, assuming the last build on power6 has no regressions?
1) Little endian power8 system (64-bit), --with-cpu=power8;
2) Big endian power8 system (64/32-bit), --with-cpu=power8;
3) Big endian power8 system (64/32-bit), no --with-cpu;
I'm currently building it on:
4) Big endian power6 system (64/32-bit), no --with-cpu.
2018-06-11 Michael Meissner <meissner@linux.ibm.com>
PR target/85358
* config/rs6000/rs6000-modes.def (toplevel): Rework the 128-bit
floating point modes, so that IFmode is numerically greater than
TFmode, which is greater than KFmode using FRACTIONAL_FLOAT_MODE
to declare the ordering. This prevents IFmode from being
converted to TFmode when long double is IEEE 128-bit on an ISA 3.0
machine. Include rs6000-modes.h to share the fractional values
between genmodes* and the rest of the compiler.
(IFmode): Likewise.
(KFmode): Likewise.
(TFmode): Likewise.
* config/rs6000/rs6000-modes.h: New file.
* config/rs6000/rs6000.c (rs6000_debug_reg_global): Change the
meaning of rs6000_long_double_size so that 126..128 selects an
appropriate 128-bit floating point type.
(rs6000_option_override_internal): Likewise.
* config/rs6000/rs6000.h (toplevel): Include rs6000-modes.h.
(TARGET_LONG_DOUBLE_128): Change the meaning of
rs6000_long_double_size so that 126..128 selects an appropriate
128-bit floating point type.
(LONG_DOUBLE_TYPE_SIZE): Update comment.
* config/rs6000/rs6000.md (trunciftf2): Correct the modes of the
source and destination to match the standard usage.
(truncifkf2): Likewise.
On Mon, Jun 11, 2018 at 07:31:44PM -0400, Michael Meissner wrote: > This patch is a complete rework of the previous patch. Previously I used new > target hooks to provide IFmode (__ibm128) from being widened by default to > TFmode (long double) on power9 systems when long double is IEEE 128-bit. > > This patch reorganizes the 3 128-bit floating point types, so that IFmode is > numerically higher than TFmode/KFmode. This means IFmode is considered the > widest type. Since we do not define arithmetic insns for IFmode, other than > negate/absolute value (that we define for the other types), we will not have > undesirable widening. I needed to change long double size so that lookup of > size would get the TFmode type and not the IFmode. > > Since I reorganized the modes, the compiler now uses truncif{kf,tf}2 instead of > extend{tf,kf}if2. It turns out I had the argument modes backwards for trunc. > I have included a fix for this thinko. > > I have built this on the following systems, and they bootstrapped and had no > regressions. Can I check this into the trunk, and after a burn-in period check > it into GCC 8.2, assuming the last build on power6 has no regressions? > > 1) Little endian power8 system (64-bit), --with-cpu=power8; > 2) Big endian power8 system (64/32-bit), --with-cpu=power8; > 3) Big endian power8 system (64/32-bit), no --with-cpu; > > I'm currently building it on: > > 4) Big endian power6 system (64/32-bit), no --with-cpu. Whoops, I forgot I also did the following and it had no regressions: 5) Little endian power9 system (64-bit), --with-cpu=power9
Hi! Many thanks for all your work on this. On Mon, Jun 11, 2018 at 07:31:44PM -0400, Michael Meissner wrote: > This patch is a complete rework of the previous patch. Previously I used new > target hooks to provide IFmode (__ibm128) from being widened by default to > TFmode (long double) on power9 systems when long double is IEEE 128-bit. > > This patch reorganizes the 3 128-bit floating point types, so that IFmode is > numerically higher than TFmode/KFmode. This means IFmode is considered the > widest type. Sneaky. It probably helps. I like it :-) > Since we do not define arithmetic insns for IFmode, other than > negate/absolute value (that we define for the other types), we will not have > undesirable widening. I don't understand this part. We _do_ need to have all the basic operations for IFmode. How else can __ibm128 variables work (with -mabi=ieeelongdouble)? > @@ -865,9 +872,8 @@ extern unsigned char rs6000_recip_bits[] > words. */ > #define DOUBLE_TYPE_SIZE 64 > > -/* A C expression for the size in bits of the type `long double' on > - the target machine. If you don't define this, the default is two > - words. */ > +/* A C expression for the size in bits of the type `long double' on the target > + machine. If you don't define this, the default is two words. */ Please don't change things that don't change anything. > --- gcc/config/rs6000/rs6000.md (revision 261349) > +++ gcc/config/rs6000/rs6000.md (working copy) > @@ -8159,8 +8159,8 @@ (define_expand "extendtfkf2" > }) > > (define_expand "trunciftf2" > - [(set (match_operand:IF 0 "gpc_reg_operand") > - (float_truncate:IF (match_operand:TF 1 "gpc_reg_operand")))] > + [(set (match_operand:TF 0 "gpc_reg_operand") > + (float_truncate:TF (match_operand:IF 1 "gpc_reg_operand")))] > "TARGET_FLOAT128_TYPE" > { > rs6000_expand_float128_convert (operands[0], operands[1], false); > @@ -8168,8 +8168,8 @@ (define_expand "trunciftf2" > }) > > (define_expand "truncifkf2" > - [(set (match_operand:IF 0 "gpc_reg_operand") > - (float_truncate:IF (match_operand:KF 1 "gpc_reg_operand")))] > + [(set (match_operand:KF 0 "gpc_reg_operand") > + (float_truncate:KF (match_operand:IF 1 "gpc_reg_operand")))] > "TARGET_FLOAT128_TYPE" > { > rs6000_expand_float128_convert (operands[0], operands[1], false); These bugfixes really should have been a separate patch. Please explain that IFmode arith part? Segher
On Thu, Jun 14, 2018 at 04:27:26PM -0500, Segher Boessenkool wrote: > Hi! > > Many thanks for all your work on this. > > On Mon, Jun 11, 2018 at 07:31:44PM -0400, Michael Meissner wrote: > > This patch is a complete rework of the previous patch. Previously I used new > > target hooks to provide IFmode (__ibm128) from being widened by default to > > TFmode (long double) on power9 systems when long double is IEEE 128-bit. > > > > This patch reorganizes the 3 128-bit floating point types, so that IFmode is > > numerically higher than TFmode/KFmode. This means IFmode is considered the > > widest type. > > Sneaky. It probably helps. I like it :-) > > > Since we do not define arithmetic insns for IFmode, other than > > negate/absolute value (that we define for the other types), we will not have > > undesirable widening. > > I don't understand this part. We _do_ need to have all the basic operations > for IFmode. How else can __ibm128 variables work (with -mabi=ieeelongdouble)? It calls the external function if the operation is not available. I.e. __gcc_qadd. We don't define addif3, and addtf3 is only defined if long double is IEEE and we are on power9. Basically when it wants to do something (such as an add) and there is no insn to support the operation, it goes up the chain of widder types to see if perhaps there is support in the wider mode (such as we do for 8/16/32-bit integer arithmetic). In this case, if the operation is not ABS or NEG, there is no IFmode operation, and the widening does not happen. Instead it calls the appropriate external function. Before the change, when IFmode had less precision than TFmode, it would widen IFmode to TFmode to do the operation, and then convert it back again. > > @@ -865,9 +872,8 @@ extern unsigned char rs6000_recip_bits[] > > words. */ > > #define DOUBLE_TYPE_SIZE 64 > > > > -/* A C expression for the size in bits of the type `long double' on > > - the target machine. If you don't define this, the default is two > > - words. */ > > +/* A C expression for the size in bits of the type `long double' on the target > > + machine. If you don't define this, the default is two words. */ > > Please don't change things that don't change anything. Ok, I think I had a longer comment, and then rewrote it. > > --- gcc/config/rs6000/rs6000.md (revision 261349) > > +++ gcc/config/rs6000/rs6000.md (working copy) > > @@ -8159,8 +8159,8 @@ (define_expand "extendtfkf2" > > }) > > > > (define_expand "trunciftf2" > > - [(set (match_operand:IF 0 "gpc_reg_operand") > > - (float_truncate:IF (match_operand:TF 1 "gpc_reg_operand")))] > > + [(set (match_operand:TF 0 "gpc_reg_operand") > > + (float_truncate:TF (match_operand:IF 1 "gpc_reg_operand")))] > > "TARGET_FLOAT128_TYPE" > > { > > rs6000_expand_float128_convert (operands[0], operands[1], false); > > @@ -8168,8 +8168,8 @@ (define_expand "trunciftf2" > > }) > > > > (define_expand "truncifkf2" > > - [(set (match_operand:IF 0 "gpc_reg_operand") > > - (float_truncate:IF (match_operand:KF 1 "gpc_reg_operand")))] > > + [(set (match_operand:KF 0 "gpc_reg_operand") > > + (float_truncate:KF (match_operand:IF 1 "gpc_reg_operand")))] > > "TARGET_FLOAT128_TYPE" > > { > > rs6000_expand_float128_convert (operands[0], operands[1], false); > > These bugfixes really should have been a separate patch. Why? Before hand they were never called, so it didn't matter that the arguments were wrong. Now that the order of the modes is different, it calls trunc instead of expand. > Please explain that IFmode arith part? See above.
What is the status of this patch? You never said it was either approved or suggested changes? https://gcc.gnu.org/ml/gcc-patches/2018-06/msg00634.html
On Thu, Jun 14, 2018 at 05:48:42PM -0400, Michael Meissner wrote: > On Thu, Jun 14, 2018 at 04:27:26PM -0500, Segher Boessenkool wrote: > > On Mon, Jun 11, 2018 at 07:31:44PM -0400, Michael Meissner wrote: > > > Since we do not define arithmetic insns for IFmode, other than > > > negate/absolute value (that we define for the other types), we will not have > > > undesirable widening. > > > > I don't understand this part. We _do_ need to have all the basic operations > > for IFmode. How else can __ibm128 variables work (with -mabi=ieeelongdouble)? > > It calls the external function if the operation is not available. > I.e. __gcc_qadd. We don't define addif3, and addtf3 is only defined if long > double is IEEE and we are on power9. Ah, so we *do* have the operations. But they are in optabs, not in define_insn or similar. Gotcha. And the expand code will not widen in this case. Please clarify this in whatever code comments or commit message applies. > > > --- gcc/config/rs6000/rs6000.md (revision 261349) > > > +++ gcc/config/rs6000/rs6000.md (working copy) > > > @@ -8159,8 +8159,8 @@ (define_expand "extendtfkf2" > > > }) > > > > > > (define_expand "trunciftf2" > > > - [(set (match_operand:IF 0 "gpc_reg_operand") > > > - (float_truncate:IF (match_operand:TF 1 "gpc_reg_operand")))] > > > + [(set (match_operand:TF 0 "gpc_reg_operand") > > > + (float_truncate:TF (match_operand:IF 1 "gpc_reg_operand")))] > > > "TARGET_FLOAT128_TYPE" > > > { > > > rs6000_expand_float128_convert (operands[0], operands[1], false); > > > @@ -8168,8 +8168,8 @@ (define_expand "trunciftf2" > > > }) > > > > > > (define_expand "truncifkf2" > > > - [(set (match_operand:IF 0 "gpc_reg_operand") > > > - (float_truncate:IF (match_operand:KF 1 "gpc_reg_operand")))] > > > + [(set (match_operand:KF 0 "gpc_reg_operand") > > > + (float_truncate:KF (match_operand:IF 1 "gpc_reg_operand")))] > > > "TARGET_FLOAT128_TYPE" > > > { > > > rs6000_expand_float128_convert (operands[0], operands[1], false); > > > > These bugfixes really should have been a separate patch. > > Why? Before hand they were never called, so it didn't matter that the > arguments were wrong. Now that the order of the modes is different, it calls > trunc instead of expand. It would help anyone unfortunate enough to have to bisect this in the future. And makes reviewing easier, too. Knowing what parts play together with what other parts helps a *lot*. (Note I said "should have", no need to change it anymore). OK for trunk if I didn't say so before (and for 8.2 together with the rest). Segher
Index: gcc/target.def =================================================================== --- gcc/target.def (revision 260550) +++ gcc/target.def (working copy) @@ -3498,6 +3498,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) + +DEFHOOK (libgcc_floating_mode_supported_p, "Define this to return nonzero if libgcc provides support for the \n\ floating-point mode @var{mode}, which is known to pass \n\ Index: gcc/targhooks.c =================================================================== --- gcc/targhooks.c (revision 260550) +++ gcc/targhooks.c (working copy) @@ -2345,4 +2345,12 @@ default_select_early_remat_modes (sbitma { } +bool +hook_bool_mode_mode_bool_true (machine_mode from_mode ATTRIBUTE_UNUSED, + machine_mode to_mode ATTRIBUTE_UNUSED, + bool unsigned_p ATTRIBUTE_UNUSED) +{ + return true; +} + #include "gt-targhooks.h" Index: gcc/targhooks.h =================================================================== --- gcc/targhooks.h (revision 260550) +++ gcc/targhooks.h (working copy) @@ -289,5 +289,6 @@ extern enum flt_eval_method default_excess_precision (enum excess_precision_type ATTRIBUTE_UNUSED); extern bool default_stack_clash_protection_final_dynamic_probe (rtx); extern void default_select_early_remat_modes (sbitmap); +extern bool hook_bool_mode_mode_bool_true (machine_mode, machine_mode, bool); #endif /* GCC_TARGHOOKS_H */ Index: gcc/optabs.c =================================================================== --- gcc/optabs.c (revision 260550) +++ gcc/optabs.c (working copy) @@ -1284,14 +1284,15 @@ expand_binop (machine_mode mode, optab b FOR_EACH_WIDER_MODE (wider_mode, mode) { machine_mode next_mode; - if (optab_handler (binoptab, wider_mode) != CODE_FOR_nothing - || (binoptab == smul_optab - && GET_MODE_WIDER_MODE (wider_mode).exists (&next_mode) - && (find_widening_optab_handler ((unsignedp - ? umul_widen_optab - : smul_widen_optab), - next_mode, mode) - != CODE_FOR_nothing))) + if ((optab_handler (binoptab, wider_mode) != CODE_FOR_nothing + || (binoptab == smul_optab + && GET_MODE_WIDER_MODE (wider_mode).exists (&next_mode) + && (find_widening_optab_handler ((unsignedp + ? umul_widen_optab + : smul_widen_optab), + next_mode, mode) + != CODE_FOR_nothing))) + && targetm.default_widening_p (mode, wider_mode, unsignedp)) { rtx xop0 = op0, xop1 = op1; int no_extend = 0; @@ -1834,9 +1835,10 @@ expand_binop (machine_mode mode, optab b gcc_assert (!convert_optab_p (binoptab)); FOR_EACH_WIDER_MODE (wider_mode, mode) { - if (optab_handler (binoptab, wider_mode) - || (methods == OPTAB_LIB - && optab_libfunc (binoptab, wider_mode))) + if ((optab_handler (binoptab, wider_mode) + || (methods == OPTAB_LIB + && optab_libfunc (binoptab, wider_mode))) + && targetm.default_widening_p (mode, wider_mode, unsignedp)) { rtx xop0 = op0, xop1 = op1; int no_extend = 0; @@ -1989,7 +1991,8 @@ expand_twoval_unop (optab unoptab, rtx o { FOR_EACH_WIDER_MODE (wider_mode, mode) { - if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing) + if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing + && targetm.default_widening_p (mode, wider_mode, unsignedp)) { rtx t0 = gen_reg_rtx (wider_mode); rtx t1 = gen_reg_rtx (wider_mode); @@ -2070,7 +2073,8 @@ expand_twoval_binop (optab binoptab, rtx { FOR_EACH_WIDER_MODE (wider_mode, mode) { - if (optab_handler (binoptab, wider_mode) != CODE_FOR_nothing) + if (optab_handler (binoptab, wider_mode) != CODE_FOR_nothing + && targetm.default_widening_p (mode, wider_mode, unsignedp)) { rtx t0 = gen_reg_rtx (wider_mode); rtx t1 = gen_reg_rtx (wider_mode); @@ -2169,7 +2173,9 @@ widen_leading (scalar_int_mode mode, rtx FOR_EACH_WIDER_MODE (wider_mode_iter, mode) { scalar_int_mode wider_mode = wider_mode_iter.require (); - if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing) + bool unsignedp = unoptab != clrsb_optab; + if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing + && targetm.default_widening_p (mode, wider_mode, unsignedp)) { rtx xop0, temp; rtx_insn *last; @@ -2178,10 +2184,8 @@ widen_leading (scalar_int_mode mode, rtx if (target == 0) target = gen_reg_rtx (mode); - xop0 = widen_operand (op0, wider_mode, mode, - unoptab != clrsb_optab, false); - temp = expand_unop (wider_mode, unoptab, xop0, NULL_RTX, - unoptab != clrsb_optab); + xop0 = widen_operand (op0, wider_mode, mode, unsignedp, false); + temp = expand_unop (wider_mode, unoptab, xop0, NULL_RTX, unsignedp); if (temp != 0) temp = expand_binop (wider_mode, sub_optab, temp, @@ -2333,9 +2337,12 @@ widen_bswap (scalar_int_mode mode, rtx o opt_scalar_int_mode wider_mode_iter; FOR_EACH_WIDER_MODE (wider_mode_iter, mode) - if (optab_handler (bswap_optab, wider_mode_iter.require ()) - != CODE_FOR_nothing) - break; + { + machine_mode wider_mode = wider_mode_iter.require (); + if (optab_handler (bswap_optab, wider_mode) != CODE_FOR_nothing + && targetm.default_widening_p (mode, wider_mode, true)) + break; + } if (!wider_mode_iter.exists ()) return NULL_RTX; @@ -2865,7 +2872,8 @@ expand_unop (machine_mode mode, optab un if (CLASS_HAS_WIDER_MODES_P (mclass)) FOR_EACH_WIDER_MODE (wider_mode, mode) { - if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing) + if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing + && targetm.default_widening_p (mode, wider_mode, unsignedp)) { rtx xop0 = op0; rtx_insn *last = get_last_insn (); @@ -3032,8 +3040,9 @@ expand_unop (machine_mode mode, optab un { FOR_EACH_WIDER_MODE (wider_mode, mode) { - if (optab_handler (unoptab, wider_mode) != CODE_FOR_nothing - || optab_libfunc (unoptab, wider_mode)) + if ((optab_handler (unoptab, wider_mode) != CODE_FOR_nothing + || optab_libfunc (unoptab, wider_mode)) + && targetm.default_widening_p (mode, wider_mode, unsignedp)) { rtx xop0 = op0; rtx_insn *last = get_last_insn (); Index: gcc/cse.c =================================================================== --- gcc/cse.c (revision 260550) +++ gcc/cse.c (working copy) @@ -4885,6 +4885,9 @@ cse_insn (rtx_insn *insn) if (GET_MODE_PRECISION (wider_mode) > BITS_PER_WORD) break; + if (!targetm.default_widening_p (mode, wider_mode, false)) + continue; + struct table_elt *const_elt = lookup (src_const, HASH (src_const, wider_mode), wider_mode); @@ -4924,6 +4927,9 @@ cse_insn (rtx_insn *insn) if (GET_MODE_SIZE (tmode) > UNITS_PER_WORD) break; + if (!targetm.default_widening_p (mode, tmode, false)) + continue; + rtx inner = gen_lowpart (tmode, XEXP (src, 0)); struct table_elt *larger_elt; @@ -4979,6 +4985,9 @@ cse_insn (rtx_insn *insn) if (GET_MODE_SIZE (tmode) > UNITS_PER_WORD) break; + if (!targetm.default_widening_p (mode, tmode, false)) + continue; + PUT_MODE (memory_extend_rtx, tmode); larger_elt = lookup (memory_extend_rtx, HASH (memory_extend_rtx, tmode), tmode); Index: gcc/combine.c =================================================================== --- gcc/combine.c (revision 260550) +++ gcc/combine.c (working copy) @@ -12965,6 +12965,9 @@ simplify_comparison (enum rtx_code code, || (nonzero_bits (op1, tmode) & ~GET_MODE_MASK (mode)) == 0))); + if (!targetm.default_widening_p (mode, tmode, zero_extended)) + continue; + if (zero_extended || ((num_sign_bit_copies (op0, tmode) > (unsigned int) (GET_MODE_PRECISION (tmode) Index: gcc/var-tracking.c =================================================================== --- gcc/var-tracking.c (revision 260550) +++ gcc/var-tracking.c (working copy) @@ -6347,10 +6347,14 @@ prepare_call_arguments (basic_block bb, opt_scalar_int_mode mode_iter; FOR_EACH_WIDER_MODE (mode_iter, mode) { + machine_mode old_mode = mode; mode = mode_iter.require (); if (GET_MODE_BITSIZE (mode) > BITS_PER_WORD) break; + if (!targetm.default_widening_p (old_mode, mode, false)) + continue; + rtx reg = simplify_subreg (mode, x, GET_MODE (x), 0); if (reg == NULL_RTX || !REG_P (reg)) continue; Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 260552) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -1973,6 +1973,9 @@ static const struct attribute_spec rs600 #undef TARGET_STARTING_FRAME_OFFSET #define TARGET_STARTING_FRAME_OFFSET rs6000_starting_frame_offset + +#undef TARGET_DEFAULT_WIDENING_P +#define TARGET_DEFAULT_WIDENING_P rs6000_default_widening_p /* Processor table. */ @@ -16502,6 +16505,29 @@ rs6000_init_builtins (void) #endif } +/* Return true if FROM_MODE can be widened to TO_MODE automatically. UNSIGNEDP + says the conversion is unsigned. + + On PowerPC, don't allow IBM extended double to widen to an IEEE 128-bit + floating point value or vice versa. */ + +static bool +rs6000_default_widening_p (machine_mode from_mode, + machine_mode to_mode, + bool unsignedp ATTRIBUTE_UNUSED) +{ + if (!TARGET_FLOAT128_TYPE) + return true; + + if (FLOAT128_IEEE_P (from_mode) && FLOAT128_IBM_P (to_mode)) + return false; + + if (FLOAT128_IBM_P (from_mode) && FLOAT128_IEEE_P (to_mode)) + return false; + + return true; +} + /* Returns the rs6000 builtin decl for CODE. */ static tree Index: gcc/doc/tm.texi =================================================================== --- gcc/doc/tm.texi (revision 260550) +++ gcc/doc/tm.texi (working copy) @@ -4315,6 +4315,11 @@ If this hook allows @code{val} to have a @code{int8x8x3_t}s in registers rather than forcing them onto the stack. @end deftypefn +@deftypefn {Target Hook} bool TARGET_DEFAULT_WIDENING_P (machine_mode, @var{machine_mode}, @var{bool}) +Return true if GCC can automatically widen from @var{from_mode} to +@var{to_mode}. Conversions are unsigned if @var{unsigned_p} is true. +@end deftypefn + @deftypefn {Target Hook} bool TARGET_LIBGCC_FLOATING_MODE_SUPPORTED_P (scalar_float_mode @var{mode}) Define this to return nonzero if libgcc provides support for the floating-point mode @var{mode}, which is known to pass Index: gcc/doc/tm.texi.in =================================================================== --- gcc/doc/tm.texi.in (revision 260550) +++ gcc/doc/tm.texi.in (working copy) @@ -3344,6 +3344,8 @@ stack. @hook TARGET_ARRAY_MODE_SUPPORTED_P +@hook TARGET_DEFAULT_WIDENING_P + @hook TARGET_LIBGCC_FLOATING_MODE_SUPPORTED_P @hook TARGET_FLOATN_MODE Index: gcc/testsuite/gcc.target/powerpc/pr85358.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/pr85358.c (revision 0) +++ gcc/testsuite/gcc.target/powerpc/pr85358.c (working copy) @@ -0,0 +1,14 @@ +/* { dg-do compile { target { powerpc*-*-linux* && lp64 } } } */ +/* { dg-require-effective-target powerpc_p9vector_ok } */ +/* { dg-options "-mpower9-vector -mfloat128 -O2 -mabi=ieeelongdouble -Wno-psabi" } */ + +/* Verify that __ibm128 does not get converted automatically to IEEE 128-bit on + machines with IEEE 128-bit hardware support. */ + +__ibm128 +add (__ibm128 a, __ibm128 b) +{ + return a + b; +} + +/* { dg-final { scan-assembler-not {\mxsaddqp\M} } } */
I posted this patch at the end of GCC 8 as a RFC. Now that we are in GCC 9, I would like to repose it. Sorry to spam some of you. It is unclear whom the reviewers for things like target hooks and basic mode handling are. Here is the original patch. https://gcc.gnu.org/ml/gcc-patches/2018-04/msg00764.html PowerPC has 3 different 128-bit floating point types (KFmode, IFmode, and TFmode). We are in the process of migrating long double from IBM extended double to IEEE 128-bit floating point. * IFmode is IBM extended double (__ibm128) * KFmode is IEEE 128-bit floating point (__float128, _Float128N) * TFmode is whatever long double maps to If we are compiling for a power8 system (which does not have hardware IEEE 128-bit floating point), the current system works because each of the 3 modes do not have hardware support. If we are compiling for a power9 system and long double is IBM extended double, again things are fine. However, if we compiling for power9 and we've flipped the default for long double to be IEEE 128-bit floating point, then the code to support __ibm128 breaks. The machine independent portions of the mode handling says oh, there is hardware to support TFmode operations, lets widen the type to TFmode and do those operations. However, converting IFmode to TFmode is not cheap, it has to be done in a function call. This patch adds a new target hook, that if it is overriden, the backend can say don't automatically widen this type to that type. The PowerPC port defines the target hook so that it doesn't automatically convert IBM extended double to IEEE 128-bit and vice versa. This patch goes through all of the places that calls GET_MODE_WIDER_MODE and then calls the target hook. Now, the PowerPC only needs to block certain floating point widenings. Several of the changes are to integer widenings, and if desired, we could restrict the changes to just floating point types. However, there might be other ports that need the flexibility for other types. I have tried various other approprches to fix this problem, and so far, I have not been able to come up with a PowerPC back-end only solution that works. Alternatively, Segher has suggested that the call to the target hook be in GET_MODE_WIDER_MODE and GET_MODE_2XWIDER_MODE (plus any places where we access the mode_wider array direcly). I have built little endian PowerPC builds with this patch, and I have verified that it does work. I have tested the same patch in April on a big endian PowerPC system and x86_64 and it worked there also. Can I check in this patch as is (I will verify x86/PowerPC big endian still works before checkin). Or would people prefer modifications to the patch? [gcc] 2018-05-22 Michael Meissner <meissner@linux.vnet.ibm.com> PR target/85358 * target.def (default_widening_p): New target hook to say whether default widening between modes should be done. * targhooks.h (hook_bool_mode_mode_bool_true): New declaration. * targhooks.c (hook_bool_mode_mode_bool_true): New default target hook. * optabs.c (expand_binop): Before doing default widening, check whether the backend allows the widening. (expand_twoval_unop): Likewise. (expand_twoval_binop): Likewise. (widen_leading): Likewise. (widen_bswap): Likewise. (expand_unop): Likewise. * cse.c (cse_insn): Likewise. * combine.c (simplify_comparison): Likewise. * var-tracking.c (prepare_call_arguments): Likewise. * config/rs6000/rs6000.c (TARGET_DEFAULT_WIDENING_P): Define target hook to prevent IBM extended double and IEEE 128-bit floating point from being converted to each by default. (rs6000_default_widening_p): Likewise. * doc/tm.texi (TARGET_DEFAULT_WIDENING_P): Document the new default widening hook. * doc/tm.texi.in (TARGET_DEFAULT_WIDENING_P): Likewise. [gcc/testsuite] 2018-05-22 Michael Meissner <meissner@linux.vnet.ibm.com> PR target/85358 * gcc.target/powerpc/pr85358.c: New test to make sure __ibm128 does not widen to __float128 on ISA 3.0 systems. In order to start the transition of PowerPC long double to IEEE 128-bit, we will need this patch or a similar patch to be back ported to GCC 8.2.