Re: [PATCH, rs6000] Switch the rs6000 port over to LRA
On 8/3/16 6:03 PM, David Edelsohn wrote: Please open a Bugzilla for the rs6000 backend about the vsx-timode performance regression. The vsx-timode regression needs to be fixed for GCC 7. Ok, I opened https://gcc.gnu.org/PR72804 and will start debugging the problem. Peter
Re: [PATCH, rs6000] Switch the rs6000 port over to LRA
On 8/3/16 6:03 PM, David Edelsohn wrote: On Wed, Aug 3, 2016 at 6:59 PM, Peter Bergner wrote: My question, is since these failures are not due to LRA, do we want to consider the switch to LRA ok to commit or do we want to wait until the -mvsx-timode performance bug is fixed? Peter, Please go ahead and commit the LRA switch patch. Ok, committed as revision 239105. Thanks! Please open a Bugzilla for the rs6000 backend about the vsx-timode performance regression. The vsx-timode regression needs to be fixed for GCC 7. Will do. Peter
Re: [PATCH, rs6000] Switch the rs6000 port over to LRA
On Wed, Aug 3, 2016 at 6:59 PM, Peter Bergner wrote: > On 8/2/16 3:17 PM, Peter Bergner wrote: >> >> Now that Vlad has fixed PR69847, which was the last problem holding the >> rs6000 port from switching from reload to LRA, we are ready to flip the >> switch. >> >> Is the following ok once bootstrap/regtesting on both LE and BE >> (32 & 64 regtesting) comes out clean? > > > So we have two "regressions": > > +FAIL: gcc.target/powerpc/bool3-p7.c scan-assembler-not [ \\t]xxlnor > +FAIL: gcc.target/powerpc/bool3-p8.c scan-assembler-not [ \\t]xxlnor > > > Looking into these "failures", they show up because when we enable > LRA, we also implicitly enable -mvsx-timode and these failures are > due to -mvsx-timode. The same test cases fail when we use -mvsx-timode > with reload. > > I'll note that these failures are not code correctness bugs, but > performance bugs. I plan to open a bugzilla to track the fixing > of these failures. > > My question, is since these failures are not due to LRA, do we > want to consider the switch to LRA ok to commit or do we want to > wait until the -mvsx-timode performance bug is fixed? Peter, Please go ahead and commit the LRA switch patch. Please open a Bugzilla for the rs6000 backend about the vsx-timode performance regression. The vsx-timode regression needs to be fixed for GCC 7. Thanks, David
Re: [PATCH, rs6000] Switch the rs6000 port over to LRA
On 8/2/16 3:17 PM, Peter Bergner wrote: Now that Vlad has fixed PR69847, which was the last problem holding the rs6000 port from switching from reload to LRA, we are ready to flip the switch. Is the following ok once bootstrap/regtesting on both LE and BE (32 & 64 regtesting) comes out clean? So we have two "regressions": +FAIL: gcc.target/powerpc/bool3-p7.c scan-assembler-not [ \\t]xxlnor +FAIL: gcc.target/powerpc/bool3-p8.c scan-assembler-not [ \\t]xxlnor Looking into these "failures", they show up because when we enable LRA, we also implicitly enable -mvsx-timode and these failures are due to -mvsx-timode. The same test cases fail when we use -mvsx-timode with reload. I'll note that these failures are not code correctness bugs, but performance bugs. I plan to open a bugzilla to track the fixing of these failures. My question, is since these failures are not due to LRA, do we want to consider the switch to LRA ok to commit or do we want to wait until the -mvsx-timode performance bug is fixed? Peter
Re: [PATCH, rs6000] Switch the rs6000 port over to LRA
On Tue, Aug 02, 2016 at 03:38:10PM -0500, Peter Bergner wrote: > On 8/2/16 3:30 PM, David Edelsohn wrote: > >>> * config/rs6000/rs6000.c (rs6000_option_override_internal): Make > >>> LRA > >>> the default for the rs6000 port. > > > >Okay. > > > >Do we eventually want to remove the switch? > > I think we want to keep it for at least one release, so we can > fall back to reload in case we hit any bugs in the transition. > Eventually removing the switch will be nice, since it will allow > us to clean up (ie, remove!) some code in our port. Maybe we can remove it this release already. We'll see, we have some time still. Hopefully there won't be many new issues cropping up. Segher
Re: [PATCH, rs6000] Switch the rs6000 port over to LRA
On 8/2/16 3:30 PM, David Edelsohn wrote: * config/rs6000/rs6000.c (rs6000_option_override_internal): Make LRA the default for the rs6000 port. Okay. Do we eventually want to remove the switch? I think we want to keep it for at least one release, so we can fall back to reload in case we hit any bugs in the transition. Eventually removing the switch will be nice, since it will allow us to clean up (ie, remove!) some code in our port. Peter
Re: [PATCH, rs6000] Switch the rs6000 port over to LRA
On Tue, Aug 2, 2016 at 4:20 PM, Segher Boessenkool wrote: > On Tue, Aug 02, 2016 at 03:17:45PM -0500, Peter Bergner wrote: >> Now that Vlad has fixed PR69847, which was the last problem holding the >> rs6000 port from switching from reload to LRA, we are ready to flip the >> switch. >> >> Is the following ok once bootstrap/regtesting on both LE and BE >> (32 & 64 regtesting) comes out clean? > > Oh, I don't know. David, what do you think? > > > Segher > > >> * config/rs6000/rs6000.c (rs6000_option_override_internal): Make LRA >> the default for the rs6000 port. Okay. Do we eventually want to remove the switch? Thanks, David
Re: [PATCH, rs6000] Switch the rs6000 port over to LRA
On Tue, Aug 02, 2016 at 03:17:45PM -0500, Peter Bergner wrote: > Now that Vlad has fixed PR69847, which was the last problem holding the > rs6000 port from switching from reload to LRA, we are ready to flip the > switch. > > Is the following ok once bootstrap/regtesting on both LE and BE > (32 & 64 regtesting) comes out clean? Oh, I don't know. David, what do you think? Segher > * config/rs6000/rs6000.c (rs6000_option_override_internal): Make LRA > the default for the rs6000 port. > > Index: gcc/config/rs6000/rs6000.c > === > --- gcc/config/rs6000/rs6000.c(revision 238996) > +++ gcc/config/rs6000/rs6000.c(working copy) > @@ -4323,6 +4323,10 @@ rs6000_option_override_internal (bool gl >rs6000_isa_flags &= ~OPTION_MASK_P9_DFORM_SCALAR; > } > > + /* Enable LRA by default. */ > + if ((rs6000_isa_flags_explicit & OPTION_MASK_LRA) == 0) > +rs6000_isa_flags |= OPTION_MASK_LRA; > + >/* 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
[PATCH, rs6000] Switch the rs6000 port over to LRA
Now that Vlad has fixed PR69847, which was the last problem holding the rs6000 port from switching from reload to LRA, we are ready to flip the switch. Is the following ok once bootstrap/regtesting on both LE and BE (32 & 64 regtesting) comes out clean? Peter * config/rs6000/rs6000.c (rs6000_option_override_internal): Make LRA the default for the rs6000 port. Index: gcc/config/rs6000/rs6000.c === --- gcc/config/rs6000/rs6000.c (revision 238996) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -4323,6 +4323,10 @@ rs6000_option_override_internal (bool gl rs6000_isa_flags &= ~OPTION_MASK_P9_DFORM_SCALAR; } + /* Enable LRA by default. */ + if ((rs6000_isa_flags_explicit & OPTION_MASK_LRA) == 0) +rs6000_isa_flags |= OPTION_MASK_LRA; + /* 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
