Re: [PATCH] rs6000: Make P10_FUSION honour tuning setting
on 2023/1/6 17:28, Kewen.Lin via Gcc-patches wrote: > Hi Pat, > > on 2023/1/6 03:30, Pat Haugen wrote: >> On 1/4/23 3:20 AM, Kewen.Lin via Gcc-patches wrote: >>> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc >>> index 88c865b6b4b..6fa084c0807 100644 >>> --- a/gcc/config/rs6000/rs6000.cc >>> +++ b/gcc/config/rs6000/rs6000.cc >>> @@ -4378,9 +4378,15 @@ rs6000_option_override_internal (bool global_init_p) >>> rs6000_isa_flags &= ~OPTION_MASK_MMA; >>> } >>> >>> - if (TARGET_POWER10 >>> - && (rs6000_isa_flags_explicit & OPTION_MASK_P10_FUSION) == 0) >>> - rs6000_isa_flags |= OPTION_MASK_P10_FUSION; >>> + /* Enable power10 fusion if we are tuning for power10, even if we aren't >>> + generating power10 instructions. */ >>> + if (!(rs6000_isa_flags_explicit & OPTION_MASK_P10_FUSION)) >>> + { >>> + if (processor_target_table[tune_index].processor == >>> PROCESSOR_POWER10) >> >> You can use (rs6000_tune == PROCESSOR_POWER10) at this point. > > Good catch, I will update it. Thanks! Committed the updated version (as attached) in r13-5107-g6224db0e4d6d3b. BR, Kewen Subject: [PATCH] rs6000: Make P10_FUSION honour tuning setting We noticed this issue when Segher reviewed the patch for PR104024. When there is no explicit setting for option -mpower10-fusion, we enable OPTION_MASK_P10_FUSION for TARGET_POWER10. But it's not right, it should honour tuning setting instead. This patch is to fix it accordingly, it's bootstrapped , and regtested on powerpc64-linux-gnu P8 and powerpc64le-linux-gnu P9. But on powerpc64le-linux-gnu P10 it had one regression failure against the test case gcc.target/powerpc/pr105586.c. I looked into it and confirmed that a latent bug was exposed and filed one separated bug PR108273 instead. gcc/ChangeLog: * config/rs6000/rs6000.cc (rs6000_option_override_internal): Make OPTION_MASK_P10_FUSION implicit setting honour Power10 tuning setting. * config/rs6000/rs6000-cpus.def (ISA_3_1_MASKS_SERVER): Remove OPTION_MASK_P10_FUSION. --- gcc/config/rs6000/rs6000-cpus.def | 3 +-- gcc/config/rs6000/rs6000.cc | 12 +--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/gcc/config/rs6000/rs6000-cpus.def b/gcc/config/rs6000/rs6000-cpus.def index c3825bcccd8..4d5544e927a 100644 --- a/gcc/config/rs6000/rs6000-cpus.def +++ b/gcc/config/rs6000/rs6000-cpus.def @@ -84,8 +84,7 @@ #define ISA_3_1_MASKS_SERVER (ISA_3_0_MASKS_SERVER \ | OPTION_MASK_POWER10 \ -| OTHER_POWER10_MASKS \ -| OPTION_MASK_P10_FUSION) +| OTHER_POWER10_MASKS) /* Flags that need to be turned off if -mno-power9-vector. */ #define OTHER_P9_VECTOR_MASKS (OPTION_MASK_FLOAT128_HW\ diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc index 6ac3adcec6b..3baa2c3b7b0 100644 --- a/gcc/config/rs6000/rs6000.cc +++ b/gcc/config/rs6000/rs6000.cc @@ -4397,9 +4397,15 @@ rs6000_option_override_internal (bool global_init_p) rs6000_isa_flags &= ~OPTION_MASK_MMA; } - if (TARGET_POWER10 - && (rs6000_isa_flags_explicit & OPTION_MASK_P10_FUSION) == 0) -rs6000_isa_flags |= OPTION_MASK_P10_FUSION; + /* Enable power10 fusion if we are tuning for power10, even if we aren't + generating power10 instructions. */ + if (!(rs6000_isa_flags_explicit & OPTION_MASK_P10_FUSION)) +{ + if (rs6000_tune == PROCESSOR_POWER10) + rs6000_isa_flags |= OPTION_MASK_P10_FUSION; + else + rs6000_isa_flags &= ~OPTION_MASK_P10_FUSION; +} /* MMA requires SIMD support as ISA 3.1 claims and our implementation such as "*movoo" uses vector pair access which use VSX registers. -- 2.34.1
Re: [PATCH] rs6000: Make P10_FUSION honour tuning setting
Hi Pat, on 2023/1/6 03:30, Pat Haugen wrote: > On 1/4/23 3:20 AM, Kewen.Lin via Gcc-patches wrote: >> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc >> index 88c865b6b4b..6fa084c0807 100644 >> --- a/gcc/config/rs6000/rs6000.cc >> +++ b/gcc/config/rs6000/rs6000.cc >> @@ -4378,9 +4378,15 @@ rs6000_option_override_internal (bool global_init_p) >> rs6000_isa_flags &= ~OPTION_MASK_MMA; >> } >> >> - if (TARGET_POWER10 >> - && (rs6000_isa_flags_explicit & OPTION_MASK_P10_FUSION) == 0) >> - rs6000_isa_flags |= OPTION_MASK_P10_FUSION; >> + /* Enable power10 fusion if we are tuning for power10, even if we aren't >> + generating power10 instructions. */ >> + if (!(rs6000_isa_flags_explicit & OPTION_MASK_P10_FUSION)) >> + { >> + if (processor_target_table[tune_index].processor == PROCESSOR_POWER10) > > You can use (rs6000_tune == PROCESSOR_POWER10) at this point. Good catch, I will update it. Thanks! BR, Kewen
Re: [PATCH] rs6000: Make P10_FUSION honour tuning setting
On 1/4/23 3:20 AM, Kewen.Lin via Gcc-patches wrote: diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc index 88c865b6b4b..6fa084c0807 100644 --- a/gcc/config/rs6000/rs6000.cc +++ b/gcc/config/rs6000/rs6000.cc @@ -4378,9 +4378,15 @@ rs6000_option_override_internal (bool global_init_p) rs6000_isa_flags &= ~OPTION_MASK_MMA; } - if (TARGET_POWER10 - && (rs6000_isa_flags_explicit & OPTION_MASK_P10_FUSION) == 0) -rs6000_isa_flags |= OPTION_MASK_P10_FUSION; + /* Enable power10 fusion if we are tuning for power10, even if we aren't + generating power10 instructions. */ + if (!(rs6000_isa_flags_explicit & OPTION_MASK_P10_FUSION)) +{ + if (processor_target_table[tune_index].processor == PROCESSOR_POWER10) You can use (rs6000_tune == PROCESSOR_POWER10) at this point. -Pat + rs6000_isa_flags |= OPTION_MASK_P10_FUSION; + else + rs6000_isa_flags &= ~OPTION_MASK_P10_FUSION; +}
[PATCH] rs6000: Make P10_FUSION honour tuning setting
Hi, We noticed this issue when Segher reviewed the patch for PR104024. When there is no explicit setting for option -mpower10-fusion, we enable OPTION_MASK_P10_FUSION for TARGET_POWER10. But it's not right, it should honour tuning setting instead. This patch is to fix it accordingly, it's bootstrapped and regtested on powerpc64-linux-gnu P8 and powerpc64le-linux-gnu P9, but on powerpc64le-linux-gnu P10 it had one regression failure against the test case gcc.target/powerpc/pr105586.c. I looked into it and confirmed a latent bug was exposed and filed one separated bug PR108273 instead. I'm going to push this soon if no objections. BR, Kewen - gcc/ChangeLog: * config/rs6000/rs6000.cc (rs6000_option_override_internal): Make OPTION_MASK_P10_FUSION implicit setting honour Power10 tuning setting. * config/rs6000/rs6000-cpus.def (ISA_3_1_MASKS_SERVER): Remove OPTION_MASK_P10_FUSION. --- gcc/config/rs6000/rs6000-cpus.def | 3 +-- gcc/config/rs6000/rs6000.cc | 12 +--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/gcc/config/rs6000/rs6000-cpus.def b/gcc/config/rs6000/rs6000-cpus.def index c3825bcccd8..4d5544e927a 100644 --- a/gcc/config/rs6000/rs6000-cpus.def +++ b/gcc/config/rs6000/rs6000-cpus.def @@ -84,8 +84,7 @@ #define ISA_3_1_MASKS_SERVER (ISA_3_0_MASKS_SERVER \ | OPTION_MASK_POWER10 \ -| OTHER_POWER10_MASKS \ -| OPTION_MASK_P10_FUSION) +| OTHER_POWER10_MASKS) /* Flags that need to be turned off if -mno-power9-vector. */ #define OTHER_P9_VECTOR_MASKS (OPTION_MASK_FLOAT128_HW\ diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc index 88c865b6b4b..6fa084c0807 100644 --- a/gcc/config/rs6000/rs6000.cc +++ b/gcc/config/rs6000/rs6000.cc @@ -4378,9 +4378,15 @@ rs6000_option_override_internal (bool global_init_p) rs6000_isa_flags &= ~OPTION_MASK_MMA; } - if (TARGET_POWER10 - && (rs6000_isa_flags_explicit & OPTION_MASK_P10_FUSION) == 0) -rs6000_isa_flags |= OPTION_MASK_P10_FUSION; + /* Enable power10 fusion if we are tuning for power10, even if we aren't + generating power10 instructions. */ + if (!(rs6000_isa_flags_explicit & OPTION_MASK_P10_FUSION)) +{ + if (processor_target_table[tune_index].processor == PROCESSOR_POWER10) + rs6000_isa_flags |= OPTION_MASK_P10_FUSION; + else + rs6000_isa_flags &= ~OPTION_MASK_P10_FUSION; +} /* MMA requires SIMD support as ISA 3.1 claims and our implementation such as "*movoo" uses vector pair access which use VSX registers. -- 2.27.0