[PING][PATCH v2] combine: Don't turn (mult (extend x) 2^n) into extract [PR96998]
Alex Coplan
alex.coplan@arm.com
Thu Oct 15 09:14:38 GMT 2020
Hi Segher,
On 12/10/2020 12:14, Segher Boessenkool wrote:
> On Mon, Oct 12, 2020 at 05:19:58PM +0100, Richard Sandiford wrote:
> >
> > I agree there's a target bug here. Please see the explanation I posted
> > in: https://gcc.gnu.org/pipermail/gcc-patches/2020-September/554518.html
> > (especially the first sentence quoted below :-)).
> >
> > The situation as things stand is that aarch64 has a bug: it accepts
> > an odd sign_extract representation of addresses, but doesn't accept
> > that same odd form of address as an LEA. We have two options:
> >
> > (a) add back instructions that recognise the odd form of LEA, or
> > (b) remove the code that accepts the odd addresses
> >
> > I think (b) is the way to go here.
>
> Either seems to be fine.
>
> > But doing that on its own
> > would regress code quality. The reason we recognised the odd
> > addresses in the first place was because that was the rtl that
> > combine happened to generate for an important case.
> >
> > So if we go for (b) but fix the aarch64 bug strictly before the
> > combine patch, we would need to:
>
> This is necessary to be able to evaluate what such a combine patch
> does in practice -- so there is no other way.
>
> > (1) Apply the target fix and adjust the testsuite markup to make sure
> > that the git commit doesn't regress anyone's test results.
>
> It is normal to regress the testsuite for a little while.
>
> > (2) Apply the combine patch and revert the testsuite markup changes
> > from (1).
> >
> > That seems like make-work, and would still show as a blip for
> > people doing performance tracking.
>
> Yes, that is make-work. Just regress the testsuite.
>
> You do not even have to apply the target patch first (but you need to
> send it as separate patch, so that other people can test it!)
>
> > If you prefer, we could fix the aarch64 bug and patch combine as a
> > single commit. See:
> >
> > https://gcc.gnu.org/pipermail/gcc-patches/2020-September/554257.html
> >
> > for the full patch, including the aarch64 bugfix.
>
> I need separate patches, so that I can see what the current combine
> does, without ICEing all over. That is all. Send it as a series of two
> patches, or something.
Apologies for the misunderstanding and my inaccurate summary in the previous
email, there is of course a target bug here as well.
I've sent the patches as a two-patch series where the first patch fixes the
AArch64 bug and the second restores code quality with the patch to combine:
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/556237.html
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/556238.html
Thanks,
Alex
More information about the Gcc-patches
mailing list