Re: [PATCH 06/10] target/arm: Convert Neon narrowing shifts with op==8 to decodetree

[Richard Henderson]

On 5/15/20 7:20 AM, Peter Maydell wrote:
> Convert the Neon narrowing shifts where op==8 to decodetree:
>  * VSHRN
>  * VRSHRN
>  * VQSHRUN
>  * VQRSHRUN
> 
> Signed-off-by: Peter Maydell <address@hidden>
> ---
>  target/arm/neon-dp.decode       |  32 ++++++
>  target/arm/translate-neon.inc.c | 168 ++++++++++++++++++++++++++++++++
>  target/arm/translate.c          |   1 +
>  3 files changed, 201 insertions(+)
> 
> diff --git a/target/arm/neon-dp.decode b/target/arm/neon-dp.decode
> index 6456b53a690..f8d19c5819c 100644
> --- a/target/arm/neon-dp.decode
> +++ b/target/arm/neon-dp.decode
> @@ -208,6 +208,10 @@ VMINNM_fp_3s     1111 001 1 0 . 1 . .... .... 1111 ... 1 
> .... @3same_fp
>  
>  @2reg_shift      .... ... . . . ...... .... .... . q:1 . . .... \
>                   &2reg_shift vm=%vm_dp vd=%vd_dp
> +@2reg_shift_q0   .... ... . . . ...... .... .... . 0 . . .... \
> +                 &2reg_shift vm=%vm_dp vd=%vd_dp q=0
> +@2reg_shift_q1   .... ... . . . ...... .... .... . 1 . . .... \
> +                 &2reg_shift vm=%vm_dp vd=%vd_dp q=1

I'm not sure this part makes sense.  Correct, you cannot leave the q field
unset and continue to use &2reg_shift, but the insn field q is decode.  We wind
up with VSHRN having q=0 and VRSHRN having q=1, which is a distinction without
meaning.

While we could perhaps reasonably set q to a consistent constant, the only
driving reason to do so would be to share code with do_vector_2sh or
do_2shift_env_*.

But since we can't do that, due to the expansion algorithm, I think it would be
better to create a new &2reg_shift_nq that does not contain the q field.

The rest of the code looks good.


r~