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

Fails to detect endianness on 64-bit ARM #39

Closed
musicinmybrain opened this issue Sep 25, 2021 · 3 comments · Fixed by #40
Closed

Fails to detect endianness on 64-bit ARM #39

musicinmybrain opened this issue Sep 25, 2021 · 3 comments · Fixed by #40

Comments

@musicinmybrain
Copy link
Contributor

On 64-bit ARM (aarch64), which can technically be little- or big-endian, since c233f40 the project cannot compile because the endianness is not successfully detected from predefined preprocessor macros.

I’ve tested this on Linux, with recent (11.x) GCC, on little-endian aarch64.

In file included from /builddir/build/BUILD/c4core-0.1.3/src/c4/config.hpp:34,
                 from /builddir/build/BUILD/c4core-0.1.3/src/c4/error.hpp:8,
                 from /builddir/build/BUILD/c4core-0.1.3/src/c4/error.cpp:1:
/builddir/build/BUILD/c4core-0.1.3/src/c4/cpu.hpp:76:9: error: #error "unknown endianness"
   76 | #       error "unknown endianness"
      |         ^~~~~

Perhaps it would help, across all architectures, to look for __BYTE_ORDER__ and/or __FLOAT_WORD_ORDER__, which may be defined to __ORDER_BIG_ENDIAN__, __ORDER_LITTLE_ENDIAN__, or (if you are cursed) __ORDER_PDP_ENDIAN__. These are GCC-specific, but they allow you to tell which mode is in use on bi-endian architectures.

Similarly, some toolchains (clang, I think) define __BIG_ENDIAN__ or __LITTLE_ENDIAN__. When present, they are conclusive.

Finally, according to https://sourceforge.net/p/predef/wiki/Endianness/, some environments apparently define __AARCH64EB__ or __AARCH64EL__ on aarch64. These are not present on gcc/Linux.

@biojppm
Copy link
Owner

biojppm commented Sep 25, 2021

Thanks for reporting.

The byte order check for aarch64 (and all other ARMs) is this:

   #ifdef __ARMEL__
       #define C4_BYTE_ORDER _C4EL
   #elif defined(__ARMEB__)
       #define C4_BYTE_ORDER _C4EB
   #else
       #error "unknown endianness"
   #endif

There are no tests for this architecture (or s390x) in the current pipeline, and although I intend to add them unfortunately I cannot work on that ATM.

So can you confirm this (or something similar) fixes your problem?

   #if defined(__ARMEL__) || defined(__LITTLE_ENDIAN__) || defined(__AARCH64EL__) \
       || (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__))
       #define C4_BYTE_ORDER _C4EL
   #elif defined(__ARMEB__) || defined(__BIG_ENDIAN__) || defined(__AARCH64EB__) \
       || (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__))
       #define C4_BYTE_ORDER _C4EB
   #elif defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_PDP_ENDIAN__)
       #define C4_BYTE_ORDER _C4EM
   #else
       #error "unknown endianness"
   #endif

@biojppm
Copy link
Owner

biojppm commented Sep 26, 2021

I added s390x tests to the CI (as well as aarch64 and ppc64le), and they succeed with this change. So the PR will close this.

@musicinmybrain
Copy link
Contributor Author

Thanks! I didn’t have a chance to test your change yet, but this is appreciated.

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 a pull request may close this issue.

2 participants