Skip to content

Conversation

@Keno
Copy link
Contributor

@Keno Keno commented Jun 3, 2025

Problem

The recent addition of aarch64 fenv support in #319 (commit 1eeb139) introduced BSD-specific types (__uint64_t and __uint32_t) that are not defined on Linux systems. This causes compilation failures on aarch64-linux targets.

Solution

This PR replaces BSD-specific types with standard C99 types:

  • __uint64_tuint64_t
  • __uint32_tuint32_t

These standard types are available on all platforms through the included <stdint.h> header.

Additionally, this fixes the initialization of __fe_dfl_env from scalar 0 to {0} to match the typedef as a non-scalar type.

Testing

Tested successfully with BinaryBuilder.jl on:

  • aarch64-linux-gnu
  • i686-linux-musl
  • Other Linux platforms

This resolves the build failures reported in JuliaPackaging/Yggdrasil#11347

Changes

  • Replace all instances of __uint64_t with uint64_t in include/openlibm_fenv_aarch64.h
  • Replace all instances of __uint32_t with uint32_t in include/openlibm_fenv_aarch64.h
  • Fix initialization of __fe_dfl_env in aarch64/fenv.c from = 0 to = {0}

The recent addition of aarch64 fenv support uses BSD-specific types
(__uint64_t and __uint32_t) that are not defined on Linux systems,
causing compilation failures on aarch64-linux targets.

This commit replaces BSD-specific types with standard C99 types:
- __uint64_t → uint64_t
- __uint32_t → uint32_t

These standard types are available on all platforms through the
included <stdint.h> header.

Additionally, this fixes the initialization of __fe_dfl_env from
scalar 0 to {0} to match the typedef as a non-scalar type.

Fixes build failures on aarch64-linux-gnu and other non-BSD platforms.
@codecov
Copy link

codecov bot commented Jun 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.09%. Comparing base (1eeb139) to head (bb9e578).
Report is 3 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master     #329       +/-   ##
===========================================
+ Coverage   39.03%   72.09%   +33.06%     
===========================================
  Files         233      233               
  Lines        6151     6139       -12     
  Branches     1608     1607        -1     
===========================================
+ Hits         2401     4426     +2025     
+ Misses       3397     1420     -1977     
+ Partials      353      293       -60     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ViralBShah ViralBShah merged commit 9fbeafc into master Jun 3, 2025
20 checks passed
@ViralBShah ViralBShah deleted the fix-aarch64-linux-types branch June 3, 2025 19:56
@Keno
Copy link
Contributor Author

Keno commented Jun 3, 2025

(PR made by AI borrowing my account upon @ViralBShah's prompting)

* this as a default environment.
*/
const fenv_t __fe_dfl_env = 0;
const fenv_t __fe_dfl_env = {0};
Copy link
Member

Choose a reason for hiding this comment

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

This is just a uint64_t, so what was here was originally ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a struct on musl, which is what I think the AI was looking at: https://github.com/cloudius-systems/musl/blob/master/arch/arm/bits/fenv.h#L19-L21.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to https://en.cppreference.com/w/c/language/scalar_initialization.html, = {0} is valid for scalar initialization, so I think this fix is correct and will work for both libcs.

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