diff mbox

rs6000: Fix separate shrink-wrapping for TARGET_MULTIPLE

Message ID fe0813498b3407117eea0a56cab98d1225be0ef0.1476658601.git.segher@kernel.crashing.org
State New
Headers show

Commit Message

Segher Boessenkool Oct. 17, 2016, 8:13 a.m. UTC
We cannot use {SAVE,REST}_MULTIPLE and separate shrink-wrapping together,
not without checking when actually emitting the prologue/epilogue that the
registers to save/restore are actually still one contiguous block up to
(and including) 31.  So either:

1) We delay the decision of whether to use lmw/stmw to later;
2) We disallow shrink-wrapping separate (integer) components when those
strategies are selected; or
3) We don't use those strategies if we use separate shrink-wrapping.

This patch does 3).  In the long term it may be best to do 1) instead,
it can be slightly more efficient.

This caused problems on darwin (it is the only config that uses lmw/stmw
instructions by default).

Bootstrapped and tested on powerpc64-linux {-m64,-m32}.  I'll commit it
if Iain's testing (on darwin) also succeeds.


Segher


2016-10-17  Segher Boessenkool  <segher@kernel.crashing.org>

	* config/rs6000/rs6000.c: Add include of shrink-wrap.h .
	(rs6000_savres_strategy): Do not select {SAVE,REST}_MULTIPLE if
	shrink-wrapping separate components.
	(rs6000_get_separate_components): Assert we do not have those
	strategies selected.

---
 gcc/config/rs6000/rs6000.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Iain Sandoe Oct. 17, 2016, 11:17 p.m. UTC | #1
Hi Segher,

> On 17 Oct 2016, at 09:13, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> 
> We cannot use {SAVE,REST}_MULTIPLE and separate shrink-wrapping together,
> not without checking when actually emitting the prologue/epilogue that the
> registers to save/restore are actually still one contiguous block up to
> (and including) 31.  So either:
> 
> 1) We delay the decision of whether to use lmw/stmw to later;
> 2) We disallow shrink-wrapping separate (integer) components when those
> strategies are selected; or
> 3) We don't use those strategies if we use separate shrink-wrapping.
> 
> This patch does 3).  In the long term it may be best to do 1) instead,
> it can be slightly more efficient.
> 
> This caused problems on darwin (it is the only config that uses lmw/stmw
> instructions by default).
> 
> Bootstrapped and tested on powerpc64-linux {-m64,-m32}.  I'll commit it
> if Iain's testing (on darwin) also succeeds.

thanks!

All-langs bootstrap was restored with the patch (and others in progress for existing known issues); 

I can’t see any evidence of the assert firing in the test-suite, so it all looks good to me (there’s quite a bit of stage-1-ish testsuite noise at present, however).

cheers
Iain
> 
> 
> 2016-10-17  Segher Boessenkool  <segher@kernel.crashing.org>
> 
> 	* config/rs6000/rs6000.c: Add include of shrink-wrap.h .
> 	(rs6000_savres_strategy): Do not select {SAVE,REST}_MULTIPLE if
> 	shrink-wrapping separate components.
> 	(rs6000_get_separate_components): Assert we do not have those
> 	strategies selected.
> 
> ---
> gcc/config/rs6000/rs6000.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index df48980..83a7139 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -64,6 +64,7 @@
> #include "target-globals.h"
> #include "builtins.h"
> #include "context.h"
> +#include "shrink-wrap.h"
> #include "tree-pass.h"
> #if TARGET_XCOFF
> #include "xcoffout.h"  /* get declarations of xcoff_*_section_name */
> @@ -25506,7 +25507,10 @@ rs6000_savres_strategy (rs6000_stack_t *info,
>   if (TARGET_MULTIPLE
>       && !TARGET_POWERPC64
>       && !(TARGET_SPE_ABI && info->spe_64bit_regs_used)
> -      && info->first_gp_reg_save < 31)
> +      && info->first_gp_reg_save < 31
> +      && !(SHRINK_WRAPPING_ENABLED
> +	   && flag_shrink_wrap_separate
> +	   && optimize_function_for_speed_p (cfun)))
>     {
>       /* Prefer store multiple for saves over out-of-line routines,
> 	 since the store-multiple instruction will always be smaller.  */
> @@ -27440,6 +27444,9 @@ rs6000_get_separate_components (void)
>   sbitmap components = sbitmap_alloc (32);
>   bitmap_clear (components);
> 
> +  gcc_assert (!(info->savres_strategy & SAVE_MULTIPLE)
> +	      && !(info->savres_strategy & REST_MULTIPLE));
> +
>   /* The GPRs we need saved to the frame.  */
>   if ((info->savres_strategy & SAVE_INLINE_GPRS)
>       && (info->savres_strategy & REST_INLINE_GPRS))
> -- 
> 1.9.3
>
diff mbox

Patch

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index df48980..83a7139 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -64,6 +64,7 @@ 
 #include "target-globals.h"
 #include "builtins.h"
 #include "context.h"
+#include "shrink-wrap.h"
 #include "tree-pass.h"
 #if TARGET_XCOFF
 #include "xcoffout.h"  /* get declarations of xcoff_*_section_name */
@@ -25506,7 +25507,10 @@  rs6000_savres_strategy (rs6000_stack_t *info,
   if (TARGET_MULTIPLE
       && !TARGET_POWERPC64
       && !(TARGET_SPE_ABI && info->spe_64bit_regs_used)
-      && info->first_gp_reg_save < 31)
+      && info->first_gp_reg_save < 31
+      && !(SHRINK_WRAPPING_ENABLED
+	   && flag_shrink_wrap_separate
+	   && optimize_function_for_speed_p (cfun)))
     {
       /* Prefer store multiple for saves over out-of-line routines,
 	 since the store-multiple instruction will always be smaller.  */
@@ -27440,6 +27444,9 @@  rs6000_get_separate_components (void)
   sbitmap components = sbitmap_alloc (32);
   bitmap_clear (components);
 
+  gcc_assert (!(info->savres_strategy & SAVE_MULTIPLE)
+	      && !(info->savres_strategy & REST_MULTIPLE));
+
   /* The GPRs we need saved to the frame.  */
   if ((info->savres_strategy & SAVE_INLINE_GPRS)
       && (info->savres_strategy & REST_INLINE_GPRS))