Hi Segher,

On 08/10/2020 15:20, Segher Boessenkool wrote:
> On Thu, Oct 08, 2020 at 11:21:26AM +0100, Alex Coplan wrote:
> > Ping. The kernel is still broken on AArch64.
> 
> You *cannot* fix a correctness bug with a combine addition.

https://gcc.gnu.org/pipermail/gcc-patches/2020-September/555158.html
explains why we do precisely that.

Also, as your own testing confirmed, the patch does indeed fix the issue.

> So please fix the target bug first.

I think the problem here -- the reason that we're talking past each
other -- is that there are (at least) two parts of the codebase that can
be blamed for the ICE here:

1. aarch64: "The insn is unrecognized, so it's a backend bug
(missing pattern or similar)."

2. combine: "Combine produces a non-canonical insn, so the backend
(correctly) doesn't recognise it, and combine is at fault."

Now I initially (naively) took interpretation 1 here and tried to fix
the ICE by adding a pattern to recognise the sign_extract insn that
combine is producing here:
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553605.html

Howerver, in the review of that patch, Richard opened my eyes to
interpretation 2, which in hindsight is clearly a better way to fix the
issue.

Combine already does the canonicalisation for the (ashift x n) case, so
it seems like an obvious improvement to do the same for the (mult x 2^n)
case, as this is how shifts are represented inside mem rtxes.

Again, please see Richard's comments here:
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/554518.html

> 
> I haven't had time to look at your patch yet, sorry.

Not to worry. Hopefully this clears up any confusion around what we're
trying to do here and why.

Thanks,
Alex

Reply via email to