| Message ID | Zv++UP3egqbdh8GK@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | i386: Fix up *minmax<mode>3_2 splitter [PR116925] | expand |
On Fri, Oct 4, 2024 at 12:07 PM Jakub Jelinek <jakub@redhat.com> wrote: > > Hi! > > While *minmax<mode>3_1 correctly uses > if (MEM_P (operands[1])) > operands[1] = force_reg (<MODE>mode, operands[1]); > to ensure operands[1] is not a MEM, *minmax<mode>3_2 does it wrongly > by calling force_reg but ignoring its return value. > > The following borderingly obvious patch fixes that, bootstrapped/regtested > on x86_64-linux and i686-linux, ok for trunk? > > Didn't find similar other errors in the backend with force_reg calls. > > 2024-10-04 Jakub Jelinek <jakub@redhat.com> > > PR target/116925 > * config/i386/sse.md (*minmax<mode>3_2): Assign force_reg result > back to operands[2] instead of throwing it away. > > * g++.target/i386/avx-pr116925.C: New test. OK, even obvious. Thanks, Uros. > > --- gcc/config/i386/sse.md.jj 2024-10-01 09:38:57.000000000 +0200 > +++ gcc/config/i386/sse.md 2024-10-03 17:33:12.071507421 +0200 > @@ -3269,7 +3269,7 @@ (define_insn_and_split "*minmax<mode>3_2 > u = UNSPEC_IEEE_MAX; > > if (MEM_P (operands[2])) > - force_reg (<MODE>mode, operands[2]); > + operands[2] = force_reg (<MODE>mode, operands[2]); > rtvec v = gen_rtvec (2, operands[2], operands[1]); > rtx tmp = gen_rtx_UNSPEC (<MODE>mode, v, u); > emit_move_insn (operands[0], tmp); > --- gcc/testsuite/g++.target/i386/avx-pr116925.C.jj 2024-10-03 17:36:10.124061111 +0200 > +++ gcc/testsuite/g++.target/i386/avx-pr116925.C 2024-10-03 17:35:26.805656671 +0200 > @@ -0,0 +1,12 @@ > +// PR target/116925 > +// { dg-do compile } > +// { dg-options "-O2 -mavx -ffloat-store" } > + > +typedef float V __attribute__((vector_size (16))); > +V a, b, c; > + > +void > +foo () > +{ > + c = a > b ? a : b; > +} > > Jakub >
--- gcc/config/i386/sse.md.jj 2024-10-01 09:38:57.000000000 +0200 +++ gcc/config/i386/sse.md 2024-10-03 17:33:12.071507421 +0200 @@ -3269,7 +3269,7 @@ (define_insn_and_split "*minmax<mode>3_2 u = UNSPEC_IEEE_MAX; if (MEM_P (operands[2])) - force_reg (<MODE>mode, operands[2]); + operands[2] = force_reg (<MODE>mode, operands[2]); rtvec v = gen_rtvec (2, operands[2], operands[1]); rtx tmp = gen_rtx_UNSPEC (<MODE>mode, v, u); emit_move_insn (operands[0], tmp); --- gcc/testsuite/g++.target/i386/avx-pr116925.C.jj 2024-10-03 17:36:10.124061111 +0200 +++ gcc/testsuite/g++.target/i386/avx-pr116925.C 2024-10-03 17:35:26.805656671 +0200 @@ -0,0 +1,12 @@ +// PR target/116925 +// { dg-do compile } +// { dg-options "-O2 -mavx -ffloat-store" } + +typedef float V __attribute__((vector_size (16))); +V a, b, c; + +void +foo () +{ + c = a > b ? a : b; +}
Hi! While *minmax<mode>3_1 correctly uses if (MEM_P (operands[1])) operands[1] = force_reg (<MODE>mode, operands[1]); to ensure operands[1] is not a MEM, *minmax<mode>3_2 does it wrongly by calling force_reg but ignoring its return value. The following borderingly obvious patch fixes that, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Didn't find similar other errors in the backend with force_reg calls. 2024-10-04 Jakub Jelinek <jakub@redhat.com> PR target/116925 * config/i386/sse.md (*minmax<mode>3_2): Assign force_reg result back to operands[2] instead of throwing it away. * g++.target/i386/avx-pr116925.C: New test. Jakub