Skip to content

Attempt to Add float16_t support#123

Draft
spinicist wants to merge 1 commit intovitaut:mainfrom
spinicist:half
Draft

Attempt to Add float16_t support#123
spinicist wants to merge 1 commit intovitaut:mainfrom
spinicist:half

Conversation

@spinicist
Copy link
Copy Markdown
Contributor

Hello,

As discussed here fmtlib/fmt#4725 I need to format float16_ts. I started trying to work on this today, but have failed early on.

I am working with GCC 15.2 (Clang does not have stdfloat support yet). I have changed CMakeLists.txt to compile in C++23 mode. I then started to add float16_t overloads. However, if I compile with the set of changes in this MR I get:

/software/system/gcc/15.2/bin/g++  -I/home/k1078535/Code/zmij/. -std=gnu++23 -pthread -MD -MT CMakeFiles/zmij.dir/zmij.cc.o -MF CMakeFiles/zmij.dir/zmij.cc.o.d -o CMakeFiles/zmij.dir/zmij.cc.o -c /home/k1078535/Code/zmij/zmij.cc
In file included from /home/k1078535/Code/zmij/zmij.cc:25:
/nan/pacific/network/system/el8/gcc/15.2/include/c++/15.2.0/limits:2015:27: error: ‘__gnu_cxx’ was not declared in this scope; did you mean ‘zmij::__gnu_cxx’?
 2015 |     struct numeric_limits<__gnu_cxx::__bfloat16_t>
      |                           ^~~~~~~~~
      |                           zmij::__gnu_cxx
In file included from /nan/pacific/network/system/el8/gcc/15.2/include/c++/15.2.0/stdfloat:33,
                 from /home/k1078535/Code/zmij/zmij.h:66,
                 from /home/k1078535/Code/zmij/zmij.cc:8:
/nan/pacific/network/system/el8/gcc/15.2/include/c++/15.2.0/x86_64-pc-linux-gnu/bits/c++config.h:373:11: note: ‘zmij::__gnu_cxx’ declared here
  373 | namespace __gnu_cxx
      |           ^~~~~~~~~

This doesn't make any sense to me - any ideas are most welcome.

(I have checked that a simple test float16_t program not using zmij compiles correctly on my setup).

@Alcaro
Copy link
Copy Markdown

Alcaro commented Apr 13, 2026

This doesn't make any sense to me - any ideas are most welcome.

Your #include is inside a namespace zmij {.

#include in namespaces is not allowed, move it outside.

You'll most likely need to duplicate that #ifdef, a namespace zmij {, or something.

@spinicist
Copy link
Copy Markdown
Contributor Author

I swear I had tried that initially and seen the same error. Apologies for wasting your time.

@spinicist spinicist changed the title A Failed Attempt to Add float16_t support Attempt to Add float16_t support Apr 14, 2026
@spinicist
Copy link
Copy Markdown
Contributor Author

spinicist commented Apr 14, 2026

I have made some progress now. I have added enough #ifdef __STDCPP_FLOAT16_T__ that I can compile a modified example.cc to output a float16_t. However, the output is incorrect. It prints 0.0001 instead of 1. I suspect that I have the traits set up wrong, but I can't see where. I'll continue to hack on this, but any suggestions are very welcome.

P.S. One definite issue is that I'm getting bad information from dec_exp_formats.get<>. This gives a start_pos of 2 for float16_t when by comparison to float I think it should be 0.

Copy link
Copy Markdown
Owner

@vitaut vitaut left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Please revert any unrelated changes such as commenting out sse tests.

@spinicist
Copy link
Copy Markdown
Contributor Author

Sorry - I should have made it clearer that:

  1. This PR isn't ready to merge. If there's a better forum to ask for help with getting it ready, let me know.
  2. I forgot that I disabled SIMD entirely for now. I thought it best to get it correct first and then re-enable and work out the correct assembly for that section.

@Antares0982
Copy link
Copy Markdown
Contributor

I'm also very interested in float16-to-string conversion. But I'm wondering whether we can avoid using std::float16_t and instead use a uint16_t or another type of the same width as a substitute, since std::float16_t requires additional compiler support and checks.

@vitaut
Copy link
Copy Markdown
Owner

vitaut commented Apr 14, 2026

I'm wondering whether we can avoid using std::float16_t and instead use a uint16_t or another type of the same width as a substitute, since std::float16_t requires additional compiler support and checks.

Let's first make std::float16_t work without these complications.

@spinicist
Copy link
Copy Markdown
Contributor Author

@vitaut A better explanation: The SSE tests are commented out because:

  1. I was getting SIMD issues in zmij.cc
  2. Hence I set # define ZMIJ_USE_SIMD 0 for now
  3. This then made the SSE tests fail to compile

What is the correct way to disable SIMD for both zmij and the tests?

@spinicist spinicist marked this pull request as draft April 15, 2026 09:34
Comment thread zmij.cc Outdated
};

#ifdef __STDCPP_FLOAT16_T__
template <> struct float_traits<std::float16_t> : std::numeric_limits<std::float16_t> {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You shouldn't be adding a new trait but update the main trait to handle 16-bit float correctly (mostly num_bits).

@vitaut
Copy link
Copy Markdown
Owner

vitaut commented Apr 15, 2026

What is the correct way to disable SIMD for both zmij and the tests?

In your local build you can set ZMIJ_USE_SIMD macro to 0. In zmij itself you shouldn't be disabling anything - if there is an issue it should be investigated and fixed.

Comment thread zmij.h Outdated
auto to_decimal(double value) noexcept -> dec_fp;

enum {
half_buffer_size = 9,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

half -> float16

@spinicist
Copy link
Copy Markdown
Contributor Author

I have rebased and rewritten this incorporating your requests. It still gives incorrect output.

I think the key issue is here: https://github.com/vitaut/zmij/blob/main/zmij.cc#L1063

I assume this needs an extra special case for 16 bits, but I don't know which constants then need changing. Any pointers much appreciated.

@vitaut
Copy link
Copy Markdown
Owner

vitaut commented Apr 22, 2026

I think the key issue is here: https://github.com/vitaut/zmij/blob/main/zmij.cc#L1063

I think the code you were referring to has moved. Use a permalink to prevent this.

@spinicist
Copy link
Copy Markdown
Contributor Author

I think the code you were referring to has moved. Use a permalink to prevent this.

Yes, it has moved up slightly:

zmij/zmij.cc

Line 975 in f1126cd

if (num_bits == 32) {

(This is a block that checks for 32 bits)

@vitaut
Copy link
Copy Markdown
Owner

vitaut commented Apr 24, 2026

That particular check could be changed to <= 32 although we'll probably need more adjustments elsewhere.

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.

4 participants