Skip to content

Conversation

@vejbomar
Copy link
Contributor

Update fast_float code to use correct 64bit full multiplication for MinGW on ARM64. This fixes the following error when building with MinGW for ARM64 from https://github.com/Windows-on-ARM-Experiments/mingw-woarm64-build:

In file included from ./boost/charconv/detail/fast_float/fast_float.hpp:11,
                 from libs/charconv/build/../src/from_chars.cpp:16:
./boost/charconv/detail/fast_float/float_common.hpp: In function ‘boost::charconv::detail::fast_float::value128 boost::charconv::detail::fast_float::full_multiplication(uint64_t, uint64_t)’:
./boost/charconv/detail/fast_float/float_common.hpp:254:16: error: ‘_umul128’ was not declared in this scope; did you mean ‘umul128’?
  254 |   answer.low = _umul128(a, b, &high); // _umul128 not available on ARM64
      |                ^~~~~~~~
      |                umul128

Corresponding change in fast_float repository: fastfloat/fast_float#269

Please let me know if there is some other way how the code from fast_float is updated here. If this is fine, I'll create a similar PR in https://github.com/boostorg/json

Copy link
Member

@mborland mborland left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks. @grisumbras is there any need to update in JSON? I thought you were moving to charconv this cycle with dropping GCC 4.X series.

@grisumbras
Copy link
Member

I'll be switching to Charconv after the next release (that is, after 1.87).

@mborland
Copy link
Member

I'll be switching to Charconv after the next release (that is, after 1.87).

Ok. @vejbomar if you could transpose this into the same place in Boost.JSON for the meantime that would be great. I'm going to merge it and then fix the CI. Need to update the Drone images to LTS.

@vejbomar
Copy link
Contributor Author

Thank you for the brisk merging! I've created PR for Boost.JSON as suggested: boostorg/json#1057

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