This is an archive of the discontinued LLVM Phabricator instance.

[flang] Use CMake to determine endianness.
ClosedPublic

Authored by Meinersbur on Sep 1 2021, 4:18 PM.

Details

Summary

The preprocessor definitions __BYTE_ORDER__, __ORDER_BIG_ENDIAN__, and __ORDER_LITTLE_ENDIAN__ are gcc extensions (also supported by clang), but msvc (and maybe others) do not define them. As a result BYTE_ORDER and ORDER_BIG_ENDIAN both evaluate to 0 by the prepreprocess, and __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ , the first #if condition to 1, assuming the wrong byte order in x86(_64).

This patch instead uses CMake's TestBigEndian module to determine target architecture's endianness at configure time.

Note this also uses the same mechanism for the runtime. If compiling flang as a cross-compiler, the runtime for the compile-target must be built separately (Flang does not support the LLVM_ENABLE_RUNTIMES mechanism).

Fixes llvm.org/PR51597

Diff Detail

Event Timeline

Meinersbur created this revision.Sep 1 2021, 4:18 PM
Meinersbur requested review of this revision.Sep 1 2021, 4:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 1 2021, 4:18 PM
Leporacanthicus accepted this revision.Sep 2 2021, 6:59 AM

Ah, this definitely would make "random" values for floating point values - in weird ways that are hard to understand!

Looks good to me!

This revision is now accepted and ready to land.Sep 2 2021, 6:59 AM
ijan1 accepted this revision.Sep 2 2021, 12:25 PM

This fixed the issue, I checked and none of the tests fail now. Thanks a ton.
LGTM.

This revision was automatically updated to reflect the committed changes.

Time to upgrade to C++20?
https://en.cppreference.com/w/cpp/types/endian

This change conflates the endianness of the compile-time host with the endianness of the target. Perhaps it better for now to hard-code WIN32 as little endian?

sscalpone edited reviewers, added: klausler; removed: tskeith.Sep 28 2021, 6:35 AM

This change conflates the endianness of the compile-time host with the endianness of the target. Perhaps it better for now to hard-code WIN32 as little endian?

The current build system uses the same compiler for host and runtime. As mentioned in the summary, LLVM has the LLVM_ENABLE_RUNTIMES system to build runtimes for multiple targets, which unfortunately is not used by flang.

I would be loathe to adopt a newer C++ standard at the moment. Didn't the larger LLVM community initially object to Flang's usage of C++17 because the rest of the codebase only used C++11? I don't remember hearing that they have updated standards yet, and I wouldn't want to create a bigger stumbling block for integration with the larger community.

I would be loathe to adopt a newer C++ standard at the moment. Didn't the larger LLVM community initially object to Flang's usage of C++17 because the rest of the codebase only used C++11? I don't remember hearing that they have updated standards yet, and I wouldn't want to create a bigger stumbling block for integration with the larger community.

I agree.

While LLVM has since switched to C++14, we should wait for the rest of LLVM to switch to C++17 before considering C++20.

@Meinersbur I understand.

Others: Re: C++20 :)

This uses a manually-maintained list of compilers and with their endianness

llvm/Support/SwapByteOrder.h
#if defined(__linux__) || defined(__GNU__) || defined(__HAIKU__) ||            \
    defined(__Fuchsia__) || defined(__EMSCRIPTEN__)
#include <endian.h>
#elif defined(_AIX)
#include <sys/machine.h>
#elif defined(__sun)
/* Solaris provides _BIG_ENDIAN/_LITTLE_ENDIAN selector in sys/types.h */
#include <sys/types.h>
#define BIG_ENDIAN 4321
#define LITTLE_ENDIAN 1234
#if defined(_BIG_ENDIAN)
#define BYTE_ORDER BIG_ENDIAN
#else
#define BYTE_ORDER LITTLE_ENDIAN
#endif
#elif defined(__MVS__)
#define BIG_ENDIAN 4321
#define LITTLE_ENDIAN 1234
#define BYTE_ORDER BIG_ENDIAN
#else
#if !defined(BYTE_ORDER) && !defined(_WIN32)
#include <machine/endian.h>
#endif
#endif

I personally find asking cmake the cleaner solution (it does introspection anyway, and endianness must be consistent with the rest of build environment), but I guess the LLVM community will be committed to maintain that list with all supported compilers and would ensure consistency with the rest of LLVM, I could use that if others prefer it.