[rs6000] Disable gimple fold for float or double vec_minmax when fast-math is not set

Message ID d98a12d0-6089-315d-9264-62260ec85530@linux.ibm.com
State Superseded
Headers show
Series
  • [rs6000] Disable gimple fold for float or double vec_minmax when fast-math is not set
Related show

Commit Message

Xionghu Luo via Gcc-patches Oct. 12, 2021, 8:57 a.m.
Hi,

    This patch disables gimple folding for float or double vec_min/max when fast-math is not set. It makes vec_min/max conform with the guide.

Bootstrapped and tested on powerpc64le-linux with no regressions. Is this okay for trunk? Any recommendations? Thanks a lot.

    I re-send the patch as previous one is messed up in email thread. Sorry for that.

ChangeLog

2021-08-25 Haochen Gui <guihaoc@linux.ibm.com>

gcc/
         * config/rs6000/rs6000-call.c (rs6000_gimple_fold_builtin):
         Modify the VSX_BUILTIN_XVMINDP, ALTIVEC_BUILTIN_VMINFP,
         VSX_BUILTIN_XVMAXDP, ALTIVEC_BUILTIN_VMAXFP expansions.

gcc/testsuite/
         * gcc.target/powerpc/vec-minmax-1.c: New test.
         * gcc.target/powerpc/vec-minmax-2.c: Likewise.


patch.diff

Comments

Xionghu Luo via Gcc-patches Oct. 12, 2021, 9:57 a.m. | #1
On Tue, Oct 12, 2021 at 10:59 AM HAO CHEN GUI via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>

> Hi,

>

>     This patch disables gimple folding for float or double vec_min/max when fast-math is not set. It makes vec_min/max conform with the guide.

>

> Bootstrapped and tested on powerpc64le-linux with no regressions. Is this okay for trunk? Any recommendations? Thanks a lot.

>

>     I re-send the patch as previous one is messed up in email thread. Sorry for that.


If the VSX/altivec min/max instructions conform to IEEE behavior then
you could instead fold
to .F{MIN,MAX} internal functions and define the f{min,max} optabs.

Otherwise the patch looks correct to me - MIN_EXPR and MAX_EXPR are
not IEEE conforming.
Note a better check would be to use HONOR_NANS/HONOR_SIGNED_ZEROS on
the argument type
(that also works for the integer types with the obvious answer).

Richard.

> ChangeLog

>

> 2021-08-25 Haochen Gui <guihaoc@linux.ibm.com>

>

> gcc/

>          * config/rs6000/rs6000-call.c (rs6000_gimple_fold_builtin):

>          Modify the VSX_BUILTIN_XVMINDP, ALTIVEC_BUILTIN_VMINFP,

>          VSX_BUILTIN_XVMAXDP, ALTIVEC_BUILTIN_VMAXFP expansions.

>

> gcc/testsuite/

>          * gcc.target/powerpc/vec-minmax-1.c: New test.

>          * gcc.target/powerpc/vec-minmax-2.c: Likewise.

>

>

> patch.diff

>

> diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c

> index b4e13af4dc6..90527734ceb 100644

> --- a/gcc/config/rs6000/rs6000-call.c

> +++ b/gcc/config/rs6000/rs6000-call.c

> @@ -12159,6 +12159,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

>         return true;

>       /* flavors of vec_min.  */

>       case VSX_BUILTIN_XVMINDP:

> +    case ALTIVEC_BUILTIN_VMINFP:

> +      if (!flag_finite_math_only || flag_signed_zeros)

> +       return false;

> +      /* Fall through to MIN_EXPR.  */

> +      gcc_fallthrough ();

>       case P8V_BUILTIN_VMINSD:

>       case P8V_BUILTIN_VMINUD:

>       case ALTIVEC_BUILTIN_VMINSB:

> @@ -12167,7 +12172,6 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

>       case ALTIVEC_BUILTIN_VMINUB:

>       case ALTIVEC_BUILTIN_VMINUH:

>       case ALTIVEC_BUILTIN_VMINUW:

> -    case ALTIVEC_BUILTIN_VMINFP:

>         arg0 = gimple_call_arg (stmt, 0);

>         arg1 = gimple_call_arg (stmt, 1);

>         lhs = gimple_call_lhs (stmt);

> @@ -12177,6 +12181,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

>         return true;

>       /* flavors of vec_max.  */

>       case VSX_BUILTIN_XVMAXDP:

> +    case ALTIVEC_BUILTIN_VMAXFP:

> +      if (!flag_finite_math_only || flag_signed_zeros)

> +       return false;

> +      /* Fall through to MAX_EXPR.  */

> +      gcc_fallthrough ();

>       case P8V_BUILTIN_VMAXSD:

>       case P8V_BUILTIN_VMAXUD:

>       case ALTIVEC_BUILTIN_VMAXSB:

> @@ -12185,7 +12194,6 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

>       case ALTIVEC_BUILTIN_VMAXUB:

>       case ALTIVEC_BUILTIN_VMAXUH:

>       case ALTIVEC_BUILTIN_VMAXUW:

> -    case ALTIVEC_BUILTIN_VMAXFP:

>         arg0 = gimple_call_arg (stmt, 0);

>         arg1 = gimple_call_arg (stmt, 1);

>         lhs = gimple_call_lhs (stmt);

> diff --git a/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c b/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c

> new file mode 100644

> index 00000000000..547798fd65c

> --- /dev/null

> +++ b/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c

> @@ -0,0 +1,53 @@

> +/* { dg-do compile { target { powerpc*-*-* } } } */

> +/* { dg-require-effective-target powerpc_p9vector_ok } */

> +/* { dg-options "-O2 -mdejagnu-cpu=power9" } */

> +/* { dg-final { scan-assembler-times {\mxvmaxdp\M} 1 } } */

> +/* { dg-final { scan-assembler-times {\mxvmaxsp\M} 1 } } */

> +/* { dg-final { scan-assembler-times {\mxvmindp\M} 1 } } */

> +/* { dg-final { scan-assembler-times {\mxvminsp\M} 1 } } */

> +

> +/* This test verifies that float or double vec_min/max are bound to

> +   xv[min|max][d|s]p instructions when fast-math is not set.  */

> +

> +

> +#include <altivec.h>

> +

> +#ifdef _BIG_ENDIAN

> +   const int PREF_D = 0;

> +#else

> +   const int PREF_D = 1;

> +#endif

> +

> +double vmaxd (double a, double b)

> +{

> +  vector double va = vec_promote (a, PREF_D);

> +  vector double vb = vec_promote (b, PREF_D);

> +  return vec_extract (vec_max (va, vb), PREF_D);

> +}

> +

> +double vmind (double a, double b)

> +{

> +  vector double va = vec_promote (a, PREF_D);

> +  vector double vb = vec_promote (b, PREF_D);

> +  return vec_extract (vec_min (va, vb), PREF_D);

> +}

> +

> +#ifdef _BIG_ENDIAN

> +   const int PREF_F = 0;

> +#else

> +   const int PREF_F = 3;

> +#endif

> +

> +float vmaxf (float a, float b)

> +{

> +  vector float va = vec_promote (a, PREF_F);

> +  vector float vb = vec_promote (b, PREF_F);

> +  return vec_extract (vec_max (va, vb), PREF_F);

> +}

> +

> +float vminf (float a, float b)

> +{

> +  vector float va = vec_promote (a, PREF_F);

> +  vector float vb = vec_promote (b, PREF_F);

> +  return vec_extract (vec_min (va, vb), PREF_F);

> +}

> diff --git a/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c b/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c

> new file mode 100644

> index 00000000000..4c6f4365830

> --- /dev/null

> +++ b/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c

> @@ -0,0 +1,51 @@

> +/* { dg-do compile { target { powerpc*-*-* } } } */

> +/* { dg-require-effective-target powerpc_p9vector_ok } */

> +/* { dg-options "-O2 -mdejagnu-cpu=power9 -ffast-math" } */

> +/* { dg-final { scan-assembler-times {\mxsmaxcdp\M} 2 } } */

> +/* { dg-final { scan-assembler-times {\mxsmincdp\M} 2 } } */

> +

> +/* This test verifies that float or double vec_min/max can be converted

> +   to scalar comparison when fast-math is set.  */

> +

> +

> +#include <altivec.h>

> +

> +#ifdef _BIG_ENDIAN

> +   const int PREF_D = 0;

> +#else

> +   const int PREF_D = 1;

> +#endif

> +

> +double vmaxd (double a, double b)

> +{

> +  vector double va = vec_promote (a, PREF_D);

> +  vector double vb = vec_promote (b, PREF_D);

> +  return vec_extract (vec_max (va, vb), PREF_D);

> +}

> +

> +double vmind (double a, double b)

> +{

> +  vector double va = vec_promote (a, PREF_D);

> +  vector double vb = vec_promote (b, PREF_D);

> +  return vec_extract (vec_min (va, vb), PREF_D);

> +}

> +

> +#ifdef _BIG_ENDIAN

> +   const int PREF_F = 0;

> +#else

> +   const int PREF_F = 3;

> +#endif

> +

> +float vmaxf (float a, float b)

> +{

> +  vector float va = vec_promote (a, PREF_F);

> +  vector float vb = vec_promote (b, PREF_F);

> +  return vec_extract (vec_max (va, vb), PREF_F);

> +}

> +

> +float vminf (float a, float b)

> +{

> +  vector float va = vec_promote (a, PREF_F);

> +  vector float vb = vec_promote (b, PREF_F);

> +  return vec_extract (vec_min (va, vb), PREF_F);

> +}

>
Xionghu Luo via Gcc-patches Oct. 13, 2021, 7:43 a.m. | #2
Richard,

   Thanks so much for your comments.

   As far as I know, VSX/altivec min/max instructions don't conform with C-Sytle Min/Max Macro. The fold converts it to MIN/MAX_EXPR then it has a chance to be implemented by scalar min/max instructions which conform with C-Sytle Min/Max Macro. That's why I made this patch.

   As to IEEE behavior, do you mean "Minimum and maximum operations" defined in IEEE-754 2019?  If so, I think VSX/altivec min/max instructions don't conform with it. It demands a quite NaN if either operand is a NaN while our instructions don't.

IEEE-754 2019 maximum(x, y) is xif x>y, yif y>x, and a quiet NaN if either operand is a NaN, according to 6.2. For this operation, +0 compares greater than −0. Otherwise (i.e., when x=y and signs are the same) it is either xor y. Actions for xvmaxdp

On 12/10/2021 下午 5:57, Richard Biener wrote:
> On Tue, Oct 12, 2021 at 10:59 AM HAO CHEN GUI via Gcc-patches

> <gcc-patches@gcc.gnu.org> wrote:

>> Hi,

>>

>>      This patch disables gimple folding for float or double vec_min/max when fast-math is not set. It makes vec_min/max conform with the guide.

>>

>> Bootstrapped and tested on powerpc64le-linux with no regressions. Is this okay for trunk? Any recommendations? Thanks a lot.

>>

>>      I re-send the patch as previous one is messed up in email thread. Sorry for that.

> If the VSX/altivec min/max instructions conform to IEEE behavior then

> you could instead fold

> to .F{MIN,MAX} internal functions and define the f{min,max} optabs.

>

> Otherwise the patch looks correct to me - MIN_EXPR and MAX_EXPR are

> not IEEE conforming.

> Note a better check would be to use HONOR_NANS/HONOR_SIGNED_ZEROS on

> the argument type

> (that also works for the integer types with the obvious answer).

>

> Richard.

>

>> ChangeLog

>>

>> 2021-08-25 Haochen Gui <guihaoc@linux.ibm.com>

>>

>> gcc/

>>           * config/rs6000/rs6000-call.c (rs6000_gimple_fold_builtin):

>>           Modify the VSX_BUILTIN_XVMINDP, ALTIVEC_BUILTIN_VMINFP,

>>           VSX_BUILTIN_XVMAXDP, ALTIVEC_BUILTIN_VMAXFP expansions.

>>

>> gcc/testsuite/

>>           * gcc.target/powerpc/vec-minmax-1.c: New test.

>>           * gcc.target/powerpc/vec-minmax-2.c: Likewise.

>>

>>

>> patch.diff

>>

>> diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c

>> index b4e13af4dc6..90527734ceb 100644

>> --- a/gcc/config/rs6000/rs6000-call.c

>> +++ b/gcc/config/rs6000/rs6000-call.c

>> @@ -12159,6 +12159,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

>>          return true;

>>        /* flavors of vec_min.  */

>>        case VSX_BUILTIN_XVMINDP:

>> +    case ALTIVEC_BUILTIN_VMINFP:

>> +      if (!flag_finite_math_only || flag_signed_zeros)

>> +       return false;

>> +      /* Fall through to MIN_EXPR.  */

>> +      gcc_fallthrough ();

>>        case P8V_BUILTIN_VMINSD:

>>        case P8V_BUILTIN_VMINUD:

>>        case ALTIVEC_BUILTIN_VMINSB:

>> @@ -12167,7 +12172,6 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

>>        case ALTIVEC_BUILTIN_VMINUB:

>>        case ALTIVEC_BUILTIN_VMINUH:

>>        case ALTIVEC_BUILTIN_VMINUW:

>> -    case ALTIVEC_BUILTIN_VMINFP:

>>          arg0 = gimple_call_arg (stmt, 0);

>>          arg1 = gimple_call_arg (stmt, 1);

>>          lhs = gimple_call_lhs (stmt);

>> @@ -12177,6 +12181,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

>>          return true;

>>        /* flavors of vec_max.  */

>>        case VSX_BUILTIN_XVMAXDP:

>> +    case ALTIVEC_BUILTIN_VMAXFP:

>> +      if (!flag_finite_math_only || flag_signed_zeros)

>> +       return false;

>> +      /* Fall through to MAX_EXPR.  */

>> +      gcc_fallthrough ();

>>        case P8V_BUILTIN_VMAXSD:

>>        case P8V_BUILTIN_VMAXUD:

>>        case ALTIVEC_BUILTIN_VMAXSB:

>> @@ -12185,7 +12194,6 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

>>        case ALTIVEC_BUILTIN_VMAXUB:

>>        case ALTIVEC_BUILTIN_VMAXUH:

>>        case ALTIVEC_BUILTIN_VMAXUW:

>> -    case ALTIVEC_BUILTIN_VMAXFP:

>>          arg0 = gimple_call_arg (stmt, 0);

>>          arg1 = gimple_call_arg (stmt, 1);

>>          lhs = gimple_call_lhs (stmt);

>> diff --git a/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c b/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c

>> new file mode 100644

>> index 00000000000..547798fd65c

>> --- /dev/null

>> +++ b/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c

>> @@ -0,0 +1,53 @@

>> +/* { dg-do compile { target { powerpc*-*-* } } } */

>> +/* { dg-require-effective-target powerpc_p9vector_ok } */

>> +/* { dg-options "-O2 -mdejagnu-cpu=power9" } */

>> +/* { dg-final { scan-assembler-times {\mxvmaxdp\M} 1 } } */

>> +/* { dg-final { scan-assembler-times {\mxvmaxsp\M} 1 } } */

>> +/* { dg-final { scan-assembler-times {\mxvmindp\M} 1 } } */

>> +/* { dg-final { scan-assembler-times {\mxvminsp\M} 1 } } */

>> +

>> +/* This test verifies that float or double vec_min/max are bound to

>> +   xv[min|max][d|s]p instructions when fast-math is not set.  */

>> +

>> +

>> +#include <altivec.h>

>> +

>> +#ifdef _BIG_ENDIAN

>> +   const int PREF_D = 0;

>> +#else

>> +   const int PREF_D = 1;

>> +#endif

>> +

>> +double vmaxd (double a, double b)

>> +{

>> +  vector double va = vec_promote (a, PREF_D);

>> +  vector double vb = vec_promote (b, PREF_D);

>> +  return vec_extract (vec_max (va, vb), PREF_D);

>> +}

>> +

>> +double vmind (double a, double b)

>> +{

>> +  vector double va = vec_promote (a, PREF_D);

>> +  vector double vb = vec_promote (b, PREF_D);

>> +  return vec_extract (vec_min (va, vb), PREF_D);

>> +}

>> +

>> +#ifdef _BIG_ENDIAN

>> +   const int PREF_F = 0;

>> +#else

>> +   const int PREF_F = 3;

>> +#endif

>> +

>> +float vmaxf (float a, float b)

>> +{

>> +  vector float va = vec_promote (a, PREF_F);

>> +  vector float vb = vec_promote (b, PREF_F);

>> +  return vec_extract (vec_max (va, vb), PREF_F);

>> +}

>> +

>> +float vminf (float a, float b)

>> +{

>> +  vector float va = vec_promote (a, PREF_F);

>> +  vector float vb = vec_promote (b, PREF_F);

>> +  return vec_extract (vec_min (va, vb), PREF_F);

>> +}

>> diff --git a/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c b/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c

>> new file mode 100644

>> index 00000000000..4c6f4365830

>> --- /dev/null

>> +++ b/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c

>> @@ -0,0 +1,51 @@

>> +/* { dg-do compile { target { powerpc*-*-* } } } */

>> +/* { dg-require-effective-target powerpc_p9vector_ok } */

>> +/* { dg-options "-O2 -mdejagnu-cpu=power9 -ffast-math" } */

>> +/* { dg-final { scan-assembler-times {\mxsmaxcdp\M} 2 } } */

>> +/* { dg-final { scan-assembler-times {\mxsmincdp\M} 2 } } */

>> +

>> +/* This test verifies that float or double vec_min/max can be converted

>> +   to scalar comparison when fast-math is set.  */

>> +

>> +

>> +#include <altivec.h>

>> +

>> +#ifdef _BIG_ENDIAN

>> +   const int PREF_D = 0;

>> +#else

>> +   const int PREF_D = 1;

>> +#endif

>> +

>> +double vmaxd (double a, double b)

>> +{

>> +  vector double va = vec_promote (a, PREF_D);

>> +  vector double vb = vec_promote (b, PREF_D);

>> +  return vec_extract (vec_max (va, vb), PREF_D);

>> +}

>> +

>> +double vmind (double a, double b)

>> +{

>> +  vector double va = vec_promote (a, PREF_D);

>> +  vector double vb = vec_promote (b, PREF_D);

>> +  return vec_extract (vec_min (va, vb), PREF_D);

>> +}

>> +

>> +#ifdef _BIG_ENDIAN

>> +   const int PREF_F = 0;

>> +#else

>> +   const int PREF_F = 3;

>> +#endif

>> +

>> +float vmaxf (float a, float b)

>> +{

>> +  vector float va = vec_promote (a, PREF_F);

>> +  vector float vb = vec_promote (b, PREF_F);

>> +  return vec_extract (vec_max (va, vb), PREF_F);

>> +}

>> +

>> +float vminf (float a, float b)

>> +{

>> +  vector float va = vec_promote (a, PREF_F);

>> +  vector float vb = vec_promote (b, PREF_F);

>> +  return vec_extract (vec_min (va, vb), PREF_F);

>> +}

>>
Xionghu Luo via Gcc-patches Oct. 13, 2021, 8:29 a.m. | #3
On Wed, Oct 13, 2021 at 9:43 AM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote:
>

> Richard,

>

>    Thanks so much for your comments.

>

>    As far as I know, VSX/altivec min/max instructions don't conform with C-Sytle Min/Max Macro. The fold converts it to MIN/MAX_EXPR then it has a chance to be implemented by scalar min/max instructions which conform with C-Sytle Min/Max Macro. That's why I made this patch.

>

>    As to IEEE behavior, do you mean "Minimum and maximum operations" defined in IEEE-754 2019?  If so, I think VSX/altivec min/max instructions don't conform with it. It demands a quite NaN if either operand is a NaN while our instructions don't.

>

> IEEE-754 2019 maximum(x, y) is xif x>y, yif y>x, and a quiet NaN if either operand is a NaN, according to 6.2. For this operation, +0 compares greater than −0. Otherwise (i.e., when x=y and signs are the same) it is either xor y. Actions for xvmaxdp


Hmm, then I do not understand the reason for the patch - people using
the intrinsics cannot expect IEEE semantics then.
So you are concerned that people don't get the 1:1 machine instruction
but eventually the IEEE conforming MIN/MAX_EXPR?
But that can then still happen with -ffast-math so I wonder what's the point.

Richard.

> On 12/10/2021 下午 5:57, Richard Biener wrote:

> > On Tue, Oct 12, 2021 at 10:59 AM HAO CHEN GUI via Gcc-patches

> > <gcc-patches@gcc.gnu.org> wrote:

> >> Hi,

> >>

> >>      This patch disables gimple folding for float or double vec_min/max when fast-math is not set. It makes vec_min/max conform with the guide.

> >>

> >> Bootstrapped and tested on powerpc64le-linux with no regressions. Is this okay for trunk? Any recommendations? Thanks a lot.

> >>

> >>      I re-send the patch as previous one is messed up in email thread. Sorry for that.

> > If the VSX/altivec min/max instructions conform to IEEE behavior then

> > you could instead fold

> > to .F{MIN,MAX} internal functions and define the f{min,max} optabs.

> >

> > Otherwise the patch looks correct to me - MIN_EXPR and MAX_EXPR are

> > not IEEE conforming.

> > Note a better check would be to use HONOR_NANS/HONOR_SIGNED_ZEROS on

> > the argument type

> > (that also works for the integer types with the obvious answer).

> >

> > Richard.

> >

> >> ChangeLog

> >>

> >> 2021-08-25 Haochen Gui <guihaoc@linux.ibm.com>

> >>

> >> gcc/

> >>           * config/rs6000/rs6000-call.c (rs6000_gimple_fold_builtin):

> >>           Modify the VSX_BUILTIN_XVMINDP, ALTIVEC_BUILTIN_VMINFP,

> >>           VSX_BUILTIN_XVMAXDP, ALTIVEC_BUILTIN_VMAXFP expansions.

> >>

> >> gcc/testsuite/

> >>           * gcc.target/powerpc/vec-minmax-1.c: New test.

> >>           * gcc.target/powerpc/vec-minmax-2.c: Likewise.

> >>

> >>

> >> patch.diff

> >>

> >> diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c

> >> index b4e13af4dc6..90527734ceb 100644

> >> --- a/gcc/config/rs6000/rs6000-call.c

> >> +++ b/gcc/config/rs6000/rs6000-call.c

> >> @@ -12159,6 +12159,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

> >>          return true;

> >>        /* flavors of vec_min.  */

> >>        case VSX_BUILTIN_XVMINDP:

> >> +    case ALTIVEC_BUILTIN_VMINFP:

> >> +      if (!flag_finite_math_only || flag_signed_zeros)

> >> +       return false;

> >> +      /* Fall through to MIN_EXPR.  */

> >> +      gcc_fallthrough ();

> >>        case P8V_BUILTIN_VMINSD:

> >>        case P8V_BUILTIN_VMINUD:

> >>        case ALTIVEC_BUILTIN_VMINSB:

> >> @@ -12167,7 +12172,6 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

> >>        case ALTIVEC_BUILTIN_VMINUB:

> >>        case ALTIVEC_BUILTIN_VMINUH:

> >>        case ALTIVEC_BUILTIN_VMINUW:

> >> -    case ALTIVEC_BUILTIN_VMINFP:

> >>          arg0 = gimple_call_arg (stmt, 0);

> >>          arg1 = gimple_call_arg (stmt, 1);

> >>          lhs = gimple_call_lhs (stmt);

> >> @@ -12177,6 +12181,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

> >>          return true;

> >>        /* flavors of vec_max.  */

> >>        case VSX_BUILTIN_XVMAXDP:

> >> +    case ALTIVEC_BUILTIN_VMAXFP:

> >> +      if (!flag_finite_math_only || flag_signed_zeros)

> >> +       return false;

> >> +      /* Fall through to MAX_EXPR.  */

> >> +      gcc_fallthrough ();

> >>        case P8V_BUILTIN_VMAXSD:

> >>        case P8V_BUILTIN_VMAXUD:

> >>        case ALTIVEC_BUILTIN_VMAXSB:

> >> @@ -12185,7 +12194,6 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

> >>        case ALTIVEC_BUILTIN_VMAXUB:

> >>        case ALTIVEC_BUILTIN_VMAXUH:

> >>        case ALTIVEC_BUILTIN_VMAXUW:

> >> -    case ALTIVEC_BUILTIN_VMAXFP:

> >>          arg0 = gimple_call_arg (stmt, 0);

> >>          arg1 = gimple_call_arg (stmt, 1);

> >>          lhs = gimple_call_lhs (stmt);

> >> diff --git a/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c b/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c

> >> new file mode 100644

> >> index 00000000000..547798fd65c

> >> --- /dev/null

> >> +++ b/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c

> >> @@ -0,0 +1,53 @@

> >> +/* { dg-do compile { target { powerpc*-*-* } } } */

> >> +/* { dg-require-effective-target powerpc_p9vector_ok } */

> >> +/* { dg-options "-O2 -mdejagnu-cpu=power9" } */

> >> +/* { dg-final { scan-assembler-times {\mxvmaxdp\M} 1 } } */

> >> +/* { dg-final { scan-assembler-times {\mxvmaxsp\M} 1 } } */

> >> +/* { dg-final { scan-assembler-times {\mxvmindp\M} 1 } } */

> >> +/* { dg-final { scan-assembler-times {\mxvminsp\M} 1 } } */

> >> +

> >> +/* This test verifies that float or double vec_min/max are bound to

> >> +   xv[min|max][d|s]p instructions when fast-math is not set.  */

> >> +

> >> +

> >> +#include <altivec.h>

> >> +

> >> +#ifdef _BIG_ENDIAN

> >> +   const int PREF_D = 0;

> >> +#else

> >> +   const int PREF_D = 1;

> >> +#endif

> >> +

> >> +double vmaxd (double a, double b)

> >> +{

> >> +  vector double va = vec_promote (a, PREF_D);

> >> +  vector double vb = vec_promote (b, PREF_D);

> >> +  return vec_extract (vec_max (va, vb), PREF_D);

> >> +}

> >> +

> >> +double vmind (double a, double b)

> >> +{

> >> +  vector double va = vec_promote (a, PREF_D);

> >> +  vector double vb = vec_promote (b, PREF_D);

> >> +  return vec_extract (vec_min (va, vb), PREF_D);

> >> +}

> >> +

> >> +#ifdef _BIG_ENDIAN

> >> +   const int PREF_F = 0;

> >> +#else

> >> +   const int PREF_F = 3;

> >> +#endif

> >> +

> >> +float vmaxf (float a, float b)

> >> +{

> >> +  vector float va = vec_promote (a, PREF_F);

> >> +  vector float vb = vec_promote (b, PREF_F);

> >> +  return vec_extract (vec_max (va, vb), PREF_F);

> >> +}

> >> +

> >> +float vminf (float a, float b)

> >> +{

> >> +  vector float va = vec_promote (a, PREF_F);

> >> +  vector float vb = vec_promote (b, PREF_F);

> >> +  return vec_extract (vec_min (va, vb), PREF_F);

> >> +}

> >> diff --git a/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c b/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c

> >> new file mode 100644

> >> index 00000000000..4c6f4365830

> >> --- /dev/null

> >> +++ b/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c

> >> @@ -0,0 +1,51 @@

> >> +/* { dg-do compile { target { powerpc*-*-* } } } */

> >> +/* { dg-require-effective-target powerpc_p9vector_ok } */

> >> +/* { dg-options "-O2 -mdejagnu-cpu=power9 -ffast-math" } */

> >> +/* { dg-final { scan-assembler-times {\mxsmaxcdp\M} 2 } } */

> >> +/* { dg-final { scan-assembler-times {\mxsmincdp\M} 2 } } */

> >> +

> >> +/* This test verifies that float or double vec_min/max can be converted

> >> +   to scalar comparison when fast-math is set.  */

> >> +

> >> +

> >> +#include <altivec.h>

> >> +

> >> +#ifdef _BIG_ENDIAN

> >> +   const int PREF_D = 0;

> >> +#else

> >> +   const int PREF_D = 1;

> >> +#endif

> >> +

> >> +double vmaxd (double a, double b)

> >> +{

> >> +  vector double va = vec_promote (a, PREF_D);

> >> +  vector double vb = vec_promote (b, PREF_D);

> >> +  return vec_extract (vec_max (va, vb), PREF_D);

> >> +}

> >> +

> >> +double vmind (double a, double b)

> >> +{

> >> +  vector double va = vec_promote (a, PREF_D);

> >> +  vector double vb = vec_promote (b, PREF_D);

> >> +  return vec_extract (vec_min (va, vb), PREF_D);

> >> +}

> >> +

> >> +#ifdef _BIG_ENDIAN

> >> +   const int PREF_F = 0;

> >> +#else

> >> +   const int PREF_F = 3;

> >> +#endif

> >> +

> >> +float vmaxf (float a, float b)

> >> +{

> >> +  vector float va = vec_promote (a, PREF_F);

> >> +  vector float vb = vec_promote (b, PREF_F);

> >> +  return vec_extract (vec_max (va, vb), PREF_F);

> >> +}

> >> +

> >> +float vminf (float a, float b)

> >> +{

> >> +  vector float va = vec_promote (a, PREF_F);

> >> +  vector float vb = vec_promote (b, PREF_F);

> >> +  return vec_extract (vec_min (va, vb), PREF_F);

> >> +}

> >>
Xionghu Luo via Gcc-patches Oct. 13, 2021, 9:15 a.m. | #4
On 13/10/2021 下午 4:29, Richard Biener wrote:
> On Wed, Oct 13, 2021 at 9:43 AM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote:

>> Richard,

>>

>>     Thanks so much for your comments.

>>

>>     As far as I know, VSX/altivec min/max instructions don't conform with C-Sytle Min/Max Macro. The fold converts it to MIN/MAX_EXPR then it has a chance to be implemented by scalar min/max instructions which conform with C-Sytle Min/Max Macro. That's why I made this patch.

>>

>>     As to IEEE behavior, do you mean "Minimum and maximum operations" defined in IEEE-754 2019?  If so, I think VSX/altivec min/max instructions don't conform with it. It demands a quite NaN if either operand is a NaN while our instructions don't.

>>

>> IEEE-754 2019 maximum(x, y) is xif x>y, yif y>x, and a quiet NaN if either operand is a NaN, according to 6.2. For this operation, +0 compares greater than −0. Otherwise (i.e., when x=y and signs are the same) it is either xor y. Actions for xvmaxdp

> Hmm, then I do not understand the reason for the patch - people using

> the intrinsics cannot expect IEEE semantics then.

> So you are concerned that people don't get the 1:1 machine instruction

> but eventually the IEEE conforming MIN/MAX_EXPR?

> But that can then still happen with -ffast-math so I wonder what's the point.

>

> Richard.


The reason for the patch is to keep compatibility between different Power servers.  The old servers don't have C-style Min/Max instructions and all are implemented by VSX/altivec min/max instructions. So I just want to keep the compatibility. For fast-math, the C-style Min/Max should be acceptable, I think.

The IEEE standard changed. VSX/altivec min/max instructions conform with IEEE 754-2008 (the old standard), but not with IEEE 754-2019 (the last one).

As of 2019, the formerly required/minNum, maxNum, minNumMag, and maxNumMag/in IEEE 754-2008 are now deleted due to their non-associativity. Instead, two sets of new/minimum and maximum operations/are recommended.

754-2008

maxNum(x, y) is the canonicalized number yif x< y, xif y< x, the canonicalized number if one
operand is a number and the other a quiet NaN. Otherwise it is either xor y, canonicalized (this
means results might differ among implementations). When either xor yis a signalingNaN, then the
result is according to 6.2.

Thanks again.

Gui Haochen

>

>> On 12/10/2021 下午 5:57, Richard Biener wrote:

>>> On Tue, Oct 12, 2021 at 10:59 AM HAO CHEN GUI via Gcc-patches

>>> <gcc-patches@gcc.gnu.org> wrote:

>>>> Hi,

>>>>

>>>>       This patch disables gimple folding for float or double vec_min/max when fast-math is not set. It makes vec_min/max conform with the guide.

>>>>

>>>> Bootstrapped and tested on powerpc64le-linux with no regressions. Is this okay for trunk? Any recommendations? Thanks a lot.

>>>>

>>>>       I re-send the patch as previous one is messed up in email thread. Sorry for that.

>>> If the VSX/altivec min/max instructions conform to IEEE behavior then

>>> you could instead fold

>>> to .F{MIN,MAX} internal functions and define the f{min,max} optabs.

>>>

>>> Otherwise the patch looks correct to me - MIN_EXPR and MAX_EXPR are

>>> not IEEE conforming.

>>> Note a better check would be to use HONOR_NANS/HONOR_SIGNED_ZEROS on

>>> the argument type

>>> (that also works for the integer types with the obvious answer).

>>>

>>> Richard.

>>>

>>>> ChangeLog

>>>>

>>>> 2021-08-25 Haochen Gui <guihaoc@linux.ibm.com>

>>>>

>>>> gcc/

>>>>            * config/rs6000/rs6000-call.c (rs6000_gimple_fold_builtin):

>>>>            Modify the VSX_BUILTIN_XVMINDP, ALTIVEC_BUILTIN_VMINFP,

>>>>            VSX_BUILTIN_XVMAXDP, ALTIVEC_BUILTIN_VMAXFP expansions.

>>>>

>>>> gcc/testsuite/

>>>>            * gcc.target/powerpc/vec-minmax-1.c: New test.

>>>>            * gcc.target/powerpc/vec-minmax-2.c: Likewise.

>>>>

>>>>

>>>> patch.diff

>>>>

>>>> diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c

>>>> index b4e13af4dc6..90527734ceb 100644

>>>> --- a/gcc/config/rs6000/rs6000-call.c

>>>> +++ b/gcc/config/rs6000/rs6000-call.c

>>>> @@ -12159,6 +12159,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

>>>>           return true;

>>>>         /* flavors of vec_min.  */

>>>>         case VSX_BUILTIN_XVMINDP:

>>>> +    case ALTIVEC_BUILTIN_VMINFP:

>>>> +      if (!flag_finite_math_only || flag_signed_zeros)

>>>> +       return false;

>>>> +      /* Fall through to MIN_EXPR.  */

>>>> +      gcc_fallthrough ();

>>>>         case P8V_BUILTIN_VMINSD:

>>>>         case P8V_BUILTIN_VMINUD:

>>>>         case ALTIVEC_BUILTIN_VMINSB:

>>>> @@ -12167,7 +12172,6 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

>>>>         case ALTIVEC_BUILTIN_VMINUB:

>>>>         case ALTIVEC_BUILTIN_VMINUH:

>>>>         case ALTIVEC_BUILTIN_VMINUW:

>>>> -    case ALTIVEC_BUILTIN_VMINFP:

>>>>           arg0 = gimple_call_arg (stmt, 0);

>>>>           arg1 = gimple_call_arg (stmt, 1);

>>>>           lhs = gimple_call_lhs (stmt);

>>>> @@ -12177,6 +12181,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

>>>>           return true;

>>>>         /* flavors of vec_max.  */

>>>>         case VSX_BUILTIN_XVMAXDP:

>>>> +    case ALTIVEC_BUILTIN_VMAXFP:

>>>> +      if (!flag_finite_math_only || flag_signed_zeros)

>>>> +       return false;

>>>> +      /* Fall through to MAX_EXPR.  */

>>>> +      gcc_fallthrough ();

>>>>         case P8V_BUILTIN_VMAXSD:

>>>>         case P8V_BUILTIN_VMAXUD:

>>>>         case ALTIVEC_BUILTIN_VMAXSB:

>>>> @@ -12185,7 +12194,6 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)

>>>>         case ALTIVEC_BUILTIN_VMAXUB:

>>>>         case ALTIVEC_BUILTIN_VMAXUH:

>>>>         case ALTIVEC_BUILTIN_VMAXUW:

>>>> -    case ALTIVEC_BUILTIN_VMAXFP:

>>>>           arg0 = gimple_call_arg (stmt, 0);

>>>>           arg1 = gimple_call_arg (stmt, 1);

>>>>           lhs = gimple_call_lhs (stmt);

>>>> diff --git a/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c b/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c

>>>> new file mode 100644

>>>> index 00000000000..547798fd65c

>>>> --- /dev/null

>>>> +++ b/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c

>>>> @@ -0,0 +1,53 @@

>>>> +/* { dg-do compile { target { powerpc*-*-* } } } */

>>>> +/* { dg-require-effective-target powerpc_p9vector_ok } */

>>>> +/* { dg-options "-O2 -mdejagnu-cpu=power9" } */

>>>> +/* { dg-final { scan-assembler-times {\mxvmaxdp\M} 1 } } */

>>>> +/* { dg-final { scan-assembler-times {\mxvmaxsp\M} 1 } } */

>>>> +/* { dg-final { scan-assembler-times {\mxvmindp\M} 1 } } */

>>>> +/* { dg-final { scan-assembler-times {\mxvminsp\M} 1 } } */

>>>> +

>>>> +/* This test verifies that float or double vec_min/max are bound to

>>>> +   xv[min|max][d|s]p instructions when fast-math is not set.  */

>>>> +

>>>> +

>>>> +#include <altivec.h>

>>>> +

>>>> +#ifdef _BIG_ENDIAN

>>>> +   const int PREF_D = 0;

>>>> +#else

>>>> +   const int PREF_D = 1;

>>>> +#endif

>>>> +

>>>> +double vmaxd (double a, double b)

>>>> +{

>>>> +  vector double va = vec_promote (a, PREF_D);

>>>> +  vector double vb = vec_promote (b, PREF_D);

>>>> +  return vec_extract (vec_max (va, vb), PREF_D);

>>>> +}

>>>> +

>>>> +double vmind (double a, double b)

>>>> +{

>>>> +  vector double va = vec_promote (a, PREF_D);

>>>> +  vector double vb = vec_promote (b, PREF_D);

>>>> +  return vec_extract (vec_min (va, vb), PREF_D);

>>>> +}

>>>> +

>>>> +#ifdef _BIG_ENDIAN

>>>> +   const int PREF_F = 0;

>>>> +#else

>>>> +   const int PREF_F = 3;

>>>> +#endif

>>>> +

>>>> +float vmaxf (float a, float b)

>>>> +{

>>>> +  vector float va = vec_promote (a, PREF_F);

>>>> +  vector float vb = vec_promote (b, PREF_F);

>>>> +  return vec_extract (vec_max (va, vb), PREF_F);

>>>> +}

>>>> +

>>>> +float vminf (float a, float b)

>>>> +{

>>>> +  vector float va = vec_promote (a, PREF_F);

>>>> +  vector float vb = vec_promote (b, PREF_F);

>>>> +  return vec_extract (vec_min (va, vb), PREF_F);

>>>> +}

>>>> diff --git a/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c b/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c

>>>> new file mode 100644

>>>> index 00000000000..4c6f4365830

>>>> --- /dev/null

>>>> +++ b/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c

>>>> @@ -0,0 +1,51 @@

>>>> +/* { dg-do compile { target { powerpc*-*-* } } } */

>>>> +/* { dg-require-effective-target powerpc_p9vector_ok } */

>>>> +/* { dg-options "-O2 -mdejagnu-cpu=power9 -ffast-math" } */

>>>> +/* { dg-final { scan-assembler-times {\mxsmaxcdp\M} 2 } } */

>>>> +/* { dg-final { scan-assembler-times {\mxsmincdp\M} 2 } } */

>>>> +

>>>> +/* This test verifies that float or double vec_min/max can be converted

>>>> +   to scalar comparison when fast-math is set.  */

>>>> +

>>>> +

>>>> +#include <altivec.h>

>>>> +

>>>> +#ifdef _BIG_ENDIAN

>>>> +   const int PREF_D = 0;

>>>> +#else

>>>> +   const int PREF_D = 1;

>>>> +#endif

>>>> +

>>>> +double vmaxd (double a, double b)

>>>> +{

>>>> +  vector double va = vec_promote (a, PREF_D);

>>>> +  vector double vb = vec_promote (b, PREF_D);

>>>> +  return vec_extract (vec_max (va, vb), PREF_D);

>>>> +}

>>>> +

>>>> +double vmind (double a, double b)

>>>> +{

>>>> +  vector double va = vec_promote (a, PREF_D);

>>>> +  vector double vb = vec_promote (b, PREF_D);

>>>> +  return vec_extract (vec_min (va, vb), PREF_D);

>>>> +}

>>>> +

>>>> +#ifdef _BIG_ENDIAN

>>>> +   const int PREF_F = 0;

>>>> +#else

>>>> +   const int PREF_F = 3;

>>>> +#endif

>>>> +

>>>> +float vmaxf (float a, float b)

>>>> +{

>>>> +  vector float va = vec_promote (a, PREF_F);

>>>> +  vector float vb = vec_promote (b, PREF_F);

>>>> +  return vec_extract (vec_max (va, vb), PREF_F);

>>>> +}

>>>> +

>>>> +float vminf (float a, float b)

>>>> +{

>>>> +  vector float va = vec_promote (a, PREF_F);

>>>> +  vector float vb = vec_promote (b, PREF_F);

>>>> +  return vec_extract (vec_min (va, vb), PREF_F);

>>>> +}

>>>>
Segher Boessenkool Oct. 13, 2021, 6:19 p.m. | #5
On Wed, Oct 13, 2021 at 10:29:26AM +0200, Richard Biener wrote:
> On Wed, Oct 13, 2021 at 9:43 AM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote:

> >    As to IEEE behavior, do you mean "Minimum and maximum operations" defined in IEEE-754 2019?  If so, I think VSX/altivec min/max instructions don't conform with it. It demands a quite NaN if either operand is a NaN while our instructions don't.

> >

> > IEEE-754 2019 maximum(x, y) is xif x>y, yif y>x, and a quiet NaN if either operand is a NaN, according to 6.2. For this operation, +0 compares greater than −0. Otherwise (i.e., when x=y and signs are the same) it is either xor y. Actions for xvmaxdp

> 

> Hmm, then I do not understand the reason for the patch - people using

> the intrinsics cannot expect IEEE semantics then.

> So you are concerned that people don't get the 1:1 machine instruction

> but eventually the IEEE conforming MIN/MAX_EXPR?


I do not know about Gimple MIN_EXPR (it is not documented?), but the
RTL "smin" is meaningless in the presence of NaNs or signed zeros.  This
is documented (in rtl.texi):

"""
@findex smin
@findex smax
@cindex signed minimum
@cindex signed maximum
@item (smin:@var{m} @var{x} @var{y})
@itemx (smax:@var{m} @var{x} @var{y})
Represents the smaller (for @code{smin}) or larger (for @code{smax}) of
@var{x} and @var{y}, interpreted as signed values in mode @var{m}.
When used with floating point, if both operands are zeros, or if either
operand is @code{NaN}, then it is unspecified which of the two operands
is returned as the result.
"""

(not exactly meaningless, okay, but not usable for almost anything).

> But that can then still happen with -ffast-math so I wonder what's the point.


That :-)


Segher
Segher Boessenkool Oct. 13, 2021, 6:28 p.m. | #6
On Tue, Oct 12, 2021 at 04:57:43PM +0800, HAO CHEN GUI wrote:
> b/gcc/config/rs6000/rs6000-call.c

> index b4e13af4dc6..90527734ceb 100644

> --- a/gcc/config/rs6000/rs6000-call.c

> +++ b/gcc/config/rs6000/rs6000-call.c

> @@ -12159,6 +12159,11 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator 

> *gsi)

>        return true;

>      /* flavors of vec_min.  */

>      case VSX_BUILTIN_XVMINDP:

> +    case ALTIVEC_BUILTIN_VMINFP:

> +      if (!flag_finite_math_only || flag_signed_zeros)

> +       return false;

> +      /* Fall through to MIN_EXPR.  */

> +      gcc_fallthrough ();

>      case P8V_BUILTIN_VMINSD:

>      case P8V_BUILTIN_VMINUD:

>      case ALTIVEC_BUILTIN_VMINSB:


"Fall though to code for MIN_EXPR"?  It suggests it is a label, as
written now.  Or don't have this comment at all, maybe?

> +/* { dg-do compile { target { powerpc*-*-* } } } */


Leave out the target clause?  Testcases in gcc.target/powerpc/ are not
run when this is not satisfied anyway, testing it twice is just more
noise.


Segher
Joseph Myers Oct. 13, 2021, 10:04 p.m. | #7
On Wed, 13 Oct 2021, HAO CHEN GUI via Gcc-patches wrote:

>   As to IEEE behavior, do you mean "Minimum and maximum operations" defined in

> IEEE-754 2019?  If so, I think VSX/altivec min/max instructions don't conform

> with it. It demands a quite NaN if either operand is a NaN while our

> instructions don't.

> 

> IEEE-754 2019 maximum(x, y) is xif x>y, yif y>x, and a quiet NaN if either

> operand is a NaN, according to 6.2. For this operation, +0 compares greater

> than −0. Otherwise (i.e., when x=y and signs are the same) it is either xor

> y. Actions for xvmaxdp


We don't have any built-in functions (or I think other internal 
operations) for the IEEE 754-2019 operations (C2X function names fmaximum, 
fminimum, fmaximum_num, fminimum_num, plus per-type suffixes) either, 
though as I noted when adding those functions to glibc, having such 
built-in functions would make sense (specifically, so that RISC-V can 
expand calls to fmaximum_num and fminimum_num inline when building for F 
or D extension version 2.2 and later).  The built-in functions we have for 
fmax and fmin correspond to the IEEE 754-2008 operations (as implemented 
by the AArch64 fmaxnm / fminnm instructions, for example).

-- 
Joseph S. Myers
joseph@codesourcery.com

Patch

diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index b4e13af4dc6..90527734ceb 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -12159,6 +12159,11 @@  rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
        return true;
      /* flavors of vec_min.  */
      case VSX_BUILTIN_XVMINDP:
+    case ALTIVEC_BUILTIN_VMINFP:
+      if (!flag_finite_math_only || flag_signed_zeros)
+       return false;
+      /* Fall through to MIN_EXPR.  */
+      gcc_fallthrough ();
      case P8V_BUILTIN_VMINSD:
      case P8V_BUILTIN_VMINUD:
      case ALTIVEC_BUILTIN_VMINSB:
@@ -12167,7 +12172,6 @@  rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
      case ALTIVEC_BUILTIN_VMINUB:
      case ALTIVEC_BUILTIN_VMINUH:
      case ALTIVEC_BUILTIN_VMINUW:
-    case ALTIVEC_BUILTIN_VMINFP:
        arg0 = gimple_call_arg (stmt, 0);
        arg1 = gimple_call_arg (stmt, 1);
        lhs = gimple_call_lhs (stmt);
@@ -12177,6 +12181,11 @@  rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
        return true;
      /* flavors of vec_max.  */
      case VSX_BUILTIN_XVMAXDP:
+    case ALTIVEC_BUILTIN_VMAXFP:
+      if (!flag_finite_math_only || flag_signed_zeros)
+       return false;
+      /* Fall through to MAX_EXPR.  */
+      gcc_fallthrough ();
      case P8V_BUILTIN_VMAXSD:
      case P8V_BUILTIN_VMAXUD:
      case ALTIVEC_BUILTIN_VMAXSB:
@@ -12185,7 +12194,6 @@  rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
      case ALTIVEC_BUILTIN_VMAXUB:
      case ALTIVEC_BUILTIN_VMAXUH:
      case ALTIVEC_BUILTIN_VMAXUW:
-    case ALTIVEC_BUILTIN_VMAXFP:
        arg0 = gimple_call_arg (stmt, 0);
        arg1 = gimple_call_arg (stmt, 1);
        lhs = gimple_call_lhs (stmt);
diff --git a/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c b/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c
new file mode 100644
index 00000000000..547798fd65c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c
@@ -0,0 +1,53 @@ 
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-require-effective-target powerpc_p9vector_ok } */
+/* { dg-options "-O2 -mdejagnu-cpu=power9" } */
+/* { dg-final { scan-assembler-times {\mxvmaxdp\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mxvmaxsp\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mxvmindp\M} 1 } } */
+/* { dg-final { scan-assembler-times {\mxvminsp\M} 1 } } */
+
+/* This test verifies that float or double vec_min/max are bound to
+   xv[min|max][d|s]p instructions when fast-math is not set.  */
+
+
+#include <altivec.h>
+
+#ifdef _BIG_ENDIAN
+   const int PREF_D = 0;
+#else
+   const int PREF_D = 1;
+#endif
+
+double vmaxd (double a, double b)
+{
+  vector double va = vec_promote (a, PREF_D);
+  vector double vb = vec_promote (b, PREF_D);
+  return vec_extract (vec_max (va, vb), PREF_D);
+}
+
+double vmind (double a, double b)
+{
+  vector double va = vec_promote (a, PREF_D);
+  vector double vb = vec_promote (b, PREF_D);
+  return vec_extract (vec_min (va, vb), PREF_D);
+}
+
+#ifdef _BIG_ENDIAN
+   const int PREF_F = 0;
+#else
+   const int PREF_F = 3;
+#endif
+
+float vmaxf (float a, float b)
+{
+  vector float va = vec_promote (a, PREF_F);
+  vector float vb = vec_promote (b, PREF_F);
+  return vec_extract (vec_max (va, vb), PREF_F);
+}
+
+float vminf (float a, float b)
+{
+  vector float va = vec_promote (a, PREF_F);
+  vector float vb = vec_promote (b, PREF_F);
+  return vec_extract (vec_min (va, vb), PREF_F);
+}
diff --git a/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c b/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c
new file mode 100644
index 00000000000..4c6f4365830
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c
@@ -0,0 +1,51 @@ 
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-require-effective-target powerpc_p9vector_ok } */
+/* { dg-options "-O2 -mdejagnu-cpu=power9 -ffast-math" } */
+/* { dg-final { scan-assembler-times {\mxsmaxcdp\M} 2 } } */
+/* { dg-final { scan-assembler-times {\mxsmincdp\M} 2 } } */
+
+/* This test verifies that float or double vec_min/max can be converted
+   to scalar comparison when fast-math is set.  */
+
+
+#include <altivec.h>
+
+#ifdef _BIG_ENDIAN
+   const int PREF_D = 0;
+#else
+   const int PREF_D = 1;
+#endif
+
+double vmaxd (double a, double b)
+{
+  vector double va = vec_promote (a, PREF_D);
+  vector double vb = vec_promote (b, PREF_D);
+  return vec_extract (vec_max (va, vb), PREF_D);
+}
+
+double vmind (double a, double b)
+{
+  vector double va = vec_promote (a, PREF_D);
+  vector double vb = vec_promote (b, PREF_D);
+  return vec_extract (vec_min (va, vb), PREF_D);
+}
+
+#ifdef _BIG_ENDIAN
+   const int PREF_F = 0;
+#else
+   const int PREF_F = 3;
+#endif
+
+float vmaxf (float a, float b)
+{
+  vector float va = vec_promote (a, PREF_F);
+  vector float vb = vec_promote (b, PREF_F);
+  return vec_extract (vec_max (va, vb), PREF_F);
+}
+
+float vminf (float a, float b)
+{
+  vector float va = vec_promote (a, PREF_F);
+  vector float vb = vec_promote (b, PREF_F);
+  return vec_extract (vec_min (va, vb), PREF_F);
+}