Hi Hao Chen,
On 8/24/21 3:52 AM, HAO CHEN GUI wrote:
Hi
The patch disables gimple fold for float or double vec_min/max
builtin when fast-math is not set. Two test cases are added to verify
the patch.
The attachments are the patch diff and change log file.
Bootstrapped and tested on powerpc64le-linux with no regressions. Is
this okay for trunk? Any recommendations? Thanks a lot.
Thanks for this patch! In the future, if you can put your ChangeLog and
patch inline in your post, it makes it easier to review. (Otherwise we
have to manually copy it into our response and manipulate it to look
quoted, etc.)
Your ChangeLog isn't formatted correctly. It should look like this:
2021-08-24 Hao Chen 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, and
ALTIVEC_BUILTIN_VMAXFP expansions.
gcc/testsuite/
* gcc.target/powerpc/vec-minmax-1.c: New test.
* gcc.target/powerpc/vec-minmax-2.c: Likewise.
You forgot the committer/timestamp line and the ChangeLog location
lines. (The headers like "gcc/" ensure that the automated processing
will record your entries in the ChangeLog at the correct location in the
source tree.) Note also that the colon ":" always follows the ending
parenthesis when there's a function name listed. Please review
https://gcc.gnu.org/codingconventions.html#ChangeLogs.
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..9782d1b9308 --- /dev/null +++
b/gcc/testsuite/gcc.target/powerpc/vec-minmax-1.c @@ -0,0 +1,51 @@ +/*
{ dg-do compile { target { powerpc64le-*-* } } } */ +/* {
dg-require-effective-target powerpc_p9vector_ok } */ +/* { dg-options
"-O2 -mdejagnu-cpu=power9" } */ +/* { dg-final { scan-assembler-times
{\mxvmax[ds]p\M} 2 } } */ +/* { dg-final { scan-assembler-times
{\mxvmin[ds]p\M} 2 } } */
This is pedantic, but... You want exactly one each of xvmaxdp, xvmaxsp,
xvmindp, and xvminsp,
so please replace this with four lines with { scan-assembler-times {...} 1 }.
Thanks. :-)
Otherwise this looks fine to me. I can't approve, but recommend the
maintainers approve with
that changed.
Thanks!
Bill
+ +/* 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..d318b933181 --- /dev/null +++
b/gcc/testsuite/gcc.target/powerpc/vec-minmax-2.c @@ -0,0 +1,51 @@ +/*
{ dg-do compile { target { powerpc64le-*-* } } } */ +/* {
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); +}
