This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, PR83492] Fix selection of aarch64 big-endian shift parameters based on __AARCH64EB__
- From: Jeff Law <law at redhat dot com>
- To: Jakub Jelinek <jakub at redhat dot com>, Michael Weiser <michael dot weiser at gmx dot de>
- Cc: Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, "Richard Earnshaw (lists)" <richard dot earnshaw at arm dot com>, James Greenhalgh <james dot greenhalgh at arm dot com>, Marcus Shawcroft <marcus dot shawcroft at arm dot com>, Aldy Hernandez <aldyh at redhat dot com>
- Date: Tue, 19 Dec 2017 17:23:20 -0700
- Subject: Re: [PATCH, PR83492] Fix selection of aarch64 big-endian shift parameters based on __AARCH64EB__
- Authentication-results: sourceware.org; auth=none
- References: <20171219171011.GA9667@weiser.dinsnail.net> <5A394DC7.5080507@foss.arm.com> <20171219180144.GB9667@weiser.dinsnail.net> <20171219182332.GX2353@tucnak>
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