Remove setting -mblock-ops-vector-pair on power10.
Commit Message
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(-)
Comments
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
@@ -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. */