[PATCH][Fortran][v2] Use MIN/MAX_EXPR for min/max intrinsics

[Kyrill Tkachov]

Hi all,

Thank you for the feedback so far.
This version of the patch doesn't try to emit fmin/fmax function calls but instead
emits MIN/MAX_EXPR sequences unconditionally.
I think a source of confusion in the original proposal (for me at least) was
that on aarch64 (that I primarily work on) we implement the fmin/fmax optabs
and therefore these calls are expanded to a single instruction.
But on x86_64 these optabs are not implemented and therefore expand to actual library calls.
Therefore at -O3 (no -ffast-math) I saw a gain on aarch64. But I measured today
on x86_64 and saw a regression.

Thomas and Janne suggested that the Fortran standard does not impose a requirement
on NaN handling for the min/max intrinsics, which would make emitting MIN/MAX_EXPR
sequences unconditionally a valid approach.

However, the gfortran.dg/nan_1.f90 test checks that handling of NaN values in
these intrinsics follows the IEEE semantics (min (nan, 2.0) == 2.0, for example).
This is not required by the standard, but is the existing gfortran behaviour.

If we end up always emitting MIN/MAX_EXPR sequences, like this version of the patch does,
then that test fails on some configurations of x86_64 and not others (for me it FAILs
by default, but passes with -march=native on my machine) and passes on AArch64.
This is expected since MIN/MAX_EXPR doesn't enforce IEEE behaviour on its arguments.

However, by always emitting MIN/MAX_EXPR the gfc_conv_intrinsic_minmax function is
simplified and, perhaps more importantly, generates faster code in the -O3 case.
With this patch I see performance improvement on 521.wrf on both AArch64 (3.7%)
and x86_64 (5.4%).

Thomas, Janne, would this relaxation of NaN handling be acceptable given the benefits
mentioned above? If so, what would be the recommended adjustment to the nan_1.f90 test?

Thanks,
Kyrill

2018-07-18  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * trans-intrinsic.c: (gfc_conv_intrinsic_minmax): Emit MIN_MAX_EXPR
     sequence to calculate the min/max.

2018-07-18  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * gfortran.dg/max_float.f90: New test.
     * gfortran.dg/min_float.f90: Likewise.
     * gfortran.dg/minmax_integer.f90: Likewise.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fort-v2.patch
Type: text/x-patch
Size: 5823 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/fortran/attachments/20180718/0843f369/attachment.bin>