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

[Michael Weiser]

Hello,

below patch fixes PR 83492.

Running the preprocessor on any source gives various errors on
aarch64_be. These suggest that the proprocessor misidentifies token
boundaries. This can be traced to an optimized search_line_fast for
aarch64 in libcpp/lex.c which has a special provision for aarch64_be
based on the __AARCH64EB. This check can never succeed since the correct
name of the macro is __AARCH64EB__. Consequently the code for big-endian
aarch64 is never activated. As expected, changing __AARCH64EB to
__AARCH64EB__ or disabling the accelerated routine by substituting
search_line_acc_char() fixes the problem.

Thanks,
Michael

2017-12-19  Michael Weiser  

PR preprocessor/83492
* lex.c (search_line_fast) [__ARM_NEON && __ARM_64BIT_STATE]:
Fix selection of big-endian shift parameters based on __AARCH64EB__.

Index: lex.c
===
--- lex.c   (revision 255828)
+++ lex.c   (working copy)
@@ -772,7 +772,7 @@
   const uint8x16_t repl_qm = vdupq_n_u8 ('?');
   const uint8x16_t xmask = (uint8x16_t) vdupq_n_u64 (0x8040201008040201ULL);
 
-#ifdef __AARCH64EB
+#ifdef __AARCH64EB__
   const int16x8_t shift = {8, 8, 8, 8, 0, 0, 0, 0};
 #else
   const int16x8_t shift = {0, 0, 0, 0, 8, 8, 8, 8};

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

[Michael Weiser]

Hi Kyrill,

On Tue, Dec 19, 2017 at 05:35:03PM +, 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
-- 
Thanks,
Michael

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

[Michael Weiser]

Hi Kyrill,

On Wed, Dec 20, 2017 at 09:25:07AM +, Kyrill Tkachov wrote:

> > 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?

Happy to. Created yesterday against trunk, no changes with today's trunk.
Tested successfully by applying to 7.2.0, bootstrapping on
aarch64_be-unknown-linux-gnu and compiling various OS packages with the
resulting compiler.

2017-12-20  Michael Weiser  

PR preprocessor/83492
* lex.c (search_line_fast) [__ARM_NEON && __ARM_64BIT_STATE]:
Fix selection of big-endian shift parameters by using
__ARM_BIG_ENDIAN.

Index: libcpp/lex.c
===
--- libcpp/lex.c(revision 255828)
+++ libcpp/lex.c(working copy)
@@ -772,7 +772,7 @@
   const uint8x16_t repl_qm = vdupq_n_u8 ('?');
   const uint8x16_t xmask = (uint8x16_t) vdupq_n_u64 (0x8040201008040201ULL);
 
-#ifdef __AARCH64EB
+#ifdef __ARM_BIG_ENDIAN
   const int16x8_t shift = {8, 8, 8, 8, 0, 0, 0, 0};
 #else
   const int16x8_t shift = {0, 0, 0, 0, 8, 8, 8, 8};
-- 
Michael