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

fix musl float arithmetic and move it into sub-library #1177

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

larskuhtz
Copy link
Contributor

On some systems the c implementation of transcendental float arithmetic could result in wrong results due to conflicts with symbols in the system libm.

This PR

  • simplifies the implementation of transcendental float arithmetic by removing redundant code and making all symbols that aren't used directly by Haskell FFI static, and
  • moves the implementation into a sub-library in order to make debugging and testing easier.

static __inline uint16_t __bswap16(uint16_t __x)
{
return __x<<8 | __x>>8;
}

static __inline uint32_t __bswap32(uint32_t __x)
{
return __x>>24 | __x>>8&0xff00 | __x<<8&0xff0000 | __x<<24;
return __x>>24 | (__x>>8&0xff00) | (__x<<8&0xff0000) | __x<<24;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes a compiler warning

}

static __inline uint64_t __bswap64(uint64_t __x)
{
return __bswap32(__x)+0ULL<<32 | __bswap32(__x>>32);
return (__bswap32(__x)+0ULL)<<32 | __bswap32(__x>>32);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes a compiler warning

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Collects exp_data.h, exp_data.c, and exp.c into a single translation unit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Collects log_data.h, log_data.c, and log.c into a single translation unit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Collects pow_data.h, pow_data.c, exp_data.h, exp_data.c, and pow.c into a single translation unit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Collects sqrt_data.c and sort.c into a single translation unit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

libm.h with just what is needed to compile pow.c, sqrt.c, log.c, and exp.c. Also all symbols are prefixed with kadena

a double. (int32_t)KI is the k used in the argument reduction and exponent
adjustment of scale, positive k here means the result may overflow and
negative k means the result may underflow. */
static inline double kadena_specialcase(double_t tmp, uint64_t sbits, uint64_t ki)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
static inline double kadena_specialcase(double_t tmp, uint64_t sbits, uint64_t ki)
static inline double specialcase(double_t tmp, uint64_t sbits, uint64_t ki)

/* Worst case error is less than 0.5+1.11/N+(abs poly error * 2^53) ulp. */
tmp = tail + r + r2 * (C2 + r * C3) + r2 * r2 * (C4 + r * C5);
if (kadena_predict_false(abstop == 0))
return kadena_specialcase(tmp, sbits, ki);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
return kadena_specialcase(tmp, sbits, ki);
return specialcase(tmp, sbits, ki);

@imalsogreg
Copy link
Contributor

Is this PR obsoleted by #1184 ?

@jwiegley
Copy link
Contributor

Is this PR obsoleted by #1184 ?

I would call this PR an improvement upon it. Except that now, this PR is only doing a pure code reorg, and no longer attempting to fix an issue.

@jmcardon jmcardon self-requested a review as a code owner May 21, 2024 15:19
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.

3 participants