[PATCH][AArch64] PR84114: Avoid reassociating FMA

[PATCH][AArch64] PR84114: Avoid reassociating FMA

[Wilco Dijkstra]

As discussed in the PR, the reassociation phase runs before FMAs are formed
and so can significantly reduce FMA opportunities.  Although reassociation
could be switched off, it helps in many cases, so a better alternative is to
only avoid reassociation of floating point additions.  This fixes the testcase
and gives 1% speedup on SPECFP2017, fixing the performance regression.

OK for commit?

ChangeLog:
2018-02-23  Wilco Dijkstra  <wdijkstr@arm.com>

	PR tree-optimization/84114
	* config/aarch64/aarch64.c (aarch64_reassociation_width)
	Avoid reassociation of FLOAT_MODE addition.
--

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index b3d5fde171920e5759046a4bd61cfcf9eb78d7dd..5f9541cf700aaf18c1f1ac73054614e2932781e4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -1109,15 +1109,16 @@ aarch64_min_divisions_for_recip_mul (machine_mode mode)
   return aarch64_tune_params.min_div_recip_mul_df;
 }
 
+/* Return the reassociation width of treeop OPC with mode MODE.  */
 static int
-aarch64_reassociation_width (unsigned opc ATTRIBUTE_UNUSED,
-			     machine_mode mode)
+aarch64_reassociation_width (unsigned opc, machine_mode mode)
 {
   if (VECTOR_MODE_P (mode))
     return aarch64_tune_params.vec_reassoc_width;
   if (INTEGRAL_MODE_P (mode))
     return aarch64_tune_params.int_reassoc_width;
-  if (FLOAT_MODE_P (mode))
+  /* Avoid reassociating floating point addition so we emit more FMAs.  */
+  if (FLOAT_MODE_P (mode) && opc != PLUS_EXPR)
     return aarch64_tune_params.fp_reassoc_width;
   return 1;
 }

Re: [PATCH][AArch64] PR84114: Avoid reassociating FMA

[James Greenhalgh]

On Thu, Feb 22, 2018 at 11:38:03AM +0000, Wilco Dijkstra wrote:
> As discussed in the PR, the reassociation phase runs before FMAs are formed
> and so can significantly reduce FMA opportunities.  Although reassociation
> could be switched off, it helps in many cases, so a better alternative is to
> only avoid reassociation of floating point additions.  This fixes the testcase
> and gives 1% speedup on SPECFP2017, fixing the performance regression.
> 
> OK for commit?

This is OK as a fairly safe fix for stage 4. We should fix reassociation
properly in GCC 9.

Thanks,
James

> 
> ChangeLog:
> 2018-02-23  Wilco Dijkstra  <wdijkstr@arm.com>
> 
> 	PR tree-optimization/84114
> 	* config/aarch64/aarch64.c (aarch64_reassociation_width)
> 	Avoid reassociation of FLOAT_MODE addition.
> --
> 
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index b3d5fde171920e5759046a4bd61cfcf9eb78d7dd..5f9541cf700aaf18c1f1ac73054614e2932781e4 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -1109,15 +1109,16 @@ aarch64_min_divisions_for_recip_mul (machine_mode mode)
>    return aarch64_tune_params.min_div_recip_mul_df;
>  }
>  
> +/* Return the reassociation width of treeop OPC with mode MODE.  */
>  static int
> -aarch64_reassociation_width (unsigned opc ATTRIBUTE_UNUSED,
> -			     machine_mode mode)
> +aarch64_reassociation_width (unsigned opc, machine_mode mode)
>  {
>    if (VECTOR_MODE_P (mode))
>      return aarch64_tune_params.vec_reassoc_width;
>    if (INTEGRAL_MODE_P (mode))
>      return aarch64_tune_params.int_reassoc_width;
> -  if (FLOAT_MODE_P (mode))
> +  /* Avoid reassociating floating point addition so we emit more FMAs.  */
> +  if (FLOAT_MODE_P (mode) && opc != PLUS_EXPR)
>      return aarch64_tune_params.fp_reassoc_width;
>    return 1;
>  }

Re: [PATCH][AArch64] PR84114: Avoid reassociating FMA

[Richard Biener]

On Mon, Feb 26, 2018 at 11:25 PM, James Greenhalgh
<james.greenhalgh@arm.com> wrote:
> On Thu, Feb 22, 2018 at 11:38:03AM +0000, Wilco Dijkstra wrote:
>> As discussed in the PR, the reassociation phase runs before FMAs are formed
>> and so can significantly reduce FMA opportunities.  Although reassociation
>> could be switched off, it helps in many cases, so a better alternative is to
>> only avoid reassociation of floating point additions.  This fixes the testcase
>> and gives 1% speedup on SPECFP2017, fixing the performance regression.
>>
>> OK for commit?
>
> This is OK as a fairly safe fix for stage 4. We should fix reassociation
> properly in GCC 9.

It happens that on some targets doing two FMAs in parallel and one
non-FMA operation merging them is faster than chaining three FMAs...

But yes, somewhere I suggested that FMA detection should/could be
integrated with reassociation.

Richard.

> Thanks,
> James
>
>>
>> ChangeLog:
>> 2018-02-23  Wilco Dijkstra  <wdijkstr@arm.com>
>>
>>       PR tree-optimization/84114
>>       * config/aarch64/aarch64.c (aarch64_reassociation_width)
>>       Avoid reassociation of FLOAT_MODE addition.
>> --
>>
>> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
>> index b3d5fde171920e5759046a4bd61cfcf9eb78d7dd..5f9541cf700aaf18c1f1ac73054614e2932781e4 100644
>> --- a/gcc/config/aarch64/aarch64.c
>> +++ b/gcc/config/aarch64/aarch64.c
>> @@ -1109,15 +1109,16 @@ aarch64_min_divisions_for_recip_mul (machine_mode mode)
>>    return aarch64_tune_params.min_div_recip_mul_df;
>>  }
>>
>> +/* Return the reassociation width of treeop OPC with mode MODE.  */
>>  static int
>> -aarch64_reassociation_width (unsigned opc ATTRIBUTE_UNUSED,
>> -                          machine_mode mode)
>> +aarch64_reassociation_width (unsigned opc, machine_mode mode)
>>  {
>>    if (VECTOR_MODE_P (mode))
>>      return aarch64_tune_params.vec_reassoc_width;
>>    if (INTEGRAL_MODE_P (mode))
>>      return aarch64_tune_params.int_reassoc_width;
>> -  if (FLOAT_MODE_P (mode))
>> +  /* Avoid reassociating floating point addition so we emit more FMAs.  */
>> +  if (FLOAT_MODE_P (mode) && opc != PLUS_EXPR)
>>      return aarch64_tune_params.fp_reassoc_width;
>>    return 1;
>>  }

Re: [PATCH][AArch64] PR84114: Avoid reassociating FMA

[Wilco Dijkstra]

Richard Biener <richard.guenther@gmail.com>

> It happens that on some targets doing two FMAs in parallel and one
> non-FMA operation merging them is faster than chaining three FMAs...

Like I mentioned in the PR, long chains should be broken, but for that we need a new parameter to state how long a chain may be before it is split. The issue today is that it splits even very short chains, removing beneficial FMAs.

> But yes, somewhere I suggested that FMA detection should/could be
> integrated with reassociation.

Absolutely.

Wilco

Re: [PATCH][AArch64] PR84114: Avoid reassociating FMA

[Aaron Sawdey]

On Tue, 2018-02-27 at 14:21 +0000, Wilco Dijkstra wrote:
> Richard Biener <richard.guenther@gmail.com>
> 
> > It happens that on some targets doing two FMAs in parallel and one
> > non-FMA operation merging them is faster than chaining three
> > FMAs...
> 
> Like I mentioned in the PR, long chains should be broken, but for
> that we need a new parameter to state how long a chain may be before
> it is split. The issue today is that it splits even very short
> chains, removing beneficial FMAs.
> 
> > But yes, somewhere I suggested that FMA detection should/could be
> > integrated with reassociation.

I'd also like to see some work here. 

Doing two FMA in parallel and then a non-FMA merge is faster on ppc,
but it would be nice if the target had some more control of exactly how
this happens.

Also doing parallel reassociation increases register pressure so it
would be nice to be able to avoid causing issues as a result of that.

-- 
Aaron Sawdey, Ph.D.  acsawdey@linux.vnet.ibm.com
050-2/C113  (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain