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. jeff