Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix use of intrinsics on windows ARM platforms #928

Merged
merged 2 commits into from
Aug 23, 2023

Conversation

mborland
Copy link
Member

@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Merging #928 (0c1b469) into develop (e084a29) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #928   +/-   ##
========================================
  Coverage    92.95%   92.95%           
========================================
  Files           85       85           
  Lines         8047     8047           
========================================
  Hits          7480     7480           
  Misses         567      567           
Files Changed Coverage Δ
.../boost/json/detail/charconv/detail/emulated128.hpp 0.00% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e084a29...0c1b469. Read the comment docs.

@cppalliance-bot
Copy link

@cppalliance-bot
Copy link

@grisumbras
Copy link
Member

I feel like rather than doing if defined(BOOST_JSON_HAS_MSVC_64BIT_INTRINSICS) && !defined(_M_ARM64) and similar, we should have BOOST_JSON_HAS_MSVC_X86_64_INTRINSICS, BOOST_JSON_HAS_MSVC_ARM64_INTRINSICS BOOST_JSON_HAS_MSVC_X86_INTRINSICS, BOOST_JSON_HAS_MSVC_ARM32_INTRINSICS. Or does the overlap make it impractical?

@mborland
Copy link
Member Author

I feel like rather than doing if defined(BOOST_JSON_HAS_MSVC_64BIT_INTRINSICS) && !defined(_M_ARM64) and similar, we should have BOOST_JSON_HAS_MSVC_X86_64_INTRINSICS, BOOST_JSON_HAS_MSVC_ARM64_INTRINSICS BOOST_JSON_HAS_MSVC_X86_INTRINSICS, BOOST_JSON_HAS_MSVC_ARM32_INTRINSICS. Or does the overlap make it impractical?

There is overlap in the 64-bit platforms with __umulh, and the bit scan intrinsics which are the primary ones used. If you want me to change it I can.

@grisumbras
Copy link
Member

Nah, let's merge it as-is. I'm expecting to remove it with dependency on Boost.Charconv soon anyway.

@grisumbras grisumbras merged commit 0c1b469 into boostorg:develop Aug 23, 2023
4 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.

__emulu() is not available on arm _umul128() is not available on arm64
3 participants