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

xbox: Implement nexttoward() #32

Merged
merged 1 commit into from
Sep 3, 2020
Merged

xbox: Implement nexttoward() #32

merged 1 commit into from
Sep 3, 2020

Conversation

thrimbor
Copy link
Member

I saw that @Ryzee119's Peanut-GB port works around our lack of nexttowardf, so I decided to quickly add an implementation. The code is taken from a public domain implementation by Danny Smith and slightly adapted (mostly indentation and curly braces changes).
Even though the code comes from Cygwin and I expect it to be tested, I ran a few quick tests against glibc, using positive and negative numbers as well as combinations with NAN and INFINITY, and the results matched.

@@ -1,20 +1,108 @@
#include <math.h>
#include <assert.h>

// Adapted from public domain code by Danny Smith <[email protected]>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'd put this comment in the first line, so it's clear that this affects the entire file, and not just this function (even though the blank line hints at this).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}

long double nexttowardl(long double x, long double y)
{
assert(0); // Not implemented
return 0.0;
static_assert(sizeof(double) == sizeof(long double), "This function assumes that long double and double are the same size.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I probably prefer a shorter message like "long double and double must be the same size".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, I managed to miss your comment earlier. It's changed now. I also made nexttowardl simply call nexttoward, it just was the same code anyway, so there's no reason to duplicate it.

@dracc
Copy link

dracc commented Jul 18, 2020

Code looks good, builds without warnings. I haven't tested the functionality myself but I trust cygwin.

@Ryzee119
Copy link

Thanks @thrimbor

I removed that work-around in Peanut-GB and was successfully able to compile and run with this change

@dracc
Copy link

dracc commented Aug 30, 2020

I've since tested with Peanut-gb too. Works as expected.👍

@thrimbor thrimbor merged commit 35f8195 into XboxDev:nxdk Sep 3, 2020
@thrimbor thrimbor deleted the float_nexttoward branch September 3, 2020 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants