[0/6] ira: Fix performance regression in exchange2 [PR98782]

Message ID mptr19k2a92.fsf@arm.com
Headers
Series ira: Fix performance regression in exchange2 [PR98782] |

Message

Richard Sandiford Jan. 6, 2022, 2:45 p.m. UTC
  This series of patches recovers the exchange2 performance lost in the
GCC 11 timeframe (at least on aarch64 and Power9 -- thanks Pat for
testing the latter).

There are 6 patches, split into two groups of 3.  The first 3 are just
preparatory patches, although patch 2 does contain a minor bug fix.
The other 3 patches are the ones that together fix the regression.

I realise this is a bit invasive for stage 3.  However, the series is
fixing a large regression in an important benchmark and AFAIK there are
no known acceptable mitigations that we could apply instead.  I think
the series is also working with concepts that already exist in IRA:
it's really about tweaking the cost model used to control them.

We also still have at least 3 months (more realistically 4 months) of
testing before GCC 12 is released.  So perhaps one option would be to
apply any approved version of the series now, but with the understanding
that if there's significant fallout (more than a handful of small tweaks
or fixes), we would simply revert the patches rather than trying to
rework them in-situ.  The series is confined to IRA so reverting it
should be simple.  Would that be OK?

Each patch bootstrapped & regression-tested individually on
aarch64-linux-gnu.  Also tested as a series on aarch64_be-elf,
arm-linux-gnueabihf, powerpc64le-linux-gnu, and x86_64-linux-gnu.

Richard
  

Comments

Vladimir Makarov Jan. 7, 2022, 2:38 p.m. UTC | #1
On 2022-01-06 09:45, Richard Sandiford wrote:
> This series of patches recovers the exchange2 performance lost in the
> GCC 11 timeframe (at least on aarch64 and Power9 -- thanks Pat for
> testing the latter).
>
> There are 6 patches, split into two groups of 3.  The first 3 are just
> preparatory patches, although patch 2 does contain a minor bug fix.
> The other 3 patches are the ones that together fix the regression.
>
> I realise this is a bit invasive for stage 3.  However, the series is
> fixing a large regression in an important benchmark and AFAIK there are
> no known acceptable mitigations that we could apply instead.  I think
> the series is also working with concepts that already exist in IRA:
> it's really about tweaking the cost model used to control them.
>
> We also still have at least 3 months (more realistically 4 months) of
> testing before GCC 12 is released.  So perhaps one option would be to
> apply any approved version of the series now, but with the understanding
> that if there's significant fallout (more than a handful of small tweaks
> or fixes), we would simply revert the patches rather than trying to
> rework them in-situ.  The series is confined to IRA so reverting it
> should be simple.  Would that be OK?

Richard. thank you for working on these issues.

I don't think there is a problem with the GCC development stage here.  
These are patches solving existing PR(s).  Of course it is better to do 
such changes earlier at the stage3, so IMHO the timing is right.

I don't expect that the changes will result in serious problems like 
wrong code generation or RA crashes as they are about improving RA 
heuristics.  They can result in new GCC test failures on some targets 
(we have many overconstrained tests expecting an exact GCC output).  If 
we are overwhelmed with the new failures we can revert the patches.

The first 3 patches are ok to commit.  I'll look at the rest 3 ones this 
weekend and write you my opinion on Monday.  I don't think there will be 
a problem with the last 3 patches.  They are clearly improving RA 
heuristics.  I just need some time to think about them.

Thank you again for picking this difficult PR and working on it.

> Each patch bootstrapped & regression-tested individually on
> aarch64-linux-gnu.  Also tested as a series on aarch64_be-elf,
> arm-linux-gnueabihf, powerpc64le-linux-gnu, and x86_64-linux-gnu.
>