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

ARROW-10610: [C++] Support exponential float nontion on big-endian in fast_float #8674

Closed
wants to merge 1 commit into from

Conversation

kiszk
Copy link
Member

@kiszk kiszk commented Nov 16, 2020

This PR is a follow-up of #8494 . The fast_float vendor library does not support big-endian of exponential float notion in from_chars. This PR correctly copies the available 32-bit (offset 4-7 on big-endian) to the result.

@github-actions
Copy link

@pitrou
Copy link
Member

pitrou commented Nov 16, 2020

Did you report the issue upstream?

@kiszk
Copy link
Member Author

kiszk commented Nov 16, 2020

Not yet. I will report this issue today.

@kiszk
Copy link
Member Author

kiszk commented Nov 16, 2020

@lemire
Copy link

lemire commented Nov 16, 2020

@kiszk Thanks for the report.

@pitrou How do you test Big Endian support? I have different ARM boards, but none of them run in Big Endian mode. I have not been able to find any publicly accessible CI system that run tests on Big Endian systems. I have a POWER system, but it is only little endian.

@lemire
Copy link

lemire commented Nov 16, 2020

We are looking into fixing this upstream (big endian support).

@lemire
Copy link

lemire commented Nov 16, 2020

(The hard part is to add the necessary CI testing.)

@lemire
Copy link

lemire commented Nov 16, 2020

(I managed to add CI tests now.)

@lemire
Copy link

lemire commented Nov 16, 2020

The issue has been fixed upstream (big endian support). I am running exhaustive tests and once they complete, I will issue a new release.

If you wish, you can wait for this new release instead of merging a vendored change.

@wesm
Copy link
Member

wesm commented Nov 17, 2020

Travis CI has s390x architecture (that's how we've been testing on BE)

@lemire
Copy link

lemire commented Nov 17, 2020

@wesm Thanks to @kiszk, I figured that out.

That we could run CI on IBM mainframe-like systems was a shock to me. :-) I knew about POWER, but not that. So I learned something new today.

As written above, I have a patch that work. I'll issue a new release soon.

@lemire
Copy link

lemire commented Nov 17, 2020

Version 0.2.0 of fast_float is out:

https://github.com/lemire/fast_float/releases/tag/v0.2.0

Release notes:

  • Support for Big Endian systems
  • Build with pedantic flags under GNU GCC without warnings
  • More standard compliant handling of problematic strings (issue19)

@kiszk
Copy link
Member Author

kiszk commented Nov 18, 2020

@pitrou Will you open another PR to integrate ver. 0.2.0 of fast_float?

@pitrou
Copy link
Member

pitrou commented Nov 18, 2020

@kiszk I will, unless you beat me to it.

@kiszk
Copy link
Member Author

kiszk commented Nov 18, 2020

@pitrou Sounds good, I will close this PR.

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.

None yet

4 participants