On Thu, May 1, 2025 at 1:21 PM Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> Uros Bizjak <ubiz...@gmail.com> writes:
> > On Wed, Apr 30, 2025 at 11:31 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> >>
> >> On Wed, Apr 30, 2025 at 7:37 PM Uros Bizjak <ubiz...@gmail.com> wrote:
> >> >
> >> > On Tue, Apr 29, 2025 at 11:40 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> >> > >
> >> > > AREG, DREG, CREG and AD_REGS are kept in ix86_class_likely_spilled_p to
> >> > > avoid the following regressions with
> >> > >
> >> > > $ make check RUNTESTFLAGS="--target_board='unix{-m32,}'"
> >> > >
> >> > > FAIL: gcc.dg/pr105911.c (internal compiler error: in
> >> > > lra_split_hard_reg_for, at lra-assigns.cc:1863)
> >> > > FAIL: gcc.dg/pr105911.c (test for excess errors)
> >> > > FAIL: gcc.target/i386/avx512vl-stv-rotatedi-1.c scan-assembler-times
> >> > > vpro[lr]q 29
> >> > > FAIL: gcc.target/i386/bt-7.c scan-assembler-not and[lq][ \t]
> >> > > FAIL: gcc.target/i386/naked-4.c scan-assembler-not %[re]bp
> >> > > FAIL: gcc.target/i386/pr107548-1.c scan-assembler-not addl
> >> > > FAIL: gcc.target/i386/pr107548-1.c scan-assembler-times \tv?movd\t 3
> >> > > FAIL: gcc.target/i386/pr107548-1.c scan-assembler-times v?paddd 6
> >> > > FAIL: gcc.target/i386/pr107548-2.c scan-assembler-not \taddq\t
> >> > > FAIL: gcc.target/i386/pr107548-2.c scan-assembler-times v?paddq 2
> >> > > FAIL: gcc.target/i386/pr119171-1.c (test for excess errors)
> >> > > FAIL: gcc.target/i386/pr57189.c scan-assembler-not movaps
> >> > > FAIL: gcc.target/i386/pr57189.c scan-assembler-not movaps
> >> > > FAIL: gcc.target/i386/pr78904-1b.c scan-assembler [ \t]andb
> >> > > FAIL: gcc.target/i386/pr78904-1b.c scan-assembler [ \t]orb
> >> > > FAIL: gcc.target/i386/pr78904-7b.c scan-assembler-not movzbl
> >> > > FAIL: gcc.target/i386/pr78904-7b.c scan-assembler [ \t]orb
> >> > > FAIL: gcc.target/i386/pr91188-2c.c scan-assembler [ \t]andw
> >> > >
> >> > > Tested with glibc master branch at
> >> > >
> >> > > commit ccdb68e829a31e4cda8339ea0d2dc3e51fb81ba5
> >> > > Author: Samuel Thibault <samuel.thiba...@ens-lyon.org>
> >> > > Date: Sun Mar 2 15:16:45 2025 +0100
> >> > >
> >> > > htl: move pthread_once into libc
> >> > >
> >> > > and built Linux kernel 6.13.5 on x86-64.
> >> > >
> >> > > PR target/119083
> >> > > * config/i386/i386.cc (ix86_class_likely_spilled_p): Remove
> >> > > CREG
> >> > > and BREG.
> >> >
> >> > The commit message doesn't reflect what the patch does.
> >> >
> >> > OTOH, this is a very delicate part of the compiler. You are risking RA
> >> > failures, the risk/benefit ratio is very high, so I wouldn't touch it
> >> > without clear benefits. Do you have a concrete example where declaring
> >> > BREG as spilled hurts?
> >> >
> >>
> >> I can find a testcase to show the improvement. But I am not sure if
> >> it is what you were asking for. ix86_class_likely_spilled_p was
> >> CLASS_LIKELY_SPILLED_P which was added by
> >>
> >> commit f5316dfe88b8d1b8d3012c1f75349edf2ba1bdde
> >> Author: Michael Meissner <meiss...@gcc.gnu.org>
> >> Date: Thu Sep 8 17:59:18 1994 +0000
> >>
> >> Add support for -mreg-alloc=<xxx>
> >>
> >> This option is long gone and there is no test in GCC testsuite to show
> >> that BX should be in ix86_class_likely_spilled_p. On x86-64, BX is just
> >> another callee-saved register, not different from R12. On i386, BX is
> >> used as the PIC register. But today RA may pick a different register if
> >> PLT isn't involved. This patch gives RA a little bit more freedom.
> >
> > In the past *decades* CLASS_LIKELY_SPILLED_P was repurposed to signal
> > the compiler that some extra care is needed with listed classes. On
> > i386 and x86_64 these include single register classes that represent
> > "architectural" registers (registers with assigned role). The compiler
> > does take care to not extend life times of CLASS_LIKELY_SPILLED_P
> > classes too much to avoid reload failures in cases where instruction
> > with C_L_S_P class (e.g. shifts with %cl register) is emitted between
> > unrelated register def and use.
> >
> > Registers in these classes won't disappear from the pool of available
> > registers and RA can still use them, but with some extra care. So,
> > without clear and noticeable benefits, single register classes remain
> > declared as CLASS_LIKELY_SPILLED_P to avoid possible reload failures.
>
> I agree with what you say. But even so, part of me thinks it might be
> interesting to see how much breakage there would be if we stopped using
> likely-spilled for requirements that the RA can see for itself (through
> constraints). As the commits quoted show, this is an ancient mechanism
> and a lot of received wisdom dates from reload and pre-IRA RA.
A quick test for improved RA capabilities would be to remove CREG from
C_L_S_P. Integer shift instructions use %cl as the "architectural"
count register, so the allocator has to take extra care with its
allocation. These instructions are heavily used, so there would be
plenty of cases that exercise this functionality.
We should also not forget inline asms, these are also heavy users of
single register classes, so even if x86_64 doesn't use BREG class
explicitly, it should stay in C_L_S_P.
> I would expect IRA to handle the situation better (e.g. because it
> replaces scratches with new pseudos, so that it can allocate scratches
> directly rather than leaving it to reload/LRA). And I would expect
> LRA to avoid fatal spill failures in cases that reload couldn't.
In the past, a common wisdom was to list all single-register classes
in C_L_S_P. Please also note that it is not only the register
allocator that uses C_L_S_P.
Thanks,
Uros,
>
> Thanks,
> Richard
