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

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Wed, Oct 20, 2021 at 05:04:56PM +0800, HAO CHEN GUI wrote:
> 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 refined the patch according to reviewers' advice. The attachments are 
> the ChangeLog and patch diff in case the email body is messed up.
> 
> 
> ChangeLog
> 
> 2021-10-20 Haochen Gui <guihaoc@linux.ibm.com>
> 
> gcc/
>         * config/rs6000/rs6000-call.c (rs6000_gimple_fold_builtin):
>         Disable gimple fold for VSX_BUILTIN_XVMINDP, 
> ALTIVEC_BUILTIN_VMINFP,
>         VSX_BUILTIN_XVMAXDP, ALTIVEC_BUILTIN_VMAXFP when fast-math 
> is not
>         set.

Content-Type: text/plain; charset=UTF-8; format=flowed

Please don't use flowed.  It makes patches unreadable and unusable if
you do (they will not apply anymore).

Also, the left border should be one tab, not eight spaces, and the right
border is at 80 chars (so there are 72 usable chars on a line).  Don't
end a line in ":" if you don't overflow a line if you put text after it.

> --- a/gcc/config/rs6000/rs6000-call.c
> +++ b/gcc/config/rs6000/rs6000-call.c
> @@ -12159,6 +12159,14 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator 
> *gsi)
>        return true;
>      /* flavors of vec_min.  */
>      case VSX_BUILTIN_XVMINDP:
> +    case ALTIVEC_BUILTIN_VMINFP:
> +      {
> +       lhs = gimple_call_lhs (stmt);
> +       tree type = TREE_TYPE (lhs);
> +       if (HONOR_NANS (type) || HONOR_SIGNED_ZEROS (type))
> +         return false;
> +       gcc_fallthrough ();
> +      }

Both vminfp anf xvmindp (and xvminsp and xsmindp) return -0 or the
minimum of +0 and -0, that is okay even with HONOR_SIGNED_ZEROS, I
think?

x[sv]min[sd]p returns the number for the minimum of a NaN and a number,
but vminfp returns a NaN.  Do you really want to make the xvmindp
builtin handle less than it does currently?  And, what about vminfp?
Did tht do the wrong thing before?

There are no tests for any of that apparently.  Hrm.


Segher