Re: [PATCH][AArch64] Optimized implementation of search_line_fast for the CPP lexer

["Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>]

On 20/03/17 17:27, Andreas Schwab wrote:
> On Mär 20 2017, "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> wrote:
> 
>> I don't have access to an ILP32 run-time environment, so I'm not sure
>> how I'll be able to check this out.  There are some pointer checks in
>> the code so it's possible something is going awry.  Can you compare the
>> assembly output for ILP32 and LP64 to see if there's anything obvious?
> 
> The problem is here:
> 
>       if (__builtin_expect (vpaddd_u64 ((uint64x2_t)t), 0))
> 
> vpaddd_u64 returns a uint64_t value, but __builtin_expect takes a long
> (32-bit in ILP32 mode).
> 

Yikes!  I'm a bit surprised __builtin_expect doesn't take a bool, but I
guess that's due to needing to support old versions of C that lacked
that data type.  Either way, a silent truncation is very undesirable.

> Andreas.
> 
> 	* lex.c (search_line_fast) [__ARM_NEON && __ARM_64BIT_STATE]:
> 	Convert 64-bit value to boolean before passing to
> 	__builtin_expect.

OK.

R.

> 
> diff --git a/libcpp/lex.c b/libcpp/lex.c
> index 8a8c79cde7..a431ac8e05 100644
> --- a/libcpp/lex.c
> +++ b/libcpp/lex.c
> @@ -821,7 +821,7 @@ search_line_fast (const uchar *s, const uchar *end ATTRIBUTE_UNUSED)
>        v = vorrq_u8 (t, vceqq_u8 (data, repl_bs));
>        w = vorrq_u8 (u, vceqq_u8 (data, repl_qm));
>        t = vorrq_u8 (v, w);
> -      if (__builtin_expect (vpaddd_u64 ((uint64x2_t)t), 0))
> +      if (__builtin_expect (vpaddd_u64 ((uint64x2_t)t) != 0, 0))
>  	goto done;
>      }
>  
>