diff mbox series

[v2,06/11] target/arm: Decode aa32 armv8.1 two reg and a scalar

Message ID 20171218172425.18200-7-richard.henderson@linaro.org
State New
Headers show
Series ARM v8.1 simd + v8.3 complex insns | expand

Commit Message

Richard Henderson Dec. 18, 2017, 5:24 p.m. UTC 
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 target/arm/translate.c | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

-- 
2.14.3

Comments

Peter Maydell Jan. 15, 2018, 5:47 p.m. UTC | #1 
On 18 December 2017 at 17:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  target/arm/translate.c | 38 +++++++++++++++++++++++++++++++++++---

>  1 file changed, 35 insertions(+), 3 deletions(-)

>

> diff --git a/target/arm/translate.c b/target/arm/translate.c

> index a9587ae242..1a0b0eaced 100644

> --- a/target/arm/translate.c

> +++ b/target/arm/translate.c

> @@ -6973,11 +6973,43 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn)

>                          }

>                          neon_store_reg64(cpu_V0, rd + pass);

>                      }

> +                    break;

> +                case 14: /* VQRDMLAH scalar */

> +                case 15: /* VQRDMLSH scalar */

> +                    if (!arm_dc_feature(s, ARM_FEATURE_V8_1_SIMD)) {

> +                        return 1;

> +                    }

> +                    if (u && ((rd | rn) & 1)) {

> +                        return 1;

> +                    }


The pseudocode also has UNDEF if Q==1 && Vm<0> == 1 -- have we
already done that test earlier? I can't see it, but our neon decode
code is quite hard to read...

> +                    tmp2 = neon_get_scalar(size, rm);

> +                    for (pass = 0; pass < (u ? 4 : 2); pass++) {

> +                        void (*fn)(TCGv_i32, TCGv_env, TCGv_i32,

> +                                   TCGv_i32, TCGv_i32);


Can we define a typedef for this, please ?

>

> -

> +                        tmp = neon_load_reg(rn, pass);

> +                        tmp3 = neon_load_reg(rd, pass);

> +                        if (op == 14) {

> +                            if (size == 1) {

> +                                fn = gen_helper_neon_qrdmlah_s16;

> +                            } else {

> +                                fn = gen_helper_neon_qrdmlah_s32;

> +                            }

> +                        } else {

> +                            if (size == 1) {

> +                                fn = gen_helper_neon_qrdmlsh_s16;

> +                            } else {

> +                                fn = gen_helper_neon_qrdmlsh_s32;

> +                            }

> +                        }

> +                        fn(tmp, cpu_env, tmp, tmp2, tmp3);

> +                        tcg_temp_free_i32(tmp3);

> +                        neon_store_reg(rd, pass, tmp);

> +                    }

> +                    tcg_temp_free_i32(tmp2);

>                      break;

> -                default: /* 14 and 15 are RESERVED */

> -                    return 1;

> +                default:

> +                    g_assert_not_reached();

>                  }

>              }

>          } else { /* size == 3 */

> --

> 2.14.3


Otherwise

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>


thanks
-- PMM
Richard Henderson Jan. 26, 2018, 7:18 a.m. UTC | #2 
On 01/15/2018 09:47 AM, Peter Maydell wrote:
> On 18 December 2017 at 17:24, Richard Henderson

> <richard.henderson@linaro.org> wrote:

>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

>> ---

>>  target/arm/translate.c | 38 +++++++++++++++++++++++++++++++++++---

>>  1 file changed, 35 insertions(+), 3 deletions(-)

>>

>> diff --git a/target/arm/translate.c b/target/arm/translate.c

>> index a9587ae242..1a0b0eaced 100644

>> --- a/target/arm/translate.c

>> +++ b/target/arm/translate.c

>> @@ -6973,11 +6973,43 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn)

>>                          }

>>                          neon_store_reg64(cpu_V0, rd + pass);

>>                      }

>> +                    break;

>> +                case 14: /* VQRDMLAH scalar */

>> +                case 15: /* VQRDMLSH scalar */

>> +                    if (!arm_dc_feature(s, ARM_FEATURE_V8_1_SIMD)) {

>> +                        return 1;

>> +                    }

>> +                    if (u && ((rd | rn) & 1)) {

>> +                        return 1;

>> +                    }

> 

> The pseudocode also has UNDEF if Q==1 && Vm<0> == 1 ....


Not for the indexed version, encoding A2.

> 

>> +                    tmp2 = neon_get_scalar(size, rm);

>> +                    for (pass = 0; pass < (u ? 4 : 2); pass++) {

>> +                        void (*fn)(TCGv_i32, TCGv_env, TCGv_i32,

>> +                                   TCGv_i32, TCGv_i32);

> 

> Can we define a typedef for this, please ?


What would you name it?


r~
Peter Maydell Jan. 26, 2018, 10:05 a.m. UTC | #3 
On 26 January 2018 at 07:18, Richard Henderson
<richard.henderson@linaro.org> wrote:
> On 01/15/2018 09:47 AM, Peter Maydell wrote:

>> On 18 December 2017 at 17:24, Richard Henderson

>> <richard.henderson@linaro.org> wrote:

>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

>>> ---

>>>  target/arm/translate.c | 38 +++++++++++++++++++++++++++++++++++---

>>>  1 file changed, 35 insertions(+), 3 deletions(-)

>>>

>>> diff --git a/target/arm/translate.c b/target/arm/translate.c

>>> index a9587ae242..1a0b0eaced 100644

>>> --- a/target/arm/translate.c

>>> +++ b/target/arm/translate.c

>>> @@ -6973,11 +6973,43 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn)

>>>                          }

>>>                          neon_store_reg64(cpu_V0, rd + pass);

>>>                      }

>>> +                    break;

>>> +                case 14: /* VQRDMLAH scalar */

>>> +                case 15: /* VQRDMLSH scalar */

>>> +                    if (!arm_dc_feature(s, ARM_FEATURE_V8_1_SIMD)) {

>>> +                        return 1;

>>> +                    }

>>> +                    if (u && ((rd | rn) & 1)) {

>>> +                        return 1;

>>> +                    }

>>

>> The pseudocode also has UNDEF if Q==1 && Vm<0> == 1 ....

>

> Not for the indexed version, encoding A2.


Ah, yes.

>>

>>> +                    tmp2 = neon_get_scalar(size, rm);

>>> +                    for (pass = 0; pass < (u ? 4 : 2); pass++) {

>>> +                        void (*fn)(TCGv_i32, TCGv_env, TCGv_i32,

>>> +                                   TCGv_i32, TCGv_i32);

>>

>> Can we define a typedef for this, please ?

>

> What would you name it?


NeonGenThreeOpEnvFn would fit the naming scheme we've got
in translate-a64.c.

thanks
-- PMM
Philippe Mathieu-Daudé Jan. 26, 2018, 1:41 p.m. UTC | #4 
On 12/18/2017 02:24 PM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


> ---

>  target/arm/translate.c | 38 +++++++++++++++++++++++++++++++++++---

>  1 file changed, 35 insertions(+), 3 deletions(-)

> 

> diff --git a/target/arm/translate.c b/target/arm/translate.c

> index a9587ae242..1a0b0eaced 100644

> --- a/target/arm/translate.c

> +++ b/target/arm/translate.c

> @@ -6973,11 +6973,43 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn)

>                          }

>                          neon_store_reg64(cpu_V0, rd + pass);

>                      }

> +                    break;

> +                case 14: /* VQRDMLAH scalar */

> +                case 15: /* VQRDMLSH scalar */

> +                    if (!arm_dc_feature(s, ARM_FEATURE_V8_1_SIMD)) {

> +                        return 1;

> +                    }

> +                    if (u && ((rd | rn) & 1)) {

> +                        return 1;

> +                    }

> +                    tmp2 = neon_get_scalar(size, rm);

> +                    for (pass = 0; pass < (u ? 4 : 2); pass++) {

> +                        void (*fn)(TCGv_i32, TCGv_env, TCGv_i32,

> +                                   TCGv_i32, TCGv_i32);

>  

> -

> +                        tmp = neon_load_reg(rn, pass);

> +                        tmp3 = neon_load_reg(rd, pass);

> +                        if (op == 14) {

> +                            if (size == 1) {

> +                                fn = gen_helper_neon_qrdmlah_s16;

> +                            } else {

> +                                fn = gen_helper_neon_qrdmlah_s32;

> +                            }

> +                        } else {

> +                            if (size == 1) {

> +                                fn = gen_helper_neon_qrdmlsh_s16;

> +                            } else {

> +                                fn = gen_helper_neon_qrdmlsh_s32;

> +                            }

> +                        }

> +                        fn(tmp, cpu_env, tmp, tmp2, tmp3);

> +                        tcg_temp_free_i32(tmp3);

> +                        neon_store_reg(rd, pass, tmp);

> +                    }

> +                    tcg_temp_free_i32(tmp2);

>                      break;

> -                default: /* 14 and 15 are RESERVED */

> -                    return 1;

> +                default:

> +                    g_assert_not_reached();

>                  }

>              }

>          } else { /* size == 3 */

>
diff mbox series

Patch

diff --git a/target/arm/translate.c b/target/arm/translate.c
index a9587ae242..1a0b0eaced 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -6973,11 +6973,43 @@  static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
                         }
                         neon_store_reg64(cpu_V0, rd + pass);
                     }
+                    break;
+                case 14: /* VQRDMLAH scalar */
+                case 15: /* VQRDMLSH scalar */
+                    if (!arm_dc_feature(s, ARM_FEATURE_V8_1_SIMD)) {
+                        return 1;
+                    }
+                    if (u && ((rd | rn) & 1)) {
+                        return 1;
+                    }
+                    tmp2 = neon_get_scalar(size, rm);
+                    for (pass = 0; pass < (u ? 4 : 2); pass++) {
+                        void (*fn)(TCGv_i32, TCGv_env, TCGv_i32,
+                                   TCGv_i32, TCGv_i32);
 
-
+                        tmp = neon_load_reg(rn, pass);
+                        tmp3 = neon_load_reg(rd, pass);
+                        if (op == 14) {
+                            if (size == 1) {
+                                fn = gen_helper_neon_qrdmlah_s16;
+                            } else {
+                                fn = gen_helper_neon_qrdmlah_s32;
+                            }
+                        } else {
+                            if (size == 1) {
+                                fn = gen_helper_neon_qrdmlsh_s16;
+                            } else {
+                                fn = gen_helper_neon_qrdmlsh_s32;
+                            }
+                        }
+                        fn(tmp, cpu_env, tmp, tmp2, tmp3);
+                        tcg_temp_free_i32(tmp3);
+                        neon_store_reg(rd, pass, tmp);
+                    }
+                    tcg_temp_free_i32(tmp2);
                     break;
-                default: /* 14 and 15 are RESERVED */
-                    return 1;
+                default:
+                    g_assert_not_reached();
                 }
             }
         } else { /* size == 3 */