Skip to content

nolibc: format printf floats as double, not long double - #177

Merged
hannesm merged 1 commit into
mirage:mainfrom
samoht:nolibc-printf-double
Jul 22, 2026
Merged

nolibc: format printf floats as double, not long double#177
hannesm merged 1 commit into
mirage:mainfrom
samoht:nolibc-printf-double

Conversation

@samoht

@samoht samoht commented Jul 6, 2026

Copy link
Copy Markdown
Member

Split out of #171 for focused review, per @dinosaure.

nolibc's printf (vfprintf.c) reads float arguments as long double, but its fmt_fp already takes a double, so the value round-trips double -> long double -> double through __extenddftf2/__trunctfdf2. On aarch64-linux-gnu (the freestanding cross target) long double is IEEE binary128 (quad), so those are soft-float routines; the round-trip pulls them into every freestanding link that uses printf, even for integer-only formats.

This makes the union field double. %f/%g/%e are unaffected (already formatted at double precision, since fmt_fp takes double); only %Lf/%Le/%Lg change, from quad to double, and OCaml never emits those. Freestanding aarch64 unikernels then link with no compiler runtime library, which is what the LLVM-only macOS cross toolchain in #171 needs (it has no libgcc).

@shym shym left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice catch of (yet another) instance of implicit casts between number types being harmful! Looks good to me.

(I wondered whether the removal of EXTRALIBS in #171 would belong in that PR also? Or is that related to some other change?)

@hannesm

hannesm commented Jul 9, 2026

Copy link
Copy Markdown
Member

please rebase for improved CI experience (#178)

hannesm pushed a commit that referenced this pull request Jul 9, 2026
9fbeafc Fix BSD-specific types on Linux for aarch64 (#329)
d463259 ci: test with openlibm-test (#327)
3cfad14 Update index.html
1eeb139 aarch64/fenv: fix `fenv_t` (#319)
5a88ede Add Codecov token to CI (#318)
5a33b6b Fix a bug in remquo{,f,l}, in which the quotient didn't always have the correct sign when the remainder was 0. (#317)
798e808 ci: Add cross-loongarch64.yml (#313)
db24332 Merge pull request #314 from JuliaMath/aa/clang-aarch64-fixes
49a3204 Bump codecov/codecov-action from 4 to 5 (#310)
4d2d83f Add a Vim filetype hint for Make.inc alongside that for Emacs
42c2687 Fix setting of `USECLANG`/`USEGCC`
ca166a0 Fix detection of whether `long double` is the same as `double`
296a5e4 ci: Fix clang build and Update qemu (#312)
c4667ca Fix fenv_t initializer (#308)
5992e5a Fix risc-v build (#305)
e2482c9 Add stack markings for GNU to fmod assembly files (#307)
d66f477 Merge pull request #299 from theos-sh/master
f644978 Merge pull request #304 from ViliusSutkus89/long_double_not_double
c1e8462 Regex match Make.inc to match CMake version with Autotools version
3335ebc Generate pkg-config.pc file in CMake
7df0e2f Compile time check if LONG DOUBLE is different from DOUBLE
a245816 Add support for Android x86 (i686)
f051cd4 Add support for armv7-a
5fa83b1 Use upcoming version number in CMakeLists
a1fdd7c Merge branch 'master' into master
f10ba45 Delete appveyor.yml
79d3a07 Delete .travis.yml
87e5f0b CMake updates (#302)
d9f2b0f Minor fix-ups for Darwin PowerPC (#303)
b3a6e99 build: use clang instead of llvm-gcc (#300)
fcded85 build: allow riscv64
46c3676 build: allow riscv64
21d22c8 build: apply cmake fixes
b42f035 Merge pull request #297 from JuliaMath/dependabot/github_actions/codecov/codecov-action-4
3b6700b Bump codecov/codecov-action from 3 to 4
4b83beb CI: Setup cross build test (#296)
fc36466 test: add target to gen coverage report (#295)
d3be5df Bump actions/checkout from 2 to 4 (#294)
bc36005 Create dependabot.yml
1e3511e Delete .github/FUNDING.yml
ae59af7 Update README.md
f3eec73 Correct long double check in CMakeLists.txt and remove duplicate definitions for static library builds too (#293)
b31c645 Add support for building shared library with CMake (#291)
bd591aa Readme: add new arch: `loongarch64` (#287)
eb5330e Support for loongarch64 architecture (#280)
acf58d2 build: fix pkgconfig cflags and libdir variables (#284)
a010123 Add CMake support (#286)
12f5ffc Fix Apple PowerPC build (#276)
1d2c5e3 Restore original fdlibm copyright on sincos files (#265)
0edf8d6 Remove non-working www subdomain from OpenLibm (#261)
ae2d916 Correctly round double precision sqrt (#256)
81d5e16 Add fmod assembly version (#255)
465ca0a Update README.md
428e7af Support for riscv64 architecture (#254)
ed7aea3 Bump version to 0.8 (#248)
69bb280 Another Windows ARM64 fix (#253)
3d4a902 Fixes for Windows ARM64 (#251)
a9568fb [Windows] install import library to libdir (#249)
f88e52a CI (Windows): set `msys2 {0}` as the default shell for all Windows steps (#247)
b48a2f7 CI (Linux and macOS): Remove the `arch` variable, which currently has no effect (#246)
2a47fa5 CI: A variety of miscellaneous tweaks (#244)
d0ef09a prefix symbols with _ for 32-bit x86 Windows (#242)
60dec83 msys2 ci (#243)
6ea5fa2 Merge pull request #240 from JuliaMath/vs/msys
437c139 Update ci.yml
e993267 Update ci.yml
4a36c50 Update ci.yml
24cec17 Update ci.yml
d26ed98 Update ci.yml
7b96025 Update ci.yml
7af65db Update ci.yml
a2e053e Revert "Update ci.yml"
4a52bb0 Update ci.yml
fb10fcf Update ci.yml
abf5aaa Update ci.yml
98dcc48 Update ci.yml
ff822f3 Update ci.yml
ab8d1ad Update ci.yml
4d97e2d Update ci.yml
72caeab Update ci.yml
9dd3049 Create ci.yml
15119bc Merge pull request #239 from JuliaMath/revert-238-patch-1
4bca0f2 Revert "prefix symbols with _ for 32-bit x86 Windows"
3b9454f Merge pull request #238 from jeremyd2019/patch-1
6ae6318 Update src/cdefs-compat.h
71a8fd1 Merge pull request #233 from lephe/more-long-double-aliases
7a3ef59 prefix symbols with _ for 32-bit x86 Windows
a871457 Merge pull request #230 from PetteriAimonen/master
a090d3e Fix powf: JuliaMath/openlibm#212 (comment)
57dd0f9 add missing weak references for long double functions
327b1bd Replace remaining __strong_alias uses
f052f42 Merge pull request #228 from JuliaMath/aa/hypotl
711654e Fix incorrect results in `hypotl` near underflow
aeab19f Fix for #211 Co-authored by: @kargl
5449705 Merge pull request #227 from JuliaMath/vs/powf
98f8713 Fix #211 Patched by importing latest msun version
6a85b33 Merge pull request #225 from JuliaMath/vs/strict_assign
40dac9d Restore STRICT_ASSIGN on FreeBSD as suggested in #215
2d10c90 Merge pull request #218 from jcestibariz/fix-wasm32
5d70ac5 Merge pull request #221 from maleadt/tb/static_fenv
63aa875 Make fenv methods static on additional platforms.
9152b0d Fix compilation errors on wasm32
3cb8045 Merge pull request #217 from epsilon-0/master
c856101 Merge pull request #219 from maleadt/tb/dont_export_fenv
be31bff Revert "Export `fenv` functions on all platforms (#213)"
eb21e8a don't alter toolchain vars if already provided
b34f107 Fix Apple Silicon build (#214)
5a27b4c Export `fenv` functions on all platforms (#213)
878948d Update list of libm libraries
508603d Update index.html
0276147 Merge pull request #209 from embeddedartistry/master
f2a8b36 Update download links to point to releases
1d6befd Merge pull request #208 from cndesantana/patch-1
4f559d4 Replace a few remaining __strong_reference uses (#210)
0418324 Refactor: OLM_DLLEXPORT definition now lives in a standalone header.
861b2ad Fix small typo
5b0e7e9 Update FUNDING.yml
382b8e9 Add musl-libc math library to resources.
f6ad75a update openlibm website.
5efed30 Bump SONAME as discussed in #200
e6ac7d7 Update README with new OS and arch support
f731481 Merge pull request #199 from llucinat/wasm32-weakref
f952e16 Fix weak reference macro redefinition in wasm32 target
97de1a4 Merge pull request #198 from gufe44/netbsd-fix-openlibm_weak_reference
c4dca1e Add files via upload
d4077aa Suggestions
ea065f9 Update src/cdefs-compat.h
2080b23 NetBSD fix
14bf902 Merge pull request #195 from ode33/patch-1
3bb2215 Update README.md
33c8313 Create FUNDING.yml
72f33a3 wasm32 support (#192)
f24b1bf Fix compilation of gcc when using openlibm as system libm (#190)
0f22aeb Create CNAME
c68e7d2 Delete CNAME
b524581 Rename doc -> docs
ebbba43 Move website to doc/ on master instead of gh-pages branch
4e3d709 update ULPs for s390 (#187)
65d7406 Merge pull request #185 from sharkcz/s390x
2a1e568 s390(x) port
cca41bc Merge branch 'master' of github.com:JuliaMath/openlibm
74b54c7 Add MIPS
ce69bf1 Update references to JuliaLang to point to JuliaMath (#182)
a96f074 Merge pull request #130 from ginggs/enable-optimization-again
c782ca2 Merge pull request #177 from JuliaMath/aa/windows
52df60b Update appveyor.yml
ce33de1 Add Windows testing with AppVeyor
4971b56 Update Make.inc
ca996ac Merge pull request #180 from JuliaLang/ginggs-0.5.6
73b3d88 Merge pull request #181 from CDLuminate/mipsport
3aa5c3b Merge pull request #174 from iniserve/master
4b4b41c Merge pull request #178 from JuliaLang/aa/upstream
a4b3fde travis: Add mips, mipsel, mips64el build.
ad9673e Makefile: clean mips/*.o
4dcc76e Using cdefs-compat.h and stdint.h instead  <sys/types.h> fenv-softfloat.h file added SOFTFLOAT code parts are not tested.
4f5112e Support for mips architectures
a24a5eb Enable optimization again for *int.c and *intf.c
a40570b Bump version to 0.5.6
787652b msun: signed overflow in atan2
8d91ecb Add TOOLPREFIX

git-subtree-dir: openlibm
git-subtree-split: 9fbeafcd4f1b6ef6aa3946c1c8faead50f38a94d
Comment thread gen_toolchain_tool.sh
Comment thread nolibc/vfprintf.c Outdated
@samoht
samoht force-pushed the nolibc-printf-double branch from 5494f17 to b6b12f6 Compare July 9, 2026 23:22
On targets where long double is IEEE quad (aarch64), vfprintf's `union arg`
held the float value as long double, so `%f` (and the fmt_fp call) round-tripped
double -> long double -> double through __extenddftf2/__trunctfdf2. That pulls
libgcc (or compiler-rt) soft-float into every freestanding link that uses printf,
even for integer-only formats. fmt_fp already takes a double, so the long double
was pure overhead.

Use double for the field; %L float conversions are treated as double (never used
by OCaml unikernels). aarch64 cross unikernels now link with no libgcc at all
(clang already inlines the atomics).
@samoht
samoht force-pushed the nolibc-printf-double branch from b6b12f6 to c495d1f Compare July 11, 2026 06:04
@samoht

samoht commented Jul 11, 2026

Copy link
Copy Markdown
Member Author

Rebased and just kept the long double -> double commit as per @dinosaure request.

@hannesm

hannesm commented Jul 14, 2026

Copy link
Copy Markdown
Member

I've not much knowledge about the C code, I guess it is taken from some other library.

What I wonder about is your comment:

only %Lf/%Le/%Lg change, from quad to double, and OCaml never emits those

So, why then not remove the support for these entirely? And have a comment in the source file that support has been removed since OCaml doesn't use them?
Introducing changes to vendored code without any comment makes updating (if there's need for it) cumbersome. At least from my perspective, MirageOS (and thus ocaml-solo5) goal is to be minimal -- so removing any unnecessary code should be the top priority.

Maybe noteworthy that 86493fb as well modified fmt_fp to avoid faults on Xen (by changing long double to double).

Has there been precision lost? I don't know. I usually avoid floating point numbers.

@shym

shym commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

I've not much knowledge about the C code, I guess it is taken from some other library.

It comes from musl (thanks to the LICENSE file!). And it has indeed a few differences with the one in the latest musl release.

Maybe noteworthy that 86493fb as well modified fmt_fp to avoid faults on Xen (by changing long double to double).

Interesting! I see two (non-exclusive) possibilities:

  • we could go with that proposed patch, as it doesn’t change the current behaviour,
  • try to get back on musl’s printf (which implies to test whether the Xen bug is still present and find another way to build it on macOS).

At least from my perspective, MirageOS (and thus ocaml-solo5) goal is to be minimal -- so removing any unnecessary code should be the top priority.

This made me wonder whether there would be a better-fitted printf implementation (especially after noticing that Solo5 bindings also vendor a different printf implementation, where I didn’t find support for L* at a quick glance; but I’m not sure it deserves the same trust as musl, I’d say)… But that’s getting me well beyond the scope of that PR.

@hannesm

hannesm commented Jul 17, 2026

Copy link
Copy Markdown
Member

@shym this is a good question, as far as I can tell:

In solo5, the floating point printing is disabled (see additionally the commit c298e69a8cedb4867252bbb1ae73d5eb4eb51af4).

Size-wise and support-wise, someone should measure whether the mpaland/printf is sufficient (and smaller?) or the musl printf is needed. Keeping that up to date with the latest upstream is of course a good idea.


At the same time, if there's no interest / lack of time to look deeper, if this PR fixes something and there are no regressions, we can as well integrate it. As indicated above, I'm not an expert in this area, and appreciate if someone else takes a decision and does the merge & release work.

@dinosaure

Copy link
Copy Markdown
Member

especially after noticing that Solo5 bindings also vendor a different printf implementation, where I didn’t find support for L* at a quick glance; but I’m not sure it deserves the same trust as musl, I’d say

This is a very basic implementation that uses a fixed amount of memory, as malloc() does not exist in Solo5. The constraints on the printf function in Solo5 are not the same as those in ocaml-solo5. However, I would find it difficult to explain the underlying mechanism that leads to the use of the printf function from nolibc.

All in all, I’m not against this PR in its current form.

shym added a commit to shym/ocaml-solo5 that referenced this pull request Jul 20, 2026
This CI workflow illustrates how this PR plays with changes from
Solo5/solo5#656 and from mirage#177
It should be amended to follow updates on those other changes
shym added a commit to shym/ocaml-solo5 that referenced this pull request Jul 20, 2026
This CI workflow illustrates how this PR plays with changes from
Solo5/solo5#656 and from mirage#177
It should be amended to follow updates on those other changes
@hannesm

hannesm commented Jul 22, 2026

Copy link
Copy Markdown
Member

ok, so I can see that there's sufficient support for this change, and I'll merge that. Putting the burden to review printf differences (between solo5 and ocaml-solo5), updating from upstream printf, ... should not be onto who proposes this PR -- but may be interesting to do separately.

EDIT: I opened #184 to remember that we should look into that.

@hannesm
hannesm merged commit c07be1a into mirage:main Jul 22, 2026
3 checks passed
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.

5 participants