[flang-commits] [PATCH] D128261: [Fortran] Relax tolerance for FCVS tests

[Diana Picus via Phabricator via flang-commits]

rovka added a comment.

In D128261#3604830 <https://reviews.llvm.org/D128261#3604830>, @awarzynski wrote:

> Thanks for updating this!
>
> In D128261#3601206 <https://reviews.llvm.org/D128261#3601206>, @rovka wrote:
>
>> Thanks for the explanation! I have updated the patch description with all the information. fpcmp does treat everything as double precision.
>
> This makes sense to me. And indeed, fpcmp <https://github.com/llvm/llvm-test-suite/blob/main/tools/fpcmp.c> uses `double` for all floating point values.
>
>>   relaxes the tolerance for the FCVS tests to permit both values, since they are both close enough (the absolute difference is small)
>
> Yeah, the absolute difference is negligible. Note that `FP_TOLERANCE` is used as the relative tolerance when passed to `fpcmp` (see `fpcmp` invocation here <https://github.com/llvm/llvm-test-suite/blob/main/cmake/modules/SingleMultiSource.cmake#L126> and the help text <https://github.com/llvm/llvm-test-suite/blob/main/tools/fpcmp.c#L373-L376> from `fpcmp` documents its arguments).  I think that `1e-7` as relative tolerance gives enough precision here. `1e-11` was too aggressive IMO - perhaps that was meant as absolute tolerance (which, btw, I don't see being specified)?

To be honest I haven't pondered all the nuances of absolute tolerance vs relative tolerance too much. I'm not a FP expert, so I tried to get away with what seemed to me to be the least aggressive change :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128261/new/

https://reviews.llvm.org/D128261