diff mbox series

rs6000: Make P10_FUSION honour tuning setting

Message ID 9defdca0-1cf3-82a3-d04a-2eb4f3daf106@linux.ibm.com
State Accepted
Headers
Series rs6000: Make P10_FUSION honour tuning setting |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Kewen.Lin Jan. 4, 2023, 9:20 a.m. UTC 
  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(-)

--
2.27.0

Comments

Pat Haugen Jan. 5, 2023, 7:30 p.m. UTC | #1 
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;
> +    }
>
Kewen.Lin Jan. 6, 2023, 9:28 a.m. UTC | #2 
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
Kewen.Lin Jan. 11, 2023, 1:05 p.m. UTC | #3 
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.
diff mbox series

Patch

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.