On Sun, Aug 23, 2026 at 9:38 PM Dimitar Dimitrov <[email protected]> wrote:
>
> On Fri, Aug 21, 2026 at 09:43:30AM +0200, Uros Bizjak wrote:
> > On Wed, Aug 19, 2026 at 7:14 PM Dimitar Dimitrov <[email protected]> wrote:
> > >
> > > 8-bit and 16-bit modes are not supported by SSE registers without SSE2.
> > > When asm register variable is declared with such mode, register
> > > allocation fails with an ICE:
> > >
> > >   pr104974.c:5:1: internal compiler error: in gen_rtx_SUBREG, at 
> > > emit-rtl.cc:1047
> > >   ...
> > >   0xe4aad9 emit_spill_move
> > >           /home/mjires/git/GCC/master/gcc/lra-constraints.cc:1373
> > >
> > > Here LRA attempts to create a paradoxical subreg for a spill move:
> > >    (subreg:SI (reg/v:HI 21 xmm1))
> > >
> > > Before the tightened checks  in r16-718-geb2ea476db2182, this
> > > paradoxical subreg was valid.  But when the checks were tightened,
> > > REG_CAN_CHANGE_MODE_P started being called to ensure the hardware
> > > register can change from the inner subreg mode to the outer subreg mode.
> > > In this case - HI and SI modes, which is rejected by
> > > ix86_can_change_mode_class.
> > >
> > > Thus the subreg generated by LRA was declared invalid, resulting in ICE.
> >
> > We don't have 16-bit moves for SSE1.
> >
> > > Fix by modifying ix86_hard_regno_mode_ok to start rejecting 8-bit and
> > > 16-bit modes as invalid for SSE hardware registers when SSE2 is not
> > > available.  There are no functional changes if SSE2 is enabled.
> >
> > 8-bit modes are already rejected, see VALID_SSE_REG_MODE and
> > VALID_SSE2_REG_MODE macros.
> >
> > While the most clean fix would be:
> >
> > @@ -21861,8 +21861,9 @@ ix86_hard_regno_mode_ok (unsigned int regno,
> > machine_mode mode)
> >        /* OImode and AVX modes are available only when AVX is enabled.  */
> >        return ((TARGET_AVX
> >         && VALID_AVX256_REG_OR_OI_MODE (mode))
> > +      || (TARGET_SSE2
> > +  && VALID_SSE2_REG_MODE (mode))
> >        || VALID_SSE_REG_MODE (mode)
> > -      || VALID_SSE2_REG_MODE (mode)
> >        || VALID_MMX_REG_MODE (mode)
> >        || VALID_MMX_REG_MODE_3DNOW (mode));
> >      }
> >
> > but this would invalidate the nearby declaration:
> >
> >       /* We implement the move patterns for all vector modes into and
> > out of SSE registers, even when no operation instructions
> > are available.  */
> >
> > But in case of 16-bit moves, we would need at least 16-bit moves
> > to/from memory, which SSE1 does not provide, so your change is
> > correct.
> >
> > >
> > > This change is mostly not affecting code generation.  I built ffmpeg
> > > with and without this patch, with  "-msse -mno-sse2" options for GCC,
> > > and in some rare occasions LRA resulted in different register
> > > allocations.  This seems to be caused by targetm.modes_tieable_p,
> > > which calls the function patched with this fix: ix86_hard_regno_mode_ok.
> > > I'm not sufficiently familiar with x86 to judge whether the difference
> > > in code generation is a performance regression.
> > >
> > > Boostrapped and regtested x86_64-pc-linux-gnu for C and C++.
> > >
> > > I ran coremark with "-msse -mno-sse2 -O3", and I did not detect
> > > performance regression.
> > >
> > >         PR target/125746
> > >
> > > gcc/ChangeLog:
> > >
> > >         * config/i386/i386.cc (ix86_hard_regno_mode_ok): For SSEv1,
> > >         reject 8-bit and 16-bit modes.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > >         * gcc.target/i386/sse1-char-1.c: New test.
> > >         * gcc.target/i386/sse1-double-1.c: New test.
> > >         * gcc.target/i386/sse1-short-1.c: New test.
> > >
> > > Signed-off-by: Dimitar Dimitrov <[email protected]>
> > > ---
> > >  gcc/config/i386/i386.cc                       | 6 ++++++
> > >  gcc/testsuite/gcc.target/i386/sse1-char-1.c   | 8 ++++++++
> > >  gcc/testsuite/gcc.target/i386/sse1-double-1.c | 8 ++++++++
> > >  gcc/testsuite/gcc.target/i386/sse1-short-1.c  | 8 ++++++++
> > >  4 files changed, 30 insertions(+)
> > >  create mode 100644 gcc/testsuite/gcc.target/i386/sse1-char-1.c
> > >  create mode 100644 gcc/testsuite/gcc.target/i386/sse1-double-1.c
> > >  create mode 100644 gcc/testsuite/gcc.target/i386/sse1-short-1.c
> > >
> ...
> > > diff --git a/gcc/testsuite/gcc.target/i386/sse1-char-1.c 
> > > b/gcc/testsuite/gcc.target/i386/sse1-char-1.c
> > > new file mode 100644
> > > index 00000000000..d328d105713
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/i386/sse1-char-1.c
> > > @@ -0,0 +1,8 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-msse -mno-sse2" } */
> > > +
> > > +void foo() {
> > > +  register char b __asm("%xmm1") = 0; /* { dg-error "register specified 
> > > for 'b' isn't suitable for data type" } */
> > > +
> > > +  asm("" : "+v"(b));
> > > +}
> > > diff --git a/gcc/testsuite/gcc.target/i386/sse1-double-1.c 
> > > b/gcc/testsuite/gcc.target/i386/sse1-double-1.c
> > > new file mode 100644
> > > index 00000000000..a7ba4159ce3
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/i386/sse1-double-1.c
> > > @@ -0,0 +1,8 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-msse -mno-sse2" } */
> > > +
> > > +void foo() {
> > > +  register double b __asm("%xmm1") = 0;
> > > +
> > > +  asm("" : "+v"(b));
> > > +}
> >
> > The above two tests are not needed, char was always rejected and
> > double was always allowed.
>
> My intention was to increase test coverage.  Will drop them for v2.
>
> >
> > > diff --git a/gcc/testsuite/gcc.target/i386/sse1-short-1.c 
> > > b/gcc/testsuite/gcc.target/i386/sse1-short-1.c
> > > new file mode 100644
> > > index 00000000000..ebc27f84bf0
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/i386/sse1-short-1.c
> > > @@ -0,0 +1,8 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-msse -mno-sse2" } */
> > > +
> > > +void foo() {
> > > +  register short b __asm("%xmm1") = 0; /* { dg-error "register specified 
> > > for 'b' isn't suitable for data type" } */
> > > +
> > > +  asm("" : "+v"(b));
> > > +}
> >
> > The above is the test we want, please rename it to pr125746.c and
> > reindent it to GNU standards.
>
> I'll rename it for v2.  Note that I was following the following advice
> from https://gcc.gnu.org/wiki/TestCaseWriting :
>   *do not* name them after the PR number of the bug you fix, please

This applies to gfortran testcases. The general link regarding
testcases is at [1], but it is also a bit obsolete.

[1] https://gcc.gnu.org/wiki/HowToPrepareATestcase

Uros.

Reply via email to