Re: [PATCH] combine: Try harder to form zero_extends [PR106594]

[Segher Boessenkool <segher@kernel.crashing.org>]

On Mon, Mar 06, 2023 at 04:34:59PM +0000, Richard Sandiford wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> > On Mon, Mar 06, 2023 at 03:08:00PM +0000, Richard Sandiford via Gcc-patches wrote:
> >> Segher Boessenkool <segher@kernel.crashing.org> writes:
> >> > On Mon, Mar 06, 2023 at 12:47:06PM +0000, Richard Sandiford wrote:
> >> >> How about the patch below?
> >> >
> >> > What about it?  What would make it any better than the previous?
> >> 
> >> It does what Jeff suggested in the quoted message: work within the existing
> >> extract/make_compound_operation scheme rather than try to opt out of it.
> >
> > That still feels like it could be risky in stage4, affecting various other
> > FEs which would be expecting ANDs in their patterns instead of *_EXTEND, no?
> > So, at least we'd need something like Segher ran to test it on various
> > targets on Linux kernel (but would be really nice to get also i?86/x86_64).
> >
> > If it were on the aarch64 side just one pattern, I'd suggest a pre-reload
> > splitter, but unfortunately the sign extends (and zero extends?) are handled
> > in legitimate address hook.  Also, I see nonzero_bits only called in
> > rs6000's combine splitter and s390'x canonicalize_comparison target hook,
> > nowhere else in the backends, so I think using it outside of the combiner
> > isn't desirable.
> >
> > Could we have a target hook to canonicalize memory addresses for combiner,
> > like we have that targetm.canonicalize_comparison ?
> 
> I don't think a hook makes sense as a long-term design decision.
> The canonicalisation we're doing here isn't logically AArch64-specific,
> and in general, the less variation in RTL rules between targets, the better.

C1 is trunk, C2 is the previous patch, C3 is this one:

$ perl sizes.pl --percent C[123]
                    C1        C2        C3
       alpha   7082243  100.066%  100.000%
         arc   4207975  100.015%  100.000%
         arm  11518624  100.008%  100.000%
       arm64  24514565  100.067%  100.033%
       armhf  16661684  100.098%  100.000%
        csky   4031841  100.002%  100.000%
        i386         0         0         0
        ia64  20354295  100.029%  100.000%
        m68k   4394084  100.023%  100.000%
  microblaze   6549965  100.014%  100.000%
        mips  10684680  100.024%  100.000%
      mips64   8171850  100.002%  100.000%
       nios2   4356713  100.012%  100.000%
    openrisc   5010570  100.003%  100.000%
      parisc   8406294  100.002%  100.000%
    parisc64         0         0         0
     powerpc  11104901   99.992%  100.000%
   powerpc64  24532358  100.057%  100.000%
 powerpc64le  21293219  100.062%  100.000%
     riscv32   2028474  100.131%  100.000%
     riscv64   9515453  100.120%  100.000%
        s390  20519612  100.279%  100.000%
          sh         0         0         0
     shnommu   1840960  100.012%  100.000%
       sparc   5314422  100.004%  100.000%
     sparc64   7964129   99.992%  100.000%
      x86_64         0         0         0
      xtensa   2925723  100.070%  100.000%

It does absolutely nothing for all those other targets you say it is
beneficial for; and it is a net *negative* for aarch64 itself!


Segher