public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Remove setting -mblock-ops-vector-pair on power10.
@ 2022-07-21  6:42 Michael Meissner
  2022-07-21 20:50 ` Segher Boessenkool
  0 siblings, 1 reply; 2+ messages in thread
From: Michael Meissner @ 2022-07-21  6:42 UTC (permalink / raw)
  To: gcc-patches, Michael Meissner, Segher Boessenkool, Kewen.Lin,
	David Edelsohn, Peter Bergner, Will Schmidt

Remove setting -mblock-ops-vector-pair on power10.

Testing has shown that using the load vector pair and store vector pair
instructions for block moves has some performance issues on power10.  This
patch does not set this option by default.  If it is a win in other
machines in the future, this flag can be set in the ISA options.

This patch modifies a previous patch that attempted to do the same thing
but the previous patch was flawed in that it would generate the load
vector pair and store vector pair instructions if we are tuning for a
different machine.

I have tested this patch on power10 systems (with long double set to IEEE
128-bit and also with long double set to IBM 128-bit).  Can I check this patch
into the trunk and back port to older GCC releases?

2022-07-21   Michael Meissner  <meissner@linux.ibm.com>

gcc/

	* config/rs6000/rs6000.cc (rs6000_option_override_internal):
	Do not enable -mblock-ops-vector-pair by default on power10.
---
 gcc/config/rs6000/rs6000.cc | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 3ff16b8ae04..667f83b1dfd 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -4139,17 +4139,6 @@ rs6000_option_override_internal (bool global_init_p)
 	rs6000_isa_flags &= ~OPTION_MASK_BLOCK_OPS_UNALIGNED_VSX;
     }
 
-  if (!(rs6000_isa_flags_explicit & OPTION_MASK_BLOCK_OPS_VECTOR_PAIR))
-    {
-      /* Do not generate lxvp and stxvp on power10 since there are some
-	 performance issues.  */
-      if (TARGET_MMA && TARGET_EFFICIENT_UNALIGNED_VSX
-	  && rs6000_tune != PROCESSOR_POWER10)
-	rs6000_isa_flags |= OPTION_MASK_BLOCK_OPS_VECTOR_PAIR;
-      else
-	rs6000_isa_flags &= ~OPTION_MASK_BLOCK_OPS_VECTOR_PAIR;
-    }
-
   /* Use long double size to select the appropriate long double.  We use
      TYPE_PRECISION to differentiate the 3 different long double types.  We map
      128 into the precision used for TFmode.  */
-- 
2.35.3


-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meissner@linux.ibm.com

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Remove setting -mblock-ops-vector-pair on power10.
  2022-07-21  6:42 [PATCH] Remove setting -mblock-ops-vector-pair on power10 Michael Meissner
@ 2022-07-21 20:50 ` Segher Boessenkool
  0 siblings, 0 replies; 2+ messages in thread
From: Segher Boessenkool @ 2022-07-21 20:50 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, Kewen.Lin, David Edelsohn,
	Peter Bergner, Will Schmidt

On Thu, Jul 21, 2022 at 02:42:29AM -0400, Michael Meissner wrote:
> Testing has shown that using the load vector pair and store vector pair
> instructions for block moves has some performance issues on power10.  This
> patch does not set this option by default.  If it is a win in other
> machines in the future, this flag can be set in the ISA options.

This would make rs6000_isa_flags an even bigger misnomer than it already
is, sigh.

> 	* config/rs6000/rs6000.cc (rs6000_option_override_internal):
> 	Do not enable -mblock-ops-vector-pair by default on power10.

Do not wrap lines early, especially if that would mean leaving a colon
at the end of a line.  Changelog lines are 80 positions long (including
the leading tab, which counts as eight).

> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -4139,17 +4139,6 @@ rs6000_option_override_internal (bool global_init_p)
>  	rs6000_isa_flags &= ~OPTION_MASK_BLOCK_OPS_UNALIGNED_VSX;
>      }
>  
> -  if (!(rs6000_isa_flags_explicit & OPTION_MASK_BLOCK_OPS_VECTOR_PAIR))
> -    {
> -      /* Do not generate lxvp and stxvp on power10 since there are some
> -	 performance issues.  */
> -      if (TARGET_MMA && TARGET_EFFICIENT_UNALIGNED_VSX
> -	  && rs6000_tune != PROCESSOR_POWER10)
> -	rs6000_isa_flags |= OPTION_MASK_BLOCK_OPS_VECTOR_PAIR;
> -      else
> -	rs6000_isa_flags &= ~OPTION_MASK_BLOCK_OPS_VECTOR_PAIR;
> -    }

How does this implement what the changelog says it does?  With what it
does the changelog should instead say to not touch it at all (your patch
also disables the code that disables it!)

It isn't clear what you intended: what your changelog says, or what the
code does.


Segher

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-07-21 20:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-21  6:42 [PATCH] Remove setting -mblock-ops-vector-pair on power10 Michael Meissner
2022-07-21 20:50 ` 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).