[v2] rs6000: Disable HTM for Power10 and later

Message ID	e8fed607-eb85-14ab-5295-018dd4716f97@linux.ibm.com
State	New
Headers	show Return-Path: <gcc-patches-bounces@gcc.gnu.org> DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9DFE4383E83B Subject: [PATCH v2] rs6000: Disable HTM for Power10 and later To: Segher Boessenkool <segher@kernel.crashing.org>, GCC Patches <gcc-patches@gcc.gnu.org> References: <84e1df49-0509-461a-f5dd-765de1b83610@linux.ibm.com> <20201126211159.GF2672@gate.crashing.org> Message-ID: <e8fed607-eb85-14ab-5295-018dd4716f97@linux.ibm.com> Date: Mon, 30 Nov 2020 18:36:30 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.0; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 MIME-Version: 1.0 In-Reply-To: <20201126211159.GF2672@gate.crashing.org> Content-Type: multipart/mixed; boundary="------------B09E916A2242CAE1F1DB28BF" Content-Language: en-US Precedence: list From: "Kewen.Lin via Gcc-patches" <gcc-patches@gcc.gnu.org> Reply-To: "Kewen.Lin" <linkw@linux.ibm.com> Cc: Bill Schmidt <wschmidt@linux.ibm.com>, David Edelsohn <dje.gcc@gmail.com> Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces@gcc.gnu.org>
Series	[v2] rs6000: Disable HTM for Power10 and later \| expand [v2] rs6000: Disable HTM for Power10 and later

Message ID

e8fed607-eb85-14ab-5295-018dd4716f97@linux.ibm.com

State

New

Headers

DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9DFE4383E83B
Subject: [PATCH v2] rs6000: Disable HTM for Power10 and later
To: Segher Boessenkool <segher@kernel.crashing.org>,
 GCC Patches <gcc-patches@gcc.gnu.org>
References: <84e1df49-0509-461a-f5dd-765de1b83610@linux.ibm.com>
 <20201126211159.GF2672@gate.crashing.org>
Message-ID: <e8fed607-eb85-14ab-5295-018dd4716f97@linux.ibm.com>
Date: Mon, 30 Nov 2020 18:36:30 +0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.0; rv:68.0)
 Gecko/20100101 Thunderbird/68.9.0
MIME-Version: 1.0
In-Reply-To: <20201126211159.GF2672@gate.crashing.org>
Content-Type: multipart/mixed; boundary="------------B09E916A2242CAE1F1DB28BF"
Content-Language: en-US
Precedence: list
From: "Kewen.Lin via Gcc-patches" <gcc-patches@gcc.gnu.org>
Reply-To: "Kewen.Lin" <linkw@linux.ibm.com>
Cc: Bill Schmidt <wschmidt@linux.ibm.com>, David Edelsohn <dje.gcc@gmail.com>
Errors-To: gcc-patches-bounces@gcc.gnu.org
Sender: "Gcc-patches" <gcc-patches-bounces@gcc.gnu.org>

Series

[v2] rs6000: Disable HTM for Power10 and later | expand

Commit Message

Kewen.Lin Nov. 30, 2020, 10:36 a.m. UTC

Hi Segher,

on 2020/11/27 上午5:11, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Nov 26, 2020 at 06:15:04PM +0800, Kewen.Lin wrote:
>> During previous Power10 testing, I noticed that ISA 3.1 has
>> dropped TM support.  I think we should not generate TM
>> related instructions for Power10 and later, or no?
>>
>> This patch is to turn off HTM support once we know the cpu
>> setting is power10 or later, and warn something if the
>> explicit option -mhtm is specified.
> 
> We currently enable OPTION_MASK_HTM in ISA_2_7_MASKS_SERVER (in
> rs6000-cpus.def), and include that in all later CPUs/archs.
> 
> Could you fix it there instead?  Easiest and most obvious is to just
> add HTM to the power8, power9, and powerpc64le entries.
> 

Thanks for the comments!  The attached patch followed your suggestion.

Does it look better?

Bootstrapped/regtested on powerpc64le-linux-gnu P8 and P10.

BR,
Kewen
-----
gcc/ChangeLog:

	* config/rs6000/rs6000.c (rs6000_option_override_internal):
	Use OPTION_MASK_CRYPTO for Power8 target_enable check instead
	of OPTION_MASK_HTM.
	* config/rs6000/rs6000-cpus.def (ISA_2_7_MASKS_SERVER):
	Remove OPTION_MASK_HTM.
	(RS6000_CPU): Add OPTION_MASK_HTM to power8, power9 and
	powerpc64le entries.

Comments

Segher Boessenkool Nov. 30, 2020, 8:49 p.m. UTC | #1

Hi!

On Mon, Nov 30, 2020 at 06:36:30PM +0800, Kewen.Lin wrote:
> --- a/gcc/config/rs6000/rs6000-cpus.def
> +++ b/gcc/config/rs6000/rs6000-cpus.def
> @@ -51,7 +51,6 @@
>  				 | OPTION_MASK_CRYPTO			\
>  				 | OPTION_MASK_DIRECT_MOVE		\
>  				 | OPTION_MASK_EFFICIENT_UNALIGNED_VSX	\
> -				 | OPTION_MASK_HTM			\
>  				 | OPTION_MASK_QUAD_MEMORY		\
>  				 | OPTION_MASK_QUAD_MEMORY_ATOMIC)

(this is in #define ISA_2_7_MASKS_SERVER)

That looks fine.

> -RS6000_CPU ("power8", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER)
> -RS6000_CPU ("power9", PROCESSOR_POWER9, MASK_POWERPC64 | ISA_3_0_MASKS_SERVER)
> +RS6000_CPU ("power8", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER
> +	    | OPTION_MASK_HTM)
> +RS6000_CPU ("power9", PROCESSOR_POWER9, MASK_POWERPC64 | ISA_3_0_MASKS_SERVER
> +	    | OPTION_MASK_HTM)

> -RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER)
> +RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER
> +	    | OPTION_MASK_HTM)

This, too.

> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -3807,7 +3807,8 @@ rs6000_option_override_internal (bool global_init_p)
>    /* If little-endian, default to -mstrict-align on older processors.
>       Testing for htm matches power8 and later.  */
>    if (!BYTES_BIG_ENDIAN
> -      && !(processor_target_table[tune_index].target_enable & OPTION_MASK_HTM))
> +      && !(processor_target_table[tune_index].target_enable
> +	   & OPTION_MASK_CRYPTO))
>      rs6000_isa_flags |= ~rs6000_isa_flags_explicit & OPTION_MASK_STRICT_ALIGN;

But not this.  Not all ISA 2.07 processors implement the crypto
category.  You could use OPTION_MASK_DIRECT_MOVE, instead?

Okay for trunk with that change (if that works ;-) )  Thanks!


Segher

Kewen.Lin Dec. 2, 2020, 9:35 a.m. UTC | #2

Hi Segher,

on 2020/12/1 上午4:49, Segher Boessenkool wrote:
> Hi!
> 
> On Mon, Nov 30, 2020 at 06:36:30PM +0800, Kewen.Lin wrote:
>> --- a/gcc/config/rs6000/rs6000-cpus.def
>> +++ b/gcc/config/rs6000/rs6000-cpus.def
>> @@ -51,7 +51,6 @@
>>  				 | OPTION_MASK_CRYPTO			\
>>  				 | OPTION_MASK_DIRECT_MOVE		\
>>  				 | OPTION_MASK_EFFICIENT_UNALIGNED_VSX	\
>> -				 | OPTION_MASK_HTM			\
>>  				 | OPTION_MASK_QUAD_MEMORY		\
>>  				 | OPTION_MASK_QUAD_MEMORY_ATOMIC)
> 
> (this is in #define ISA_2_7_MASKS_SERVER)
> 
> That looks fine.
> 
>> -RS6000_CPU ("power8", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER)
>> -RS6000_CPU ("power9", PROCESSOR_POWER9, MASK_POWERPC64 | ISA_3_0_MASKS_SERVER)
>> +RS6000_CPU ("power8", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER
>> +	    | OPTION_MASK_HTM)
>> +RS6000_CPU ("power9", PROCESSOR_POWER9, MASK_POWERPC64 | ISA_3_0_MASKS_SERVER
>> +	    | OPTION_MASK_HTM)
> 
>> -RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER)
>> +RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER
>> +	    | OPTION_MASK_HTM)
> 
> This, too.
> 
>> --- a/gcc/config/rs6000/rs6000.c
>> +++ b/gcc/config/rs6000/rs6000.c
>> @@ -3807,7 +3807,8 @@ rs6000_option_override_internal (bool global_init_p)
>>    /* If little-endian, default to -mstrict-align on older processors.
>>       Testing for htm matches power8 and later.  */
>>    if (!BYTES_BIG_ENDIAN
>> -      && !(processor_target_table[tune_index].target_enable & OPTION_MASK_HTM))
>> +      && !(processor_target_table[tune_index].target_enable
>> +	   & OPTION_MASK_CRYPTO))
>>      rs6000_isa_flags |= ~rs6000_isa_flags_explicit & OPTION_MASK_STRICT_ALIGN;
> 
> But not this.  Not all ISA 2.07 processors implement the crypto
> category.  You could use OPTION_MASK_DIRECT_MOVE, instead?
> 

Thanks for the comments! 

Updated with OPTION_MASK_DIRECT_MOVE, re-testings passed and committed in r11-5645.

BR,
Kewen

diff --git a/gcc/config/rs6000/rs6000-cpus.def b/gcc/config/rs6000/rs6000-cpus.def
index 8d2c1ffd6cf..482e1b69dde 100644
--- a/gcc/config/rs6000/rs6000-cpus.def
+++ b/gcc/config/rs6000/rs6000-cpus.def
@@ -51,7 +51,6 @@ 
 				 | OPTION_MASK_CRYPTO			\
 				 | OPTION_MASK_DIRECT_MOVE		\
 				 | OPTION_MASK_EFFICIENT_UNALIGNED_VSX	\
-				 | OPTION_MASK_HTM			\
 				 | OPTION_MASK_QUAD_MEMORY		\
 				 | OPTION_MASK_QUAD_MEMORY_ATOMIC)
 
@@ -240,10 +239,13 @@  RS6000_CPU ("power6x", PROCESSOR_POWER6, MASK_POWERPC64 | MASK_PPC_GPOPT
 	    | MASK_PPC_GFXOPT | MASK_MFCRF | MASK_POPCNTB | MASK_FPRND
 	    | MASK_CMPB | MASK_DFP | MASK_RECIP_PRECISION)
 RS6000_CPU ("power7", PROCESSOR_POWER7, MASK_POWERPC64 | ISA_2_6_MASKS_SERVER)
-RS6000_CPU ("power8", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER)
-RS6000_CPU ("power9", PROCESSOR_POWER9, MASK_POWERPC64 | ISA_3_0_MASKS_SERVER)
+RS6000_CPU ("power8", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER
+	    | OPTION_MASK_HTM)
+RS6000_CPU ("power9", PROCESSOR_POWER9, MASK_POWERPC64 | ISA_3_0_MASKS_SERVER
+	    | OPTION_MASK_HTM)
 RS6000_CPU ("power10", PROCESSOR_POWER10, MASK_POWERPC64 | ISA_3_1_MASKS_SERVER)
 RS6000_CPU ("powerpc", PROCESSOR_POWERPC, 0)
 RS6000_CPU ("powerpc64", PROCESSOR_POWERPC64, MASK_PPC_GFXOPT | MASK_POWERPC64)
-RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER)
+RS6000_CPU ("powerpc64le", PROCESSOR_POWER8, MASK_POWERPC64 | ISA_2_7_MASKS_SERVER
+	    | OPTION_MASK_HTM)
 RS6000_CPU ("rs64", PROCESSOR_RS64A, MASK_PPC_GFXOPT | MASK_POWERPC64)
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index d8ac2f0cd2f..4f625d98078 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -3807,7 +3807,8 @@  rs6000_option_override_internal (bool global_init_p)
   /* If little-endian, default to -mstrict-align on older processors.
      Testing for htm matches power8 and later.  */
   if (!BYTES_BIG_ENDIAN
-      && !(processor_target_table[tune_index].target_enable & OPTION_MASK_HTM))
+      && !(processor_target_table[tune_index].target_enable
+	   & OPTION_MASK_CRYPTO))
     rs6000_isa_flags |= ~rs6000_isa_flags_explicit & OPTION_MASK_STRICT_ALIGN;
 
   if (!rs6000_fold_gimple)