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

Reply via email to