Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Resolve the macro temporary-name collision and correct the byte-order test plan count.
4 open findings
What changed in this PR
Updates floating-point and native byte-order conversions to use memcpy() and shared endian helpers while preserving stored formats.
Changes:
- Centralizes conversion macros in
my_byteorder.h. - Updates MyISAM, filesort, InnoDB, and
dtoaconversions. - Removes obsolete endian-specific headers and expands byte-order tests.
| File | Change |
|---|---|
unittest/mysys/byte_order-t.c |
Expands floating-point and native-order conversion tests. |
strings/dtoa.c |
Removes obsolete mixed-endian handling. |
storage/innobase/include/mach0data.inl |
Migrates InnoDB conversions to shared macros. |
sql/filesort.cc |
Uses big-endian floating-point storage. |
include/myisampack.h |
Adds MyISAM floating-point aliases. |
include/my_byteorder.h |
Adds centralized endian conversion macros. |
include/little_endian.h |
Removes obsolete header. |
include/CMakeLists.txt |
Removes deleted headers from installation. |
include/big_endian.h |
Removes obsolete header. |
🧠 Review effort: Lite
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
vaintroub
force-pushed
the
11.4-MDEV-39150
branch
2 times, most recently
from
October 9, 2026 16:09
bd02033 to
5b9dd95
Compare
MDEV-37788 converted the uintNkorr() and intNstore() macros to use memcpy() and byte swap intrinsics, but the floating point and short/long conversion macros in big_endian.h and myisampack.h still accessed data one byte at a time, and those in little_endian.h were a mix of both. Compilers may emit slow byte-at-a-time loads and stores for such code. Reimplement the macros with memcpy(), and with MY_BSWAP32/MY_BSWAP64 where the byte order differs from the host. big_endian.h and little_endian.h no longer differ, so move the macros to my_byteorder.h and remove the two headers, which are no longer installed. Implement mi_float4store() etc. in myisampack.h with the same helper macros. Remove the unused ulongget() macro, and the code for the mixed-endian floating point layout (a little-endian CPU with big-endian floating point word order) from the macros, change_double_for_sort() and dtoa.c. It only applied to the obsolete ARM FPA format. Reimplement mach_double_read(), mach_double_write(), mach_float_read() and mach_float_write() in InnoDB with float8get(), float8store(), float4get() and float4store(), instead of copying bytes in a loop. The stored formats do not change. The unit test byte_order-t now checks the byte layout of the floating point macros and the sign extension of the native byte order macros, so no MTR test is added.
vaintroub
force-pushed
the
11.4-MDEV-39150
branch
from
October 9, 2026 16:17
5b9dd95 to
6f569ce
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://jira.mariadb.org/browse/MDEV-39150
MDEV-37788 converted the
uintNkorr()andintNstore()macros to usememcpy()and byte swap intrinsics. The floating point and short/long macros inbig_endian.h,little_endian.handmyisampack.hstill accessed data one byte at a time, which compilers may translate to slow byte-at-a-time loads and stores.Changes
memcpy(), and withMY_BSWAP32/MY_BSWAP64where the byte order differs from the host. The helper macros areMY_SWAPCPY4/8andMY_COPY_LE4/LE8/BE4/BE8.big_endian.handlittle_endian.hno longer differ, so the macros move tomy_byteorder.hand the two headers are removed (no longer installed).float4store,float4get,float8storeandfloat8getkeep their names and store little-endian on any host.mi_float4store()etc. inmyisampack.huseMY_COPY_BE4/BE8directly and store big-endian. No new public macro names are added.change_double_for_sort()usesmi_float8store().float*getandmi_float*getmacros check the destination size withcompile_time_assert().ulongget()macro and the mixed-endian floating point layout (little-endian CPU, big-endian float word order; obsolete ARM FPA) from the macros,change_double_for_sort()anddtoa.c.mach_double_read(),mach_double_write(),mach_float_read()andmach_float_write()in InnoDB withfloat8get(),float8store(),float4get()andfloat4store().The stored formats do not change.
Testing
unittest/mysys/byte_order-tis extended to check the byte layout of the floating point macros (little-endianfloat4*/float8*, big-endianmi_float*, aligned and unaligned, -0.0 and NaN payload) and sign/zero extension of the native byte order macros: 108 checks pass on x86, and on s390x under qemu-user.innodb_gispass.s390x-ubuntu-2204-debug(big-endian) built this PR and finished with a successful result (build 17388); its mtr step reported warnings.