This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, PR83492] Fix selection of aarch64 big-endian shift parameters based on __AARCH64EB__


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]