On Wed, Oct 13, 2021 at 9:43 AM HAO CHEN GUI <guih...@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 <guih...@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);
> >> +}
> >>
