Re: [PATCH][AArch64] Enable AES fusion with -mcpu=generic

[Richard Earnshaw (lists)]

On 20/04/17 16:53, Wilco Dijkstra wrote:
> 
> ping

James has already approved this on 17 March, why are you pinging again?

https://gcc.gnu.org/ml/gcc-patches/2017-03/msg00918.html

> 
> From: Wilco Dijkstra
> Sent: 16 March 2017 17:22
> To: GCC Patches; Evandro Menezes; andrew.pin...@cavium.com; 
> jim.wil...@linaro.org
> Cc: nd
> Subject: [PATCH][AArch64] Enable AES fusion with -mcpu=generic
> 
> Many supported cores implement fusion of AES instructions.  When fusion
> happens it can give a significant performance gain.  If not, scheduling
> fusion candidates next to each other has almost no effect on performance.
> Due to the high benefit/low cost it makes sense to enable AES fusion with
> -mcpu=generic so that cores that support it always benefit.  Any objections?
> 
> Bootstrapped on AArch64, no regressions.
> 
> ChangeLog:
> 2017-03-16  Wilco Dijkstra  <wdijk...@arm.com>
> 
> * gcc/config/aarch64/aarch64.c (generic_tunings): Add AES fusion.
> 
> --
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 
> 728ce7029f1e2b5161d9f317d10e564dd5a5f472..c8cf7169a5d387de336920b50c83761dc0c96f3a
>  100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -536,7 +536,7 @@ static const struct tune_params generic_tunings =
>_approx_modes,
>4, /* memmov_cost  */
>2, /* issue_rate  */
> -  AARCH64_FUSE_NOTHING, /* fusible_ops  */
> +  (AARCH64_FUSE_AES_AESMC), /* fusible_ops  */
>8,   /* function_align.  */
>8,   /* jump_align.  */
>4,   /* loop_align.  */
> 
> 

Re: [PATCH][AArch64] Enable AES fusion with -mcpu=generic

[Wilco Dijkstra]

ping

From: Wilco Dijkstra
Sent: 16 March 2017 17:22
To: GCC Patches; Evandro Menezes; andrew.pin...@cavium.com; 
jim.wil...@linaro.org
Cc: nd
Subject: [PATCH][AArch64] Enable AES fusion with -mcpu=generic
    
Many supported cores implement fusion of AES instructions.  When fusion
happens it can give a significant performance gain.  If not, scheduling
fusion candidates next to each other has almost no effect on performance.
Due to the high benefit/low cost it makes sense to enable AES fusion with
-mcpu=generic so that cores that support it always benefit.  Any objections?

Bootstrapped on AArch64, no regressions.

ChangeLog:
2017-03-16  Wilco Dijkstra  <wdijk...@arm.com>

    * gcc/config/aarch64/aarch64.c (generic_tunings): Add AES fusion.

--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 
728ce7029f1e2b5161d9f317d10e564dd5a5f472..c8cf7169a5d387de336920b50c83761dc0c96f3a
 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -536,7 +536,7 @@ static const struct tune_params generic_tunings =
   _approx_modes,
   4, /* memmov_cost  */
   2, /* issue_rate  */
-  AARCH64_FUSE_NOTHING, /* fusible_ops  */
+  (AARCH64_FUSE_AES_AESMC), /* fusible_ops  */
   8,   /* function_align.  */
   8,   /* jump_align.  */
   4,   /* loop_align.  */

Re: [PATCH][AArch64] Enable AES fusion with -mcpu=generic

[James Greenhalgh]

On Thu, Mar 16, 2017 at 08:26:42PM -0700, Jim Wilson wrote:
> On Thu, Mar 16, 2017 at 11:01 AM, Andrew Pinski  wrote:
> > On Thu, Mar 16, 2017 at 10:22 AM, Wilco Dijkstra  
> > wrote:
> >> Many supported cores implement fusion of AES instructions.  When fusion
> >> happens it can give a significant performance gain.  If not, scheduling
> >> fusion candidates next to each other has almost no effect on performance.
> >> Due to the high benefit/low cost it makes sense to enable AES fusion with
> >> -mcpu=generic so that cores that support it always benefit.  Any 
> >> objections?
> 
> No objection.  I'm not currently tracking performance of -mcpu=generic
> on falkor, so I'm not very concerned about changes to the generic
> tuning structure.

Thanks for the feedback Jim, Andrew.

This patch is OK for trunk. As Richard pointed out on the branch costs
thread, if we had a bug here we'd likely have seen it by now on those
cores which do enable the fusion.

Thanks,
James

Re: [PATCH][AArch64] Enable AES fusion with -mcpu=generic

[Jim Wilson]

On Thu, Mar 16, 2017 at 11:01 AM, Andrew Pinski  wrote:
> On Thu, Mar 16, 2017 at 10:22 AM, Wilco Dijkstra  
> wrote:
>> Many supported cores implement fusion of AES instructions.  When fusion
>> happens it can give a significant performance gain.  If not, scheduling
>> fusion candidates next to each other has almost no effect on performance.
>> Due to the high benefit/low cost it makes sense to enable AES fusion with
>> -mcpu=generic so that cores that support it always benefit.  Any objections?

No objection.  I'm not currently tracking performance of -mcpu=generic
on falkor, so I'm not very concerned about changes to the generic
tuning structure.

Jim

Re: [PATCH][AArch64] Enable AES fusion with -mcpu=generic

[Andrew Pinski]

On Thu, Mar 16, 2017 at 10:22 AM, Wilco Dijkstra  wrote:
> Many supported cores implement fusion of AES instructions.  When fusion
> happens it can give a significant performance gain.  If not, scheduling
> fusion candidates next to each other has almost no effect on performance.
> Due to the high benefit/low cost it makes sense to enable AES fusion with
> -mcpu=generic so that cores that support it always benefit.  Any objections?

I am ok with this due to our new cores support this and there was no
performance lost for ThunderX1.

Thanks,
Andrew

>
> Bootstrapped on AArch64, no regressions.
>
> ChangeLog:
> 2017-03-16  Wilco Dijkstra  
>
> * gcc/config/aarch64/aarch64.c (generic_tunings): Add AES fusion.
>
> --
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 
> 728ce7029f1e2b5161d9f317d10e564dd5a5f472..c8cf7169a5d387de336920b50c83761dc0c96f3a
>  100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -536,7 +536,7 @@ static const struct tune_params generic_tunings =
>_approx_modes,
>4, /* memmov_cost  */
>2, /* issue_rate  */
> -  AARCH64_FUSE_NOTHING, /* fusible_ops  */
> +  (AARCH64_FUSE_AES_AESMC), /* fusible_ops  */
>8,   /* function_align.  */
>8,   /* jump_align.  */
>4,   /* loop_align.  */

[PATCH][AArch64] Enable AES fusion with -mcpu=generic

[Wilco Dijkstra]

Many supported cores implement fusion of AES instructions.  When fusion
happens it can give a significant performance gain.  If not, scheduling
fusion candidates next to each other has almost no effect on performance.
Due to the high benefit/low cost it makes sense to enable AES fusion with
-mcpu=generic so that cores that support it always benefit.  Any objections?

Bootstrapped on AArch64, no regressions.

ChangeLog:
2017-03-16  Wilco Dijkstra  

* gcc/config/aarch64/aarch64.c (generic_tunings): Add AES fusion.

--
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 
728ce7029f1e2b5161d9f317d10e564dd5a5f472..c8cf7169a5d387de336920b50c83761dc0c96f3a
 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -536,7 +536,7 @@ static const struct tune_params generic_tunings =
   _approx_modes,
   4, /* memmov_cost  */
   2, /* issue_rate  */
-  AARCH64_FUSE_NOTHING, /* fusible_ops  */
+  (AARCH64_FUSE_AES_AESMC), /* fusible_ops  */
   8,   /* function_align.  */
   8,   /* jump_align.  */
   4,   /* loop_align.  */