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

[Janne Blomqvist <blomqvist.janne@gmail.com>]

On Wed, Jul 18, 2018 at 5:03 PM, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com
> wrote:

>
> On 18/07/18 14:26, Thomas König wrote:
>
>> Hi Kyrlll,
>>
>> Am 18.07.2018 um 13:17 schrieb Kyrill Tkachov <
>>> kyrylo.tkachov@foss.arm.com>:
>>>
>>> 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?
>>>
>> I would be a bit careful about changing behavior in such a major way.
>> What would the results with NaN and infinity then be, with or without
>> optimization? Would the results be consistent with min(nan,num) vs
>> min(num,nan)? Would they be consistent with the new IEEE standard?
>>
>> In general, I think that min(nan,num) should be nan and that our current
>> behavior is not the best.
>>
>> Does anybody have dats points on how this is handled by other compilers?
>>
>> Oh, and if anything is changed, then compile and runtime behavior should
>> always be the same.
>>
>
> Thanks, that makes it clearer what behaviour is accceptable.
>
> So this v3 patch follows Richard Sandiford's suggested approach of
> emitting IFN_FMIN/FMAX
> when dealing with floating-point values and NaN handling is important and
> the target
> supports the IFN_FMIN/FMAX. Otherwise the current explicit comparison
> sequence is emitted.
> For integer types and -ffast-math floating-point it will emit MIN/MAX_EXPR.
>
> With this patch the nan_1.f90 behaviour is preserved on all targets, we
> get the optimal
> sequence on aarch64 and on x86_64 we avoid the function call, with no
> changes in code generation.
>
> This gives the performance improvement on 521.wrf on aarch64 and leaves it
> unchanged on x86_64.
>
> I'm hoping this addresses all the concerns raised in this thread:
> * The NaN-handling behaviour is unchanged on all platforms.
> * The fast inline sequence is emitted where it is available.
> * No calls to library fmin*/fmax* are emitted where there were none.
> * MIN/MAX_EXPR sequence are emitted where possible.
>
> Is this acceptable?
>

So if I understand it correctly, the "internal fn" thing is a mechanism
that allows to check whether the target supports expanding a builtin inline
or whether it requires a call to an external library function?

If so, then yes, Ok, thanks for the patch!


-- 
Janne Blomqvist