Re: [PATCH][Fortran] Use MIN/MAX_EXPR for intrinsics or __builtin_fmin/max when appropriate

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Jul 17, 2018 at 2:35 PM Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
>
> Hi all,
>
> This is my first Fortran patch, so apologies if I'm missing something.
> The current expansion of the min and max intrinsics explicitly expands
> the comparisons between each argument to calculate the global min/max.
> Some targets, like aarch64, have instructions that can calculate the min/max
> of two real (floating-point) numbers with the proper NaN-handling semantics
> (if both inputs are NaN, return Nan. If one is NaN, return the other) and those
> are the semantics provided by the __builtin_fmin/max family of functions that expand
> to these instructions.
>
> This patch makes the frontend emit __builtin_fmin/max directly to compare each
> pair of numbers when the numbers are floating-point, and use MIN_EXPR/MAX_EXPR otherwise
> (integral types and -ffast-math) which should hopefully be easier to recognise in the

What is Fortrans requirement on min/max intrinsics?  Doesn't it only
require things that
are guaranteed by MIN/MAX_EXPR anyways?  The only restriction here is

/* Minimum and maximum values.  When used with floating point, if both
   operands are zeros, or if either operand is NaN, then it is unspecified
   which of the two operands is returned as the result.  */

which means MIN/MAX_EXPR are not strictly IEEE compliant with signed
zeros or NaNs.
Thus the correct test would be !HONOR_SIGNED_ZEROS && !HONOR_NANS if singed
zeros are significant.

I'm not sure if using fmin/max calls when we cannot use MIN/MAX_EXPR
is a good idea,
this may both generate bigger code and be slower.

Richard.

> midend and optimise. The previous approach of generating the open-coded version of that
> is used when we don't have an appropriate __builtin_fmin/max available.
> For example, for a configuration of x86_64-unknown-linux-gnu that I tested there was no
> 128-bit __built_fminl available.
>
> With this patch I'm seeing more than 7000 FMINNM/FMAXNM instructions being generated at -O3
> on aarch64 for 521.wrf from fprate SPEC2017 where none before were generated
> (we were generating explicit comparisons and NaN checks). This gave a 2.4% improvement
> in performance on a Cortex-A72.
>
> Bootstrapped and tested on aarch64-none-linux-gnu and x86_64-unknown-linux-gnu.
>
> Ok for trunk?
> Thanks,
> Kyrill
>
> 2018-07-17  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>      * f95-lang.c (gfc_init_builtin_functions): Define __builtin_fmin,
>      __builtin_fminf, __builtin_fminl, __builtin_fmax, __builtin_fmaxf,
>      __builtin_fmaxl.
>      * trans-intrinsic.c: Include builtins.h.
>      (gfc_conv_intrinsic_minmax): Emit __builtin_fmin/max or MIN/MAX_EXPR
>      functions to calculate the min/max.
>
> 2018-07-17  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>      * gfortran.dg/max_fmaxf.f90: New test.
>      * gfortran.dg/min_fminf.f90: Likewise.
>      * gfortran.dg/minmax_integer.f90: Likewise.
>      * gfortran.dg/max_fmaxl_aarch64.f90: Likewise.
>      * gfortran.dg/min_fminl_aarch64.f90: Likewise.