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
