Re: [PATCH v3 3/4] ifcvt: Handle multiple rewired regs and refactor noce_convert_multiple_sets

[Manolis Tsamis <manolis.tsamis@vrull.eu>]

Hi Jeff,

Indeed, that sounds like a good idea. I will make this separate and
send it after the required testing.
I'll see what can be done about a testcase.

Best,
Manolis

On Sat, Nov 11, 2023 at 1:20 AM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 8/30/23 04:13, Manolis Tsamis wrote:
> > The existing implementation of need_cmov_or_rewire and
> > noce_convert_multiple_sets_1 assumes that sets are either REG or SUBREG.
> > This commit enchances them so they can handle/rewire arbitrary set statements.
> >
> > To do that a new helper struct noce_multiple_sets_info is introduced which is
> > used by noce_convert_multiple_sets and its helper functions. This results in
> > cleaner function signatures, improved efficientcy (a number of vecs and hash
> > set/map are replaced with a single vec of struct) and simplicity.
> >
> > gcc/ChangeLog:
> >
> >       * ifcvt.cc (need_cmov_or_rewire): Renamed init_noce_multiple_sets_info.
> >       (init_noce_multiple_sets_info): Initialize noce_multiple_sets_info.
> >       (noce_convert_multiple_sets_1): Use noce_multiple_sets_info and handle
> >       rewiring of multiple registers.
> >       (noce_convert_multiple_sets): Updated to use noce_multiple_sets_info.
> >       * ifcvt.h (struct noce_multiple_sets_info): Introduce new struct
> >       noce_multiple_sets_info to store info for noce_convert_multiple_sets.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc.target/aarch64/ifcvt_multiple_sets_rewire.c: New test.
> So this seems like (in theory) it could move forward independently.  The
> handling of arbitrary statements code wouldn't be exercised yet, but
> that's OK IMHO as I don't think anyone is fundamentally against trying
> to handle additional kinds of statements.
>
> So my suggestion would be to bootstrap & regression test this
> independently.  AFAICT this should have no functional change if it were
> to go in on its own.  Note the testsuite entry might not be applicable
> if this were to go in on its own and would need to roll into another
> patch in the series.
>
>
> Jeff