diff mbox

[6/6] target-arm: fix decoding of Neon 64 bit shifts.

Message ID 1297437062-6118-7-git-send-email-christophe.lyon@st.com
State New
Headers show

Commit Message

Christophe Lyon Feb. 11, 2011, 3:11 p.m. UTC 
From: Christophe Lyon <christophe.lyon@st.com>

Fix decoding of 64 bits variants of VSHRN, VRSHRN, VQSHRN, VQSHRUN, VQRSHRN, VQRSHRUN, taking into account whether inputs are unsigned or not.

Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
---
 target-arm/translate.c |   43 ++++++++++++++++++++++++++++---------------
 1 files changed, 28 insertions(+), 15 deletions(-)

Comments

Peter Maydell Feb. 14, 2011, 5:53 p.m. UTC | #1 
On 11 February 2011 15:11,  <christophe.lyon@st.com> wrote:
> From: Christophe Lyon <christophe.lyon@st.com>
>
> Fix decoding of 64 bits variants of VSHRN, VRSHRN, VQSHRN, VQSHRUN, VQRSHRN, VQRSHRUN, taking into account whether inputs are unsigned or not.
>
> Signed-off-by: Christophe Lyon <christophe.lyon@st.com>

Mostly OK (gives correct answers). Style issues:

>                         tmp = neon_load_reg(rm + pass, 0);
> -                        gen_neon_shift_narrow(size, tmp, tmp2, q, u);
> +                        gen_neon_shift_narrow(size, tmp, tmp2, q, input_unsigned);
>                         tmp3 = neon_load_reg(rm + pass, 1);
> -                        gen_neon_shift_narrow(size, tmp3, tmp2, q, u);
> +                        gen_neon_shift_narrow(size, tmp3, tmp2, q, input_unsigned);

These lines are >80 chars now.

>                     } else {
> -                        if (op == 8)
> +                        if (u) { /* VQSHRN / VQRSHRN */
> +                        gen_neon_narrow_satu(size - 1, tmp, cpu_V0);
> +                        } else { /* VQSHRN / VQRSHRN */

Missing indentation.

The other problem with this area of the code is that it is not
correctly handling the case where pass 1 wants to read a
register which is the target for pass 2. I have a patch to fix
this which I'll post in a moment.

-- PMM
diff mbox

Patch

diff --git a/target-arm/translate.c b/target-arm/translate.c
index ace533f..10b8c5f 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4815,6 +4815,8 @@  static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
             } else if (op < 10) {
                 /* Shift by immediate and narrow:
                    VSHRN, VRSHRN, VQSHRN, VQRSHRN.  */
+                int input_unsigned = (op == 8) ? !u : u;
+
                 shift = shift - (1 << (size + 3));
                 size++;
                 switch (size) {
@@ -4841,33 +4843,44 @@  static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
                     if (size == 3) {
                         neon_load_reg64(cpu_V0, rm + pass);
                         if (q) {
-                          if (u)
-                            gen_helper_neon_rshl_u64(cpu_V0, cpu_V0, tmp64);
-                          else
-                            gen_helper_neon_rshl_s64(cpu_V0, cpu_V0, tmp64);
+                            if (input_unsigned) {
+                                gen_helper_neon_rshl_u64(cpu_V0, cpu_V0,
+                                                         tmp64);
+                            } else {
+                                gen_helper_neon_rshl_s64(cpu_V0, cpu_V0,
+                                                         tmp64);
+                            }
                         } else {
-                          if (u)
-                            gen_helper_neon_shl_u64(cpu_V0, cpu_V0, tmp64);
-                          else
-                            gen_helper_neon_shl_s64(cpu_V0, cpu_V0, tmp64);
+                            if (input_unsigned) {
+                                gen_helper_neon_shl_u64(cpu_V0, cpu_V0,
+                                                        tmp64);
+                            } else {
+                                gen_helper_neon_shl_s64(cpu_V0, cpu_V0,
+                                                        tmp64);
+                            }
                         }
                     } else {
                         tmp = neon_load_reg(rm + pass, 0);
-                        gen_neon_shift_narrow(size, tmp, tmp2, q, u);
+                        gen_neon_shift_narrow(size, tmp, tmp2, q, input_unsigned);
                         tmp3 = neon_load_reg(rm + pass, 1);
-                        gen_neon_shift_narrow(size, tmp3, tmp2, q, u);
+                        gen_neon_shift_narrow(size, tmp3, tmp2, q, input_unsigned);
                         tcg_gen_concat_i32_i64(cpu_V0, tmp, tmp3);
                         dead_tmp(tmp);
                         dead_tmp(tmp3);
                     }
                     tmp = new_tmp();
-                    if (op == 8 && !u) {
-                        gen_neon_narrow(size - 1, tmp, cpu_V0);
+                    if (op == 8) {
+                        if (u) { /* VQSHRUN / VQRSHRUN */
+                            gen_neon_unarrow_sats(size - 1, tmp, cpu_V0);
+                        } else { /* VSHRN / VRSHRN */
+                            gen_neon_narrow(size - 1, tmp, cpu_V0);
+                        }
                     } else {
-                        if (op == 8)
+                        if (u) { /* VQSHRN / VQRSHRN */
+                        gen_neon_narrow_satu(size - 1, tmp, cpu_V0);
+                        } else { /* VQSHRN / VQRSHRN */
                             gen_neon_narrow_sats(size - 1, tmp, cpu_V0);
-                        else
-                            gen_neon_narrow_satu(size - 1, tmp, cpu_V0);
+                        }
                     }
                     neon_store_reg(rd, pass, tmp);
                 } /* for pass */