On 20/12/17 00:23, Jeff Law wrote:
On 12/19/2017 11:23 AM, Jakub Jelinek wrote:
On Tue, Dec 19, 2017 at 07:01:44PM +0100, Michael Weiser wrote:
Hi Kyrill,

On Tue, Dec 19, 2017 at 05:35:03PM +0000, Kyrill Tkachov wrote:

below patch fixes PR 83492.
I agree with your analysis of the bug and your patch will fix the
problem when compiling with GCC. However, I believe the more
appropriate macro to check here is __ARM_BIG_ENDIAN as that
is the macro defined by the ACLE specification [1].
__AARCH64EB__ is indeed defined by aarch64 GCC when compiling
for big-endian but I don't think it's standardised and may in theory
not be defined by other host compilers the user may use to compile GCC.
Since the macro is used in various places around the code, I'd suggest
fixing this bug and getting back in line with the rest of the codebase
by applying my patch as-is. The move towards a more appropriate macro to
base the checks on can then be done in a separate step using
search'n'replace.

$ grep -r __AARCH64EB__ . | cut -d: -f1 | sort -u | grep -v \.svn
./gcc/config/aarch64/aarch64-c.c
./gcc/config/aarch64/arm_neon.h
./gcc/testsuite/lib/target-supports.exp
./libcpp/ChangeLog
./libcpp/lex.c
./libffi/src/aarch64/ffi.c
./libffi/src/aarch64/sysv.S
./libgcc/config/aarch64/linux-unwind.h
./libgcc/config/aarch64/sfp-machine.h
./libstdc++-v3/config/cpu/aarch64/opt/ext/opt_random.h
That doesn't tell anything on what should be used in libcpp.  Unlike the
rest, libcpp is a library that needs to be built on the host, with the
host compiler (which doesn't have to be GCC).  The only case that is in
the same category is opt_random.h, because it is an installed header and
so again, can be used by GCC or other compilers.
GCC isn't going to stop defining __AARCH64EB__ nor __ARM_BIG_ENDIAN,
it is purely about other compilers.
I think using ACLE specified macro is the way to go here.  At least for
libcpp.  opt_random seems like a good one to fix, particularly for
situations where folks may be using LLVM with GCC's libstdc++.

A patch to use __ARM_BIG_ENDIAN in those two spots is approved.

I'll take care of the opt_random occurrence.
Michael, would you like to respin your patch to libcpp to use __ARM_BIG_ENDIAN?

Thanks,
Kyrill

jeff

Reply via email to