_Float16 undeclared when compiling on aarch64
Scrumplex opened this issue · 12 comments
Environment
toml++ version and/or commit hash: cc741c9
Compiler: GNU 12.1.0
C++ standard mode: 17
Target arch: aarch64 (arm64)
Library configuration overrides: None
Relevant compilation flags: None?
Describe the bug
Building toml++ on aarch64 results in the following error:
In file included from /run/build/prismlauncher/libraries/tomlplusplus/include/toml++/toml.h:13,
from /run/build/prismlauncher/launcher/modplatform/packwiz/Packwiz.cpp:31:
/run/build/prismlauncher/libraries/tomlplusplus/include/toml++/impl/forward_declarations.h:630:29: error: ‘_Float16’ was not declared in this scope; did you mean ‘_Float64’?
630 | struct float_traits<TOML_FLOAT16> : float_traits_base<TOML_FLOAT16, __FLT16_MANT_DIG__, __FLT16_DIG__>
| ^~~~~~~~~~~~
In file included from /run/build/prismlauncher/libraries/tomlplusplus/include/toml++/toml.h:39:
/run/build/prismlauncher/libraries/tomlplusplus/include/toml++/impl/forward_declarations.h:630:41: error: template argument 1 is invalid
630 | struct float_traits<TOML_FLOAT16> : float_traits_base<TOML_FLOAT16, __FLT16_MANT_DIG__, __FLT16_DIG__>
| ^
/run/build/prismlauncher/libraries/tomlplusplus/include/toml++/impl/forward_declarations.h:630:63: error: ‘_Float16’ was not declared in this scope; did you mean ‘_Float64’?
630 | struct float_traits<TOML_FLOAT16> : float_traits_base<TOML_FLOAT16, __FLT16_MANT_DIG__, __FLT16_DIG__>
| ^~~~~~~~~~~~
/run/build/prismlauncher/libraries/tomlplusplus/include/toml++/impl/forward_declarations.h:630:110: error: template argument 1 is invalid
630 | struct float_traits<TOML_FLOAT16> : float_traits_base<TOML_FLOAT16, __FLT16_MANT_DIG__, __FLT16_DIG__>
| ^
/run/build/prismlauncher/libraries/tomlplusplus/include/toml++/impl/forward_declarations.h:651:29: error: ‘_Float16’ was not declared in this scope; did you mean ‘_Float64’?
651 | struct value_traits<TOML_FLOAT16> : float_traits<TOML_FLOAT16>
| ^~~~~~~~~~~~
/run/build/prismlauncher/libraries/tomlplusplus/include/toml++/impl/forward_declarations.h:651:41: error: template argument 1 is invalid
651 | struct value_traits<TOML_FLOAT16> : float_traits<TOML_FLOAT16>
| ^
/run/build/prismlauncher/libraries/tomlplusplus/include/toml++/impl/forward_declarations.h:651:58: error: ‘_Float16’ was not declared in this scope; did you mean ‘_Float64’?
651 | struct value_traits<TOML_FLOAT16> : float_traits<TOML_FLOAT16>
| ^~~~~~~~~~~~
/run/build/prismlauncher/libraries/tomlplusplus/include/toml++/impl/forward_declarations.h:651:70: error: template argument 1 is invalid
651 | struct value_traits<TOML_FLOAT16> : float_traits<TOML_FLOAT16>
|
Note that this issue does not occur on x86_64 with the same toolchain.
Steps to reproduce (or a small repro code sample)
Best I can offer is the Flatpak builder here: flathub/org.prismlauncher.PrismLauncher#6
Additional information
Full build log: https://buildbot.flathub.org/#/builders/42/builds/565/steps/8/logs/stdio
I have downgraded back to 3.2.0 for now
I am assuming that c8780a5 introduced this, but I don't have an aarch64 machine to test on
That commit was an effort to fix fp16-related issues, though I guess I made it worse for some targets. I recently ran into similar issues in another project and have been meaning to backport my findings to toml++. Should be relatively easy to sort out. (I'll add an escape hatch so you can disable all that stuff anyway.)
Adding to this close / stale issue as we see this with the 3.2.0 code:
-
at CRAN the R package RcppTOML around toml++ 3.2.0 fails on arm / aarch64 and i386:
https://cloud.r-project.org/web/checks/check_results_RcppTOML.html -
the Debian compilation of the r-cran-rcpptoml package (based on that CRAN package around toml++ 3.2.0) also fails on arm
https://buildd.debian.org/status/fetch.php?pkg=r-cran-rcpptoml&arch=arm64&ver=0.2.0%2Bdfsg-1&stamp=1674418861&raw=0
Should we just try to build of the master branch? Or are you planning a release soon?
Should we just try to build of the master branch? Or are you planning a release soon?
It will be safe to use the current master, but I will be doing a release in the next week or so if you'd prefer to wait - there's a few other small issues on the backlog that I would like to include.
One of them, #187, is a TOML-language spec compliance issue. Admittedly a pretty weird edge-case, but I'd like to solve it all the same.
Hm. Apparently I already took you master branch code when I bootstrapped my repo of rcpptoml around it. The only difference is in preprocessor.h (my copy so from Jan 1):
edd@rob:~/git/tomlplusplus(master)$ git pull --all
Already up to date.
edd@rob:~/git/tomlplusplus(master)$ rsync -ncav include/toml++/ ../rcpptomlplusplus/inst/include/toml++/
sending incremental file list
impl/
impl/preprocessor.h
sent 2,091 bytes received 19 bytes 4,220.00 bytes/sec
total size is 636,899 speedup is 301.85 (DRY RUN)
edd@rob:~/git/tomlplusplus(master)$ Will give that a shot.
Yup, I just removed my attempts at trying to automatically detect _Float16 support - now it is entirely opt-in via TOML_ENABLE_FLOAT16. Despite my best intentions, compiler bugs kept killing me here.
Yeah. Saw that. I am not quite sure how to handle that, probably need a preprocessor. We can upload to an arm test builder at the R Project, and I just did that (and rushed at first, fooled myself locally) and it passes. But so does the prior version on that machine as that build is 64 bit.
Should I bail and enable TOML_ENABLE_FLOAT16 "everywhere" just in case?
Hmmn? I'm confused. If you enable it everywhere you'll get the same compiler issues you got in 3.2.0.
The thing I removed was the various preprocessor #ifdefs that tried to detect _Float16 support based on the compiler and arch; now it's entirely opt-in such that if you want toml++ to be aware of _Float16, you need to explicitly set TOML_ENABLE_FLOAT16=1, but doing so on a compiler/arch that does not support it will cause issues.
The only thing it impacts is the ability to directly get/set values as _Float16, e.g.:
std::optional<_Float16> f16_val = some_node.value<_Float16>();Which is a pretty niche thing, and unlikely to be useful for you given you're wrapping the library anyways. My recommendation would be to explicitly set TOML_ENABLE_FLOAT16=0.
I am confused to. I build RcppTOML in early Jan off your repo then, I should have most fixes but still have the now-removed heuristic of yours to see when to enable float16 support. That is what I shipped to CRAN, it clearly fails on two platforms: https://cloud.r-project.org/web/checks/check_results_RcppTOML.html My fellow Debianer looking after my (usptream) package also has woes.
I then confused myself once more by using the machine accessible at https://mac.r-project.org/macbuilder/submit.html but the CRAN arm64 aka aarch64 build already passed, it was the plain old macOS that failed per the table in
https://cloud.r-project.org/web/checks/check_results_RcppTOML.html
with log at
https://www.r-project.org/nosvn/R.check/r-release-macos-x86_64/RcppTOML-00install.html
and that looks like a ... compiler issue / the choice of
clang++ -mmacosx-version-min=10.13 -std=gnu++17
creating woes. So different issue.
Back to float16. So when/where should I enable? Only on i386? Why does arm64 fail at Debian in
https://buildd.debian.org/status/fetch.php?pkg=r-cran-rcpptoml&arch=arm64&ver=0.2.0%2Bdfsg-1&stamp=1674418861&raw=0 (and scroll to bottom) when it is 64 bit?
My recommendation would be to explicitly set TOML_ENABLE_FLOAT16=0.
I am extra dense today. Thank you.
I thought our woes were from wanting to enable rather than explicitly disable where it may fail.
Small wins. The 'win-builder' R Project machine, in its 'support previous release of R' setting where it still builds with i386 and x86_64 flavours (current release dropped i386) fared well once I added the define. See https://win-builder.r-project.org/fEJ9pDGItp63/00install.out and copying 'for keeps' as that page will get purhed in a few days:
* installing *source* package 'RcppTOML' ...
** using staged installation
** libs
*** arch - i386
d:/Compiler/rtools40/mingw32/bin/g++ -std=gnu++17 -I"D:/RCompile/recent/R-4.1.3/include" -DNDEBUG -I../inst/include -DTOML_ENABLE_FLOAT16=0 -I'd:/RCompile/CRANpkg/lib/4.1/Rcpp/include' -I"d:/Compiler/gcc-4.9.3/local330/include" -O2 -Wall -mfpmath=sse -msse2 -mstackrealign -c RcppExports.cpp -o RcppExports.o
d:/Compiler/rtools40/mingw32/bin/g++ -std=gnu++17 -I"D:/RCompile/recent/R-4.1.3/include" -DNDEBUG -I../inst/include -DTOML_ENABLE_FLOAT16=0 -I'd:/RCompile/CRANpkg/lib/4.1/Rcpp/include' -I"d:/Compiler/gcc-4.9.3/local330/include" -O2 -Wall -mfpmath=sse -msse2 -mstackrealign -c parse.cpp -o parse.o
d:/Compiler/rtools40/mingw32/bin/g++ -shared -s -static-libgcc -o RcppTOML.dll tmp.def RcppExports.o parse.o -Ld:/Compiler/gcc-4.9.3/local330/lib/i386 -Ld:/Compiler/gcc-4.9.3/local330/lib -LD:/RCompile/recent/R-4.1.3/bin/i386 -lR
installing to d:/RCompile/CRANguest/R-oldrelease/lib/00LOCK-RcppTOML/00new/RcppTOML/libs/i386
*** arch - x64
d:/Compiler/rtools40/mingw64/bin/g++ -std=gnu++17 -I"D:/RCompile/recent/R-4.1.3/include" -DNDEBUG -I../inst/include -DTOML_ENABLE_FLOAT16=0 -I'd:/RCompile/CRANpkg/lib/4.1/Rcpp/include' -I"d:/Compiler/gcc-4.9.3/local330/include" -O2 -Wall -mfpmath=sse -msse2 -mstackrealign -c RcppExports.cpp -o RcppExports.o
d:/Compiler/rtools40/mingw64/bin/g++ -std=gnu++17 -I"D:/RCompile/recent/R-4.1.3/include" -DNDEBUG -I../inst/include -DTOML_ENABLE_FLOAT16=0 -I'd:/RCompile/CRANpkg/lib/4.1/Rcpp/include' -I"d:/Compiler/gcc-4.9.3/local330/include" -O2 -Wall -mfpmath=sse -msse2 -mstackrealign -c parse.cpp -o parse.o
d:/Compiler/rtools40/mingw64/bin/g++ -shared -s -static-libgcc -o RcppTOML.dll tmp.def RcppExports.o parse.o -Ld:/Compiler/gcc-4.9.3/local330/lib/x64 -Ld:/Compiler/gcc-4.9.3/local330/lib -LD:/RCompile/recent/R-4.1.3/bin/x64 -lR
installing to d:/RCompile/CRANguest/R-oldrelease/lib/00LOCK-RcppTOML/00new/RcppTOML/libs/x64
** R
** inst
** byte-compile and prepare package for lazy loading
** help
*** installing help indices
** building package indices
** testing if installed package can be loaded from temporary location
*** arch - i386
*** arch - x64
** testing if installed package can be loaded from final location
*** arch - i386
*** arch - x64
** testing if installed package keeps a record of temporary installation path
* MD5 sums
packaged installation of 'RcppTOML' as RcppTOML_0.2.0.1.zip
* DONE (RcppTOML)
That is with the updated preprocessor.h and with -DTOML_ENABLE_FLOAT16=0 (as shown).