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.
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.
Thanks,
Richard
