-
Notifications
You must be signed in to change notification settings - Fork 210
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
Allow optimization and use fesetround(), fegetround() #642
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebase the latest master
branch.
f30c404
to
746be68
Compare
6e3fd9a
to
0285a2b
Compare
c917fce
to
2b131e7
Compare
case FE_TOWARDZERO: | ||
return _MM_ROUND_TOWARD_ZERO; | ||
default: // FIXME | ||
return _MM_ROUND_TOWARD_ZERO; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jserv I am not sure whether we have a better value to return in the error case, so I temporarily return _MM_ROUND_TOWARD_ZERO
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure whether we have a better value to return in the error case, so I temporarily return
_MM_ROUND_TOWARD_ZERO
here.
It is reasonable, and you should explain more in comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added comments for explaining the error case
rounding = FE_TOWARDZERO; | ||
break; | ||
default: // FIXME | ||
rounding = FE_TOWARDZERO; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
I would like to ask @jmather-sesi and @alexorlov124 for reviewing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the label refactor
in the git commit messages. The changes were not associated with refactoring. Instead, the commits did change the behavior with compiler optimizations.
Revert the previous restriction that some of the functions are forced to not be optimized when compiling-time optimization options were given. Now it is users' responsibility to ensure the behavior after optimization. Shifting the responsibility to the users enables sse2neon the run in the optimized state in general, but not restricted by some specific scenarios.
I changed to |
df98efc
to
398344f
Compare
Setting/getting rounding mode directly through fpcr with volatile keyword could be unstable. Therefore we use the C99 fesetround()/fegetround() here to ensure the behavior.
Thank @howjmay for contributing! |
thanks for reviewing |
feat: Allow optimization option in sse2neon.h
Revert the previous restriction that some of the functions are forced
to not be optimized when compiling-time optimization options were
given.
Now it is users' responsibility to ensure the behavior after
optimization.
Shifting the responsibility to the users enables sse2neon the run in
the optimized state in general, but not restricted by some specific
scenarios.
feat: Use fesetround() and fegetround()
Setting/getting rounding mode directly through fpcr with volatile
keyword could be unstable.
Therefore we use the C99 fesetround()/fegetround() here to ensure
the behavior.
closes #648