[Qemu-devel] [PATCH v2 06/11] target/arm: Decode aa32 armv8.1 two reg and a scalar

Richard Henderson posted 11 patches 7 years, 10 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v2 06/11] target/arm: Decode aa32 armv8.1 two reg and a scalar
Posted by Richard Henderson 7 years, 10 months ago
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;
+                    }
+                    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 */
-- 
2.14.3


Re: [Qemu-devel] [PATCH v2 06/11] target/arm: Decode aa32 armv8.1 two reg and a scalar
Posted by Peter Maydell 7 years, 9 months ago
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

Re: [Qemu-devel] [PATCH v2 06/11] target/arm: Decode aa32 armv8.1 two reg and a scalar
Posted by Richard Henderson 7 years, 9 months ago
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~

Re: [Qemu-devel] [PATCH v2 06/11] target/arm: Decode aa32 armv8.1 two reg and a scalar
Posted by Peter Maydell 7 years, 9 months ago
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

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 06/11] target/arm: Decode aa32 armv8.1 two reg and a scalar
Posted by Philippe Mathieu-Daudé 7 years, 9 months ago
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 */
>