* [PATCH], PowerPC support to enable -mlra and/or -mfloat128 @ 2016-07-11 20:07 Michael Meissner 2016-07-11 21:25 ` Michael Meissner 2016-07-12 10:29 ` [PATCH], " Richard Biener 0 siblings, 2 replies; 17+ messages in thread From: Michael Meissner @ 2016-07-11 20:07 UTC (permalink / raw) To: gcc-patches, Segher Boessenkool, David Edelsohn, Bill Schmidt These configuration switches will allow the PowerPC GCC developers to switch defaults in the compiler to debug the code, before making the decision to flip the default permanently. In the future, when the defaults have been changed, these configuration options would allow developers to go back to the previous versions without modifying the code using the --disable-<switch> form. The first option is --enable-lra, which changes the compiler so that the default is to use the LRA register allocator instead of the older RELOAD allocator. The PowerPC team would like to switch the default, but there is a critical bug in LRA that must be fixed before we can change the default: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69847 The second option is --enable-float128, which changes the compiler so that the default for VSX systems is to enable the __float128 keyword to allow users access to the IEEE 128-bit floating point implementation without having to use the keyword. Both of these switches are debug switches, and are not meant to be used by non-developers. The --enable-lra swich causes the following tests to fail: * testsuite/gcc.target/powerpc/bool3-p7.c * testsuite/gcc.target/powerpc/bool3-p8.c See bug 71846 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71846) for more details. The --enable-float128 switch causes libquadmath and libstdc++ to fail because we do not yet have enough of the support in the compiler to allow these libraries to build. It is our intention, that we will use the --enable-float128 option and work on getting the libraries fixed. If I build just a C compiler and disable building libquadmath, there are no regressions in the C tests with __float128 enabled or disabled. Can I check these options into the trunk as non-default options? -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128 2016-07-11 20:07 [PATCH], PowerPC support to enable -mlra and/or -mfloat128 Michael Meissner @ 2016-07-11 21:25 ` Michael Meissner 2016-07-20 17:04 ` [PATCH #2], " Michael Meissner 2016-07-12 10:29 ` [PATCH], " Richard Biener 1 sibling, 1 reply; 17+ messages in thread From: Michael Meissner @ 2016-07-11 21:25 UTC (permalink / raw) To: Michael Meissner, gcc-patches, Segher Boessenkool, David Edelsohn, Bill Schmidt [-- Attachment #1: Type: text/plain, Size: 729 bytes --] Sigh, I keep forgetting to attach the patch. 2016-07-11 Michael Meissner <meissner@linux.vnet.ibm.com> * doc/install.texi (Configuration): Document PowerPC specific configuration options --enable-lra and --enable-float128. * configure.ac: Add --enable-lra and --enable-float128 to turn on -mlra and -mfloat128 by default in the PowerPC compiler. * configure: Regenerate. * config.gcc (powerpc*-*-linux*): Add --enable-lra and --enable-float128 support. * config/rs6000/rs6000.c (rs6000_option_override_internal): Add support for --enable-lra and --enable-float128. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797 [-- Attachment #2: gcc-power9.patch146b --] [-- Type: text/plain, Size: 5636 bytes --] Index: gcc/doc/install.texi =================================================================== --- gcc/doc/install.texi (revision 238170) +++ gcc/doc/install.texi (working copy) @@ -1661,6 +1661,26 @@ Using the GNU Compiler Collection (GCC)} See ``RS/6000 and PowerPC Options'' in the main manual @end ifhtml +@item --enable-lra +This option enables @option{-mlra} by default for powerpc-linux. +@ifnothtml +@xref{RS/6000 and PowerPC Options,, RS/6000 and PowerPC Options, gcc, +Using the GNU Compiler Collection (GCC)}, +@end ifnothtml +@ifhtml +See ``RS/6000 and PowerPC Options'' in the main manual +@end ifhtml + +@item --enable-float128 +This option enables @option{-mfloat128} by default for powerpc-linux. +@ifnothtml +@xref{RS/6000 and PowerPC Options,, RS/6000 and PowerPC Options, gcc, +Using the GNU Compiler Collection (GCC)}, +@end ifnothtml +@ifhtml +See ``RS/6000 and PowerPC Options'' in the main manual +@end ifhtml + @item --enable-default-ssp Turn on @option{-fstack-protector-strong} by default. Index: gcc/configure =================================================================== --- gcc/configure (revision 238170) +++ gcc/configure (working copy) @@ -918,6 +918,8 @@ enable_rpath with_libiconv_prefix enable_sjlj_exceptions enable_secureplt +enable_lra +enable_float128 enable_leading_mingw64_underscores enable_cld enable_frame_pointer @@ -1630,6 +1632,9 @@ Optional Features: --enable-sjlj-exceptions arrange to use setjmp/longjmp exception handling --enable-secureplt enable -msecure-plt by default for PowerPC + --enable-lra enable -mlra by default for PowerPC + --enable-float128 enable -mfloat128 by default for PowerPC on Linux + VSX systems --enable-leading-mingw64-underscores enable leading underscores on 64 bit mingw targets --enable-cld enable -mcld by default for 32bit x86 @@ -11984,6 +11989,18 @@ if test "${enable_secureplt+set}" = set; fi +# Check whether --enable-lra was given. +if test "${enable_lra+set}" = set; then : + enableval=$enable_lra; +fi + + +# Check whether --enable-float128 was given. +if test "${enable_float128+set}" = set; then : + enableval=$enable_float128; +fi + + # Check whether --enable-leading-mingw64-underscores was given. if test "${enable_leading_mingw64_underscores+set}" = set; then : enableval=$enable_leading_mingw64_underscores; @@ -18475,7 +18492,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 18478 "configure" +#line 18495 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -18581,7 +18598,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 18584 "configure" +#line 18601 "configure" #include "confdefs.h" #if HAVE_DLFCN_H Index: gcc/configure.ac =================================================================== --- gcc/configure.ac (revision 238170) +++ gcc/configure.ac (working copy) @@ -1798,6 +1798,17 @@ AC_ARG_ENABLE(secureplt, [enable -msecure-plt by default for PowerPC])], [], []) +AC_ARG_ENABLE(lra, +[AS_HELP_STRING([--enable-lra], + [enable -mlra by default for PowerPC])], +[], []) + +AC_ARG_ENABLE(float128, +[AS_HELP_STRING([--enable-float128], + [enable -mfloat128 by default for PowerPC on Linux VSX + systems])], +[], []) + AC_ARG_ENABLE(leading-mingw64-underscores, AS_HELP_STRING([--enable-leading-mingw64-underscores], [enable leading underscores on 64 bit mingw targets]), Index: gcc/config.gcc =================================================================== --- gcc/config.gcc (revision 238170) +++ gcc/config.gcc (working copy) @@ -2414,6 +2414,12 @@ powerpc*-*-linux*) if test x${enable_secureplt} = xyes; then tm_file="rs6000/secureplt.h ${tm_file}" fi + if test x${enable_lra} = xyes; then + tm_defines="${tm_defines} ENABLE_LRA=1" + fi + if test x${enable_float128} = xyes; then + tm_defines="${tm_defines} ENABLE_FLOAT128=1" + fi ;; powerpc-wrs-vxworks|powerpc-wrs-vxworksae|powerpc-wrs-vxworksmils) tm_file="${tm_file} elfos.h freebsd-spec.h rs6000/sysv4.h" Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 238170) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -4306,6 +4306,17 @@ rs6000_option_override_internal (bool gl rs6000_isa_flags &= ~OPTION_MASK_P9_VECTOR; } + /* Enable LRA if the compiler was configured with --enable-lra. */ +#ifdef ENABLE_LRA + if ((rs6000_isa_flags_explicit & OPTION_MASK_LRA) == 0) + { + if (ENABLE_LRA) + rs6000_isa_flags |= OPTION_MASK_LRA; + else + rs6000_isa_flags &= ~OPTION_MASK_LRA; + } +#endif + /* There have been bugs with -mvsx-timode that don't show up with -mlra, but do show up with -mno-lra. Given -mlra will become the default once PR 69847 is fixed, turn off the options with problems by default if @@ -4372,6 +4383,17 @@ rs6000_option_override_internal (bool gl } } + /* Enable FLOAT128 if the compiler was configured with --enable-float128. */ +#ifdef ENABLE_FLOAT128 + if (TARGET_VSX && (rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128) == 0) + { + if (ENABLE_FLOAT128) + rs6000_isa_flags |= OPTION_MASK_FLOAT128; + else + rs6000_isa_flags &= ~(OPTION_MASK_FLOAT128 | OPTION_MASK_FLOAT128_HW); + } +#endif + /* __float128 requires VSX support. */ if (TARGET_FLOAT128 && !TARGET_VSX) { ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH #2], PowerPC support to enable -mlra and/or -mfloat128 2016-07-11 21:25 ` Michael Meissner @ 2016-07-20 17:04 ` Michael Meissner 0 siblings, 0 replies; 17+ messages in thread From: Michael Meissner @ 2016-07-20 17:04 UTC (permalink / raw) To: Michael Meissner, gcc-patches, Segher Boessenkool, David Edelsoh, Bill Schmidt, Richard Biener, Bernd Schmidt Cc: gcc-patches, Segher Boessenkool, David Edelsohn, Bill Schmidt [-- Attachment #1: Type: text/plain, Size: 1570 bytes --] This patch renames the configure switches to be explicit that they are for the PowerPC, and that they are temporary. I would hope by the time GCC 7 exits stage1 that these switches will be removed, but having them now will allow us to move to LRA and __float128 in an orderly fashion. I built a bootstrap compiler using the --enable-powerpc-lra option, and it ran fine. There were two additional tests that generate different code with -mlra and now fail. These will be fixed in later patches. I also built a C only compiler using the --enable-powerpc-float128 option (disabling libquadmath and bootstrap), and the C tests looked fine. Can I install these patches in the trunk? 2016-07-20 Michael Meissner <meissner@linux.vnet.ibm.com> * doc/install.texi (Configuration): Document PowerPC specific configuration options --enable-powerpc-lra and --enable-powerpc-float128. * configure.ac: Add support for the configuration option --enable-powerpc-lra to enable the use of the LRA register allocator by default. Add support for the configuration option --enable-powerpc-float128 to enable the use of the __float128 type in PowerPC Linux systems. * configure: Regenerate. * config.gcc (powerpc*-*-linux*): Add --enable-powerpc-lra and --enable-powerpc-float128 support. * config/rs6000/rs6000.c (rs6000_option_override_internal): Add support for --enable-powerpc-lra and --enable-powerpc-float128. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797 [-- Attachment #2: gcc-stage7.config001b --] [-- Type: text/plain, Size: 3122 bytes --] Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (.../svn+ssh://meissner@gcc.gnu.org/svn/gcc/trunk/gcc/config/rs6000) (revision 238445) +++ gcc/config/rs6000/rs6000.c (.../gcc/config/rs6000) (working copy) @@ -4306,6 +4306,17 @@ rs6000_option_override_internal (bool gl rs6000_isa_flags &= ~OPTION_MASK_P9_DFORM_SCALAR; } + /* Enable LRA if the compiler was configured with --enable-lra. */ +#ifdef ENABLE_LRA + if ((rs6000_isa_flags_explicit & OPTION_MASK_LRA) == 0) + { + if (ENABLE_LRA) + rs6000_isa_flags |= OPTION_MASK_LRA; + else + rs6000_isa_flags &= ~OPTION_MASK_LRA; + } +#endif + /* There have been bugs with -mvsx-timode that don't show up with -mlra, but do show up with -mno-lra. Given -mlra will become the default once PR 69847 is fixed, turn off the options with problems by default if @@ -4372,6 +4383,17 @@ rs6000_option_override_internal (bool gl } } + /* Enable FLOAT128 if the compiler was configured with --enable-float128. */ +#ifdef ENABLE_FLOAT128 + if (TARGET_VSX && (rs6000_isa_flags_explicit & OPTION_MASK_FLOAT128) == 0) + { + if (ENABLE_FLOAT128) + rs6000_isa_flags |= OPTION_MASK_FLOAT128; + else + rs6000_isa_flags &= ~(OPTION_MASK_FLOAT128 | OPTION_MASK_FLOAT128_HW); + } +#endif + /* __float128 requires VSX support. */ if (TARGET_FLOAT128 && !TARGET_VSX) { Index: gcc/doc/install.texi =================================================================== --- gcc/doc/install.texi (.../svn+ssh://meissner@gcc.gnu.org/svn/gcc/trunk/gcc/doc) (revision 238445) +++ gcc/doc/install.texi (.../gcc/doc) (working copy) @@ -1661,6 +1661,35 @@ Using the GNU Compiler Collection (GCC)} See ``RS/6000 and PowerPC Options'' in the main manual @end ifhtml +@item --enable-powerpc-lra +This option enables @option{-mlra} by default for powerpc-linux. This +switch is a temporary configuration switch that is intended to allow +for the transition from the reload register allocator to the newer lra +register allocator. When the transition is complete, this switch +may be deleted. +@ifnothtml +@xref{RS/6000 and PowerPC Options,, RS/6000 and PowerPC Options, gcc, +Using the GNU Compiler Collection (GCC)}, +@end ifnothtml +@ifhtml +See ``RS/6000 and PowerPC Options'' in the main manual +@end ifhtml + +@item --enable-powerpc-float128 +This option enables @option{-mfloat128} by default for powerpc-linux. +This switch is a temporary configuation switch that is intended to +allow the PowerPC GCC developers to work on implementing library +support for PowerPC IEEE 128-bit floating point functions. When the +standard GCC libraries are enhanced to support @code{__float128} by +default, this switch may be deleted. +@ifnothtml +@xref{RS/6000 and PowerPC Options,, RS/6000 and PowerPC Options, gcc, +Using the GNU Compiler Collection (GCC)}, +@end ifnothtml +@ifhtml +See ``RS/6000 and PowerPC Options'' in the main manual +@end ifhtml + @item --enable-default-ssp Turn on @option{-fstack-protector-strong} by default. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128 2016-07-11 20:07 [PATCH], PowerPC support to enable -mlra and/or -mfloat128 Michael Meissner 2016-07-11 21:25 ` Michael Meissner @ 2016-07-12 10:29 ` Richard Biener 2016-07-12 11:19 ` Segher Boessenkool 2016-07-12 11:21 ` Bernd Schmidt 1 sibling, 2 replies; 17+ messages in thread From: Richard Biener @ 2016-07-12 10:29 UTC (permalink / raw) To: Michael Meissner, GCC Patches, Segher Boessenkool, David Edelsohn, Bill Schmidt On Mon, Jul 11, 2016 at 10:07 PM, Michael Meissner <meissner@linux.vnet.ibm.com> wrote: > These configuration switches will allow the PowerPC GCC developers to switch > defaults in the compiler to debug the code, before making the decision to flip > the default permanently. In the future, when the defaults have been changed, > these configuration options would allow developers to go back to the previous > versions without modifying the code using the --disable-<switch> form. > > The first option is --enable-lra, which changes the compiler so that the > default is to use the LRA register allocator instead of the older RELOAD > allocator. The PowerPC team would like to switch the default, but there is a > critical bug in LRA that must be fixed before we can change the default: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69847 > > The second option is --enable-float128, which changes the compiler so that the > default for VSX systems is to enable the __float128 keyword to allow users > access to the IEEE 128-bit floating point implementation without having to use > the keyword. > > Both of these switches are debug switches, and are not meant to be used by > non-developers. > > The --enable-lra swich causes the following tests to fail: > > * testsuite/gcc.target/powerpc/bool3-p7.c > * testsuite/gcc.target/powerpc/bool3-p8.c > > See bug 71846 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71846) for more > details. > > The --enable-float128 switch causes libquadmath and libstdc++ to fail because > we do not yet have enough of the support in the compiler to allow these > libraries to build. It is our intention, that we will use the > --enable-float128 option and work on getting the libraries fixed. If I build > just a C compiler and disable building libquadmath, there are no regressions in > the C tests with __float128 enabled or disabled. > > Can I check these options into the trunk as non-default options? Instead of adding more configury can we please enable LRA on trunk by default _now_? Otherwise the amount of testing it recieves won't really increase. Richard. > > -- > Michael Meissner, IBM > IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA > email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797 > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128 2016-07-12 10:29 ` [PATCH], " Richard Biener @ 2016-07-12 11:19 ` Segher Boessenkool 2016-07-12 11:21 ` Bernd Schmidt 1 sibling, 0 replies; 17+ messages in thread From: Segher Boessenkool @ 2016-07-12 11:19 UTC (permalink / raw) To: Richard Biener Cc: Michael Meissner, GCC Patches, David Edelsohn, Bill Schmidt On Tue, Jul 12, 2016 at 12:29:04PM +0200, Richard Biener wrote: > Instead of adding more configury can we please enable LRA on trunk by default > _now_? Otherwise the amount of testing it recieves won't really increase. I agree we should change default_lra_p to return true instead of false. However, that won't change what the rs6000 port uses (it has its own implementation of that hook; the uses can select LRA vs. reload with -mlra). Other ports that have LRA selectable are in the same boat. For ports that always use LRA, everything works no matter what. For rs6000, we cannot change just yet, the performance regressions are just too big. Segher ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128 2016-07-12 10:29 ` [PATCH], " Richard Biener 2016-07-12 11:19 ` Segher Boessenkool @ 2016-07-12 11:21 ` Bernd Schmidt 2016-07-12 11:31 ` Richard Biener 2016-07-12 19:44 ` Michael Meissner 1 sibling, 2 replies; 17+ messages in thread From: Bernd Schmidt @ 2016-07-12 11:21 UTC (permalink / raw) To: Richard Biener, Michael Meissner, GCC Patches, Segher Boessenkool, David Edelsohn, Bill Schmidt On 07/12/2016 12:29 PM, Richard Biener wrote: > Instead of adding more configury can we please enable LRA on trunk by default > _now_? Otherwise the amount of testing it recieves won't really increase. I hope you mean for ppc only, otherwise you're breaking a lot of ports... Bernd ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128 2016-07-12 11:21 ` Bernd Schmidt @ 2016-07-12 11:31 ` Richard Biener 2016-07-12 12:16 ` Segher Boessenkool 2016-07-12 19:44 ` Michael Meissner 1 sibling, 1 reply; 17+ messages in thread From: Richard Biener @ 2016-07-12 11:31 UTC (permalink / raw) To: Bernd Schmidt Cc: Michael Meissner, GCC Patches, Segher Boessenkool, David Edelsohn, Bill Schmidt On Tue, Jul 12, 2016 at 1:21 PM, Bernd Schmidt <bschmidt@redhat.com> wrote: > On 07/12/2016 12:29 PM, Richard Biener wrote: > >> Instead of adding more configury can we please enable LRA on trunk by >> default >> _now_? Otherwise the amount of testing it recieves won't really increase. > > > I hope you mean for ppc only, otherwise you're breaking a lot of ports... Of course. Simply add Init(1) to rs6000.opt mlra. Richard. > > Bernd ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128 2016-07-12 11:31 ` Richard Biener @ 2016-07-12 12:16 ` Segher Boessenkool 2016-07-12 12:40 ` Bernd Schmidt 0 siblings, 1 reply; 17+ messages in thread From: Segher Boessenkool @ 2016-07-12 12:16 UTC (permalink / raw) To: Richard Biener Cc: Bernd Schmidt, Michael Meissner, GCC Patches, David Edelsohn, Bill Schmidt On Tue, Jul 12, 2016 at 01:31:35PM +0200, Richard Biener wrote: > On Tue, Jul 12, 2016 at 1:21 PM, Bernd Schmidt <bschmidt@redhat.com> wrote: > > On 07/12/2016 12:29 PM, Richard Biener wrote: > > > >> Instead of adding more configury can we please enable LRA on trunk by > >> default > >> _now_? Otherwise the amount of testing it recieves won't really increase. > > > > > > I hope you mean for ppc only, otherwise you're breaking a lot of ports... > > Of course. Simply add Init(1) to rs6000.opt mlra. This is blocked on PR69847. The plan is still to enable it this stage 1. How can LRA ever be made default for all targets without breaking those that do not want to change? Segher ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128 2016-07-12 12:16 ` Segher Boessenkool @ 2016-07-12 12:40 ` Bernd Schmidt 2016-07-12 12:45 ` Segher Boessenkool 2016-07-12 12:55 ` Segher Boessenkool 0 siblings, 2 replies; 17+ messages in thread From: Bernd Schmidt @ 2016-07-12 12:40 UTC (permalink / raw) To: Segher Boessenkool, Richard Biener Cc: Michael Meissner, GCC Patches, David Edelsohn, Bill Schmidt On 07/12/2016 02:15 PM, Segher Boessenkool wrote: > How can LRA ever be made default for all targets without breaking those > that do not want to change? LRA advocates would have to fix the issues with it that prevent it from being usable on certain targets. Based on my initial experiments with Blackfin, and LRA review comments I had based on c6x requirements, I suspect register elimination will have to be rewritten to start with. Bernd ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128 2016-07-12 12:40 ` Bernd Schmidt @ 2016-07-12 12:45 ` Segher Boessenkool 2016-07-12 12:48 ` Bernd Schmidt 2016-07-12 12:55 ` Segher Boessenkool 1 sibling, 1 reply; 17+ messages in thread From: Segher Boessenkool @ 2016-07-12 12:45 UTC (permalink / raw) To: Bernd Schmidt Cc: Richard Biener, Michael Meissner, GCC Patches, David Edelsohn, Bill Schmidt On Tue, Jul 12, 2016 at 02:40:28PM +0200, Bernd Schmidt wrote: > On 07/12/2016 02:15 PM, Segher Boessenkool wrote: > >How can LRA ever be made default for all targets without breaking those > >that do not want to change? > > LRA advocates would have to fix the issues with it that prevent it from > being usable on certain targets. Based on my initial experiments with > Blackfin, and LRA review comments I had based on c6x requirements, I > suspect register elimination will have to be rewritten to start with. Do you have PR #s for those issues? Segher ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128 2016-07-12 12:45 ` Segher Boessenkool @ 2016-07-12 12:48 ` Bernd Schmidt 2016-07-12 15:17 ` Bernd Schmidt 0 siblings, 1 reply; 17+ messages in thread From: Bernd Schmidt @ 2016-07-12 12:48 UTC (permalink / raw) To: Segher Boessenkool Cc: Richard Biener, Michael Meissner, GCC Patches, David Edelsohn, Bill Schmidt On 07/12/2016 02:45 PM, Segher Boessenkool wrote: > On Tue, Jul 12, 2016 at 02:40:28PM +0200, Bernd Schmidt wrote: >> On 07/12/2016 02:15 PM, Segher Boessenkool wrote: >>> How can LRA ever be made default for all targets without breaking those >>> that do not want to change? >> >> LRA advocates would have to fix the issues with it that prevent it from >> being usable on certain targets. Based on my initial experiments with >> Blackfin, and LRA review comments I had based on c6x requirements, I >> suspect register elimination will have to be rewritten to start with. > > Do you have PR #s for those issues? No. You can reproduce issues with Blackfin easily by enabling LRA for it, and I described the C6X issues when the LRA patches were posted for review. Bernd ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128 2016-07-12 12:48 ` Bernd Schmidt @ 2016-07-12 15:17 ` Bernd Schmidt 2016-07-12 16:31 ` Segher Boessenkool 0 siblings, 1 reply; 17+ messages in thread From: Bernd Schmidt @ 2016-07-12 15:17 UTC (permalink / raw) To: Segher Boessenkool Cc: Richard Biener, Michael Meissner, GCC Patches, David Edelsohn, Bill Schmidt On 07/12/2016 02:48 PM, Bernd Schmidt wrote: > No. You can reproduce issues with Blackfin easily by enabling LRA for > it, and I described the C6X issues when the LRA patches were posted for > review. That was here: https://gcc.gnu.org/ml/gcc-patches/2012-10/msg00235.html The Blackfin thing happens frequently with -fomit-frame-pointer when we have (insn 66 64 70 8 (set (reg:SI 96 [ ivtmp.32 ]) (reg/f:SI 15 FP)) 19 {*movsi_insn} Which LRA transforms to an invalid insn: (insn 66 64 70 8 (set (reg:SI 15 FP [orig:96 ivtmp.32 ] [96]) (plus:SI (reg/f:SI 14 SP) (const_int 4 [0x4]))) 50 {addsi3} (expr_list:REG_EQUAL (reg/f:SI 15 FP) (nil))) Haven't fully debugged it but it looks like an instance of the same problem: not using the correct register numbers in elimination. An FP+FP addition would be fine (which is how I'm guessing we arrived at this pattern), but once you substitute the real register number you get an invalid insn. So LRA is somewhat defective in this area. Bernd ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128 2016-07-12 15:17 ` Bernd Schmidt @ 2016-07-12 16:31 ` Segher Boessenkool 2016-07-12 16:57 ` Bernd Schmidt 0 siblings, 1 reply; 17+ messages in thread From: Segher Boessenkool @ 2016-07-12 16:31 UTC (permalink / raw) To: Bernd Schmidt Cc: Richard Biener, Michael Meissner, GCC Patches, David Edelsohn, Bill Schmidt On Tue, Jul 12, 2016 at 05:17:04PM +0200, Bernd Schmidt wrote: > The Blackfin thing happens frequently with -fomit-frame-pointer when we have > > (insn 66 64 70 8 (set (reg:SI 96 [ ivtmp.32 ]) > (reg/f:SI 15 FP)) 19 {*movsi_insn} > > Which LRA transforms to an invalid insn: > > (insn 66 64 70 8 (set (reg:SI 15 FP [orig:96 ivtmp.32 ] [96]) > (plus:SI (reg/f:SI 14 SP) > (const_int 4 [0x4]))) 50 {addsi3} > (expr_list:REG_EQUAL (reg/f:SI 15 FP) > (nil))) > > Haven't fully debugged it but it looks like an instance of the same > problem: not using the correct register numbers in elimination. An FP+FP > addition would be fine (which is how I'm guessing we arrived at this > pattern), but once you substitute the real register number you get an > invalid insn. So LRA is somewhat defective in this area. Do you have a testcase? This sounds like fun :-) Segher ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128 2016-07-12 16:31 ` Segher Boessenkool @ 2016-07-12 16:57 ` Bernd Schmidt 0 siblings, 0 replies; 17+ messages in thread From: Bernd Schmidt @ 2016-07-12 16:57 UTC (permalink / raw) To: Segher Boessenkool Cc: Richard Biener, Michael Meissner, GCC Patches, David Edelsohn, Bill Schmidt On 07/12/2016 06:31 PM, Segher Boessenkool wrote: > > Do you have a testcase? This sounds like fun :-) This is bfin-elf, gcc.c-torture/compile/pr59417.c, -O3 -fomit-frame-pointer. Bernd ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128 2016-07-12 12:40 ` Bernd Schmidt 2016-07-12 12:45 ` Segher Boessenkool @ 2016-07-12 12:55 ` Segher Boessenkool 1 sibling, 0 replies; 17+ messages in thread From: Segher Boessenkool @ 2016-07-12 12:55 UTC (permalink / raw) To: Bernd Schmidt Cc: Richard Biener, Michael Meissner, GCC Patches, David Edelsohn, Bill Schmidt On Tue, Jul 12, 2016 at 02:40:28PM +0200, Bernd Schmidt wrote: > On 07/12/2016 02:15 PM, Segher Boessenkool wrote: > >How can LRA ever be made default for all targets without breaking those > >that do not want to change? > > LRA advocates would have to fix the issues with it that prevent it from > being usable on certain targets. That would be the case if the plan was to remove reload. But the current proposal is only to change the default, and the affected targets can easily implement the hook (to say "no") if LRA won't work for them. Segher ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128 2016-07-12 11:21 ` Bernd Schmidt 2016-07-12 11:31 ` Richard Biener @ 2016-07-12 19:44 ` Michael Meissner 2016-07-12 21:20 ` Segher Boessenkool 1 sibling, 1 reply; 17+ messages in thread From: Michael Meissner @ 2016-07-12 19:44 UTC (permalink / raw) To: Bernd Schmidt Cc: Richard Biener, Michael Meissner, GCC Patches, Segher Boessenkool, David Edelsohn, Bill Schmidt On Tue, Jul 12, 2016 at 01:21:47PM +0200, Bernd Schmidt wrote: > On 07/12/2016 12:29 PM, Richard Biener wrote: > > >Instead of adding more configury can we please enable LRA on trunk by default > >_now_? Otherwise the amount of testing it recieves won't really increase. > > I hope you mean for ppc only, otherwise you're breaking a lot of ports... Yes I mean for PowerPC only. I can change the switches name to: --enable-powerpc-lra --enable-powerpc-float128 if it would be clearer. If you would prefer the switches to be undocumented, and just private switches, that is fine also. I view --enable-lra/--enble-powerpc-lra as a temporary switch to allow us to straddle the issue until the performance blocker (PR 69847) is resolved, and the PowerPC compiler switches to LRA in GCC 7. Ideally, it would be nice to eliminate the support for reload in the PowerPC backend (and hence this switch). But until we fully transition to lra, it would be useful to have a way to switch what the default is without having to edit the sandbox. Likewise, --enable-float128/--enable-powerpc-float128 is a transition switch. It would be nice to have __float128 on by default without using the -mfloat128 switch, but we have a lot of work in the library arena before we can do this. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH], PowerPC support to enable -mlra and/or -mfloat128 2016-07-12 19:44 ` Michael Meissner @ 2016-07-12 21:20 ` Segher Boessenkool 0 siblings, 0 replies; 17+ messages in thread From: Segher Boessenkool @ 2016-07-12 21:20 UTC (permalink / raw) To: Michael Meissner, Bernd Schmidt, Richard Biener, GCC Patches, David Edelsohn, Bill Schmidt On Tue, Jul 12, 2016 at 03:44:37PM -0400, Michael Meissner wrote: > > I hope you mean for ppc only, otherwise you're breaking a lot of ports... > > Yes I mean for PowerPC only. I can change the switches name to: > > --enable-powerpc-lra > --enable-powerpc-float128 > > if it would be clearer. That would be nice I think. For the PowerPC port it is okay with that; but I don't think I can approve the configure parts. Segher ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-07-20 17:04 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-07-11 20:07 [PATCH], PowerPC support to enable -mlra and/or -mfloat128 Michael Meissner 2016-07-11 21:25 ` Michael Meissner 2016-07-20 17:04 ` [PATCH #2], " Michael Meissner 2016-07-12 10:29 ` [PATCH], " Richard Biener 2016-07-12 11:19 ` Segher Boessenkool 2016-07-12 11:21 ` Bernd Schmidt 2016-07-12 11:31 ` Richard Biener 2016-07-12 12:16 ` Segher Boessenkool 2016-07-12 12:40 ` Bernd Schmidt 2016-07-12 12:45 ` Segher Boessenkool 2016-07-12 12:48 ` Bernd Schmidt 2016-07-12 15:17 ` Bernd Schmidt 2016-07-12 16:31 ` Segher Boessenkool 2016-07-12 16:57 ` Bernd Schmidt 2016-07-12 12:55 ` Segher Boessenkool 2016-07-12 19:44 ` Michael Meissner 2016-07-12 21:20 ` Segher Boessenkool
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).