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

[Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>]

Hi Michael,

[cc'ing the aarch64 maintainers]

On 19/12/17 17:10, Michael Weiser wrote:
> 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

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.

But, it may not be a big deal in practice. It's up to the maintainers...

Thanks,
Kyrill

[1] https://static.docs.arm.com/ihi0053/c/IHI0053C_acle_2_0.pdf

> 2017-12-19  Michael Weiser  <michael.weiser@gmx.de>
>
>         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};