| Message ID | 20110923081247.GQ2687@tyan-ft48-01.lab.bos.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Sep 23, 2011 at 10:12 AM, Jakub Jelinek <jakub@redhat.com> wrote: >> > --- gcc/config/i386/i386.c.jj 2011-09-02 16:29:38.000000000 +0200 >> > +++ gcc/config/i386/i386.c 2011-09-07 21:57:52.000000000 +0200 >> > @@ -18304,6 +18304,11 @@ ix86_prepare_sse_fp_compare_args (rtx de >> > { >> > rtx tmp; >> > >> > + /* AVX supports all the needed comparisons, no need to swap arguments >> > + nor help reload. */ >> > + if (TARGET_AVX) >> > + return code; >> > + >> >> Unfortunately, this part prevents generation of vmin/vmax instructions >> for TARGET_AVX. In ix86_expand_fp_movcc, we call >> ix86_prepare_sse_fp_compare_args, where we previously converted GT >> into LT. LT is recognized in ix86_expand_sse_fp_minmax as valid >> operand for MIN/MAX, whereas GT is not. >> >> I'm not sure if we can swap operands in ix86_expand_sse_fp_minmax, >> there is a scary comment in front of ix86_expand_sse_fp_minmax w.r.t. >> to IEEE safety. > > swap_condition is documented to be IEEE safe: > /* Similar, but return the code when two operands of a comparison are swapped. > This IS safe for IEEE floating-point. */ > > So, do you prefer this? Yes, since the operands can be swapped, this looks OK to me. Also, the comments are very informative. > 2011-09-23 Jakub Jelinek <jakub@redhat.com> > > * config/i386/i386.c (ix86_prepare_sse_fp_compare_args): For > GE/GT/UNLE/UNLT swap arguments and condition even for TARGET_AVX. > > * gcc.target/i386/avxfp-1.c: New test. > * gcc.target/i386/avxfp-2.c: New test. OK. Thanks, Uros.
--- gcc/config/i386/i386.c.jj 2011-09-22 18:55:45.000000000 +0200 +++ gcc/config/i386/i386.c 2011-09-23 10:04:35.000000000 +0200 @@ -18739,15 +18739,13 @@ ix86_prepare_sse_fp_compare_args (rtx de { rtx tmp; - /* AVX supports all the needed comparisons, no need to swap arguments - nor help reload. */ - if (TARGET_AVX) - return code; - switch (code) { case LTGT: case UNEQ: + /* AVX supports all the needed comparisons. */ + if (TARGET_AVX) + break; /* We have no LTGT as an operator. We could implement it with NE & ORDERED, but this requires an extra temporary. It's not clear that it's worth it. */ @@ -18764,6 +18762,9 @@ ix86_prepare_sse_fp_compare_args (rtx de case NE: case UNORDERED: case ORDERED: + /* AVX has 3 operand comparisons, no need to swap anything. */ + if (TARGET_AVX) + break; /* For commutative operators, try to canonicalize the destination operand to be first in the comparison - this helps reload to avoid extra moves. */ @@ -18775,8 +18776,10 @@ ix86_prepare_sse_fp_compare_args (rtx de case GT: case UNLE: case UNLT: - /* These are not supported directly. Swap the comparison operands - to transform into something that is supported. */ + /* These are not supported directly before AVX, and furthermore + ix86_expand_sse_fp_minmax only optimizes LT/UNGE. Swap the + comparison operands to transform into something that is + supported. */ tmp = *pop0; *pop0 = *pop1; *pop1 = tmp; --- gcc/testsuite/gcc.target/i386/avxfp-1.c.jj 2011-09-23 10:07:48.000000000 +0200 +++ gcc/testsuite/gcc.target/i386/avxfp-1.c 2011-09-23 10:08:02.000000000 +0200 @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mavx -mfpmath=sse" } */ +/* { dg-final { scan-assembler "vmaxsd" } } */ +/* { dg-final { scan-assembler "vminsd" } } */ +double x; +t() +{ + x=x>5?x:5; +} + +double x; +q() +{ + x=x<5?x:5; +} --- gcc/testsuite/gcc.target/i386/avxfp-2.c.jj 2011-09-23 10:07:51.000000000 +0200 +++ gcc/testsuite/gcc.target/i386/avxfp-2.c 2011-09-23 10:08:17.000000000 +0200 @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mavx -mfpmath=sse" } */ +/* { dg-final { scan-assembler "vmaxsd" } } */ +/* { dg-final { scan-assembler "vminsd" } } */ +double x; +q() +{ + x=x<5?5:x; +} + +double x; +q1() +{ + x=x>5?5:x; +}