Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[aarch64] Fix fmin and fmax on aarch64 #140

Merged
merged 14 commits into from
Jan 16, 2018
Merged

Conversation

shibatch
Copy link
Owner

@shibatch shibatch commented Jan 5, 2018

vmaxnum_v*_v*_v* and vminnum_v*_v*_v* are helper functions for utilizing instructions for choosing larger and smaller elements from two given vectors, and they are introduced in the following PR.

#109

It is said that vmaxnmq and vminnmq on aarch64 are IEEE754-conformant, but I recently found that handling of signalling NaN by these instructions is not conforming to the specification of fmin and fmax functions in the ANSI C standard.

This patch fixes that problem.

@fpetrogalli
Copy link
Collaborator

Would it be possible to add a regression test? Also, do you mind linking some official information about this problem?
Moreover, we introduced the SLEEF_SINGLE_MINMAXNUM_AVAILABLEand SLEEF_DOUBLE_MINMAXNUM_AVAILABLE in #109. If we opt for the solution in this PR, I think we should remove those macros and all the changes related to that.

@shibatch
Copy link
Owner Author

shibatch commented Jan 7, 2018

Do you think it is better to test a signalling NaN input to all functions?

@fpetrogalli
Copy link
Collaborator

Do you think it is better to test a signalling NaN input to all functions?

No, I think it is enough to add a regression test for this particular PR.

@shibatch
Copy link
Owner Author

apt-get update at travis did not work anymore, so I added lines for updating expired keys and removing an apt source relating to mongo-db following the comments in travis-ci/travis-ci#8554 to .travis.yml.

@shibatch
Copy link
Owner Author

apt-get update problem seems to be fixed now.

travis-ci/travis-ci#9037

@shibatch
Copy link
Owner Author

As for the operation carried out by the aarch64 instructions, they are the minNum and maxNum operations defined in section 5.3 and 6.2.3 of IEEE Std 754™-2008.

http://ieeexplore.ieee.org/document/4610935/

The IEEE standard specifies that signaling NaNs should be propagated in these operations.

On the other hand, the C99 standard does not distinguish between a quiet NaN and a signaling NaN.

Since SLEEF is a substitute of the C99 math library, propagation of a signaling NaN is not desirable.

@shibatch shibatch merged commit b82fd94 into master Jan 16, 2018
@shibatch shibatch deleted the Fix_fmin_and_fmax_on_aarch64 branch January 16, 2018 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants