[PATCH 1/4] target/mips: Fix SAT_S trans helper

[PATCH 1/4] target/mips: Fix SAT_S trans helper

[nihui]

From: Ni Hui <shuizhuyuanluo@126.com>

Fix the SAT_S and SAT_U trans helper confusion.

Signed-off-by: Ni Hui <shuizhuyuanluo@126.com>
---
 target/mips/tcg/msa_translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/mips/tcg/msa_translate.c b/target/mips/tcg/msa_translate.c
index 7576b3ed86..76307102f2 100644
--- a/target/mips/tcg/msa_translate.c
+++ b/target/mips/tcg/msa_translate.c
@@ -399,7 +399,7 @@ TRANS(BSETI,    trans_msa_bit, gen_helper_msa_bseti_df);
 TRANS(BNEGI,    trans_msa_bit, gen_helper_msa_bnegi_df);
 TRANS(BINSLI,   trans_msa_bit, gen_helper_msa_binsli_df);
 TRANS(BINSRI,   trans_msa_bit, gen_helper_msa_binsri_df);
-TRANS(SAT_S,    trans_msa_bit, gen_helper_msa_sat_u_df);
+TRANS(SAT_S,    trans_msa_bit, gen_helper_msa_sat_s_df);
 TRANS(SAT_U,    trans_msa_bit, gen_helper_msa_sat_u_df);
 TRANS(SRARI,    trans_msa_bit, gen_helper_msa_srari_df);
 TRANS(SRLRI,    trans_msa_bit, gen_helper_msa_srlri_df);
-- 
2.25.1

[PATCH 2/4] target/mips: Fix df_extract_val() and df_extract_df() dfe lookup

[nihui]

From: Ni Hui <shuizhuyuanluo@126.com>

Actually look into dfe structure data so that df_extract_val() and
df_extract_df() can return immediate and datafield other than BYTE.

Signed-off-by: Ni Hui <shuizhuyuanluo@126.com>
---
 target/mips/tcg/msa_translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/mips/tcg/msa_translate.c b/target/mips/tcg/msa_translate.c
index 76307102f2..aa45bae0aa 100644
--- a/target/mips/tcg/msa_translate.c
+++ b/target/mips/tcg/msa_translate.c
@@ -68,8 +68,8 @@ struct dfe {
 static int df_extract_val(DisasContext *ctx, int x, const struct dfe *s)
 {
     for (unsigned i = 0; i < 4; i++) {
-        if (extract32(x, s->start, s->length) == s->mask) {
-            return extract32(x, 0, s->start);
+        if (extract32(x, s[i].start, s[i].length) == s[i].mask) {
+            return extract32(x, 0, s[i].start);
         }
     }
     return -1;
@@ -82,7 +82,7 @@ static int df_extract_val(DisasContext *ctx, int x, const struct dfe *s)
 static int df_extract_df(DisasContext *ctx, int x, const struct dfe *s)
 {
     for (unsigned i = 0; i < 4; i++) {
-        if (extract32(x, s->start, s->length) == s->mask) {
+        if (extract32(x, s[i].start, s[i].length) == s[i].mask) {
             return i;
         }
     }
-- 
2.25.1

[PATCH 3/4] target/mips: Fix msa checking condition in trans_msa_elm_fn()

[nihui]

From: Ni Hui <shuizhuyuanluo@126.com>

Fix issue that condition of check_msa_enabled(ctx) is reversed
that causes segfault when msa elm_fn op encountered.

Signed-off-by: Ni Hui <shuizhuyuanluo@126.com>
---
 target/mips/tcg/msa_translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/mips/tcg/msa_translate.c b/target/mips/tcg/msa_translate.c
index aa45bae0aa..92ccc6f921 100644
--- a/target/mips/tcg/msa_translate.c
+++ b/target/mips/tcg/msa_translate.c
@@ -599,7 +599,7 @@ static bool trans_msa_elm_fn(DisasContext *ctx, arg_msa_elm_df *a,
         return false;
     }
 
-    if (check_msa_enabled(ctx)) {
+    if (!check_msa_enabled(ctx)) {
         return true;
     }
 
-- 
2.25.1

[PATCH 4/4] target/mips: Do not treat msa INSERT as NOP when wd is zero

[nihui]

From: Ni Hui <shuizhuyuanluo@126.com>

Only for msa COPY_U/COPY_S with wd zero, we treat it as NOP.

Move this special rule into COPY_U and COPY_S trans function.

Signed-off-by: Ni Hui <shuizhuyuanluo@126.com>
---
 target/mips/tcg/msa_translate.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/target/mips/tcg/msa_translate.c b/target/mips/tcg/msa_translate.c
index 92ccc6f921..0b3dd0957c 100644
--- a/target/mips/tcg/msa_translate.c
+++ b/target/mips/tcg/msa_translate.c
@@ -603,11 +603,6 @@ static bool trans_msa_elm_fn(DisasContext *ctx, arg_msa_elm_df *a,
         return true;
     }
 
-    if (a->wd == 0) {
-        /* Treat as NOP. */
-        return true;
-    }
-
     gen_msa_elm[a->df](cpu_env,
                        tcg_constant_i32(a->wd),
                        tcg_constant_i32(a->ws),
@@ -624,6 +619,11 @@ static bool trans_msa_elm_fn(DisasContext *ctx, arg_msa_elm_df *a,
 
 static bool trans_COPY_U(DisasContext *ctx, arg_msa_elm_df *a)
 {
+    if (a->wd == 0) {
+        /* Treat as NOP. */
+        return true;
+    }
+
     static gen_helper_piii * const gen_msa_copy_u[4] = {
         gen_helper_msa_copy_u_b, gen_helper_msa_copy_u_h,
         NULL_IF_MIPS32(gen_helper_msa_copy_u_w), NULL
@@ -634,6 +634,11 @@ static bool trans_COPY_U(DisasContext *ctx, arg_msa_elm_df *a)
 
 static bool trans_COPY_S(DisasContext *ctx, arg_msa_elm_df *a)
 {
+    if (a->wd == 0) {
+        /* Treat as NOP. */
+        return true;
+    }
+
     static gen_helper_piii * const gen_msa_copy_s[4] = {
         gen_helper_msa_copy_s_b, gen_helper_msa_copy_s_h,
         gen_helper_msa_copy_s_w, NULL_IF_MIPS32(gen_helper_msa_copy_s_d)
-- 
2.25.1

Re: [PATCH 1/4] target/mips: Fix SAT_S trans helper

[Richard Henderson]

On 5/3/22 06:07, nihui wrote:
> From: Ni Hui <shuizhuyuanluo@126.com>
> 
> Fix the SAT_S and SAT_U trans helper confusion.
> 
> Signed-off-by: Ni Hui <shuizhuyuanluo@126.com>
> ---
>   target/mips/tcg/msa_translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/mips/tcg/msa_translate.c b/target/mips/tcg/msa_translate.c
> index 7576b3ed86..76307102f2 100644
> --- a/target/mips/tcg/msa_translate.c
> +++ b/target/mips/tcg/msa_translate.c
> @@ -399,7 +399,7 @@ TRANS(BSETI,    trans_msa_bit, gen_helper_msa_bseti_df);
>   TRANS(BNEGI,    trans_msa_bit, gen_helper_msa_bnegi_df);
>   TRANS(BINSLI,   trans_msa_bit, gen_helper_msa_binsli_df);
>   TRANS(BINSRI,   trans_msa_bit, gen_helper_msa_binsri_df);
> -TRANS(SAT_S,    trans_msa_bit, gen_helper_msa_sat_u_df);
> +TRANS(SAT_S,    trans_msa_bit, gen_helper_msa_sat_s_df);
>   TRANS(SAT_U,    trans_msa_bit, gen_helper_msa_sat_u_df);
>   TRANS(SRARI,    trans_msa_bit, gen_helper_msa_srari_df);
>   TRANS(SRLRI,    trans_msa_bit, gen_helper_msa_srlri_df);

Cc: phil
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 2/4] target/mips: Fix df_extract_val() and df_extract_df() dfe lookup

[Richard Henderson]

On 5/3/22 06:07, nihui wrote:
> From: Ni Hui <shuizhuyuanluo@126.com>
> 
> Actually look into dfe structure data so that df_extract_val() and
> df_extract_df() can return immediate and datafield other than BYTE.
> 
> Signed-off-by: Ni Hui <shuizhuyuanluo@126.com>
> ---
>   target/mips/tcg/msa_translate.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/mips/tcg/msa_translate.c b/target/mips/tcg/msa_translate.c
> index 76307102f2..aa45bae0aa 100644
> --- a/target/mips/tcg/msa_translate.c
> +++ b/target/mips/tcg/msa_translate.c
> @@ -68,8 +68,8 @@ struct dfe {
>   static int df_extract_val(DisasContext *ctx, int x, const struct dfe *s)
>   {
>       for (unsigned i = 0; i < 4; i++) {
> -        if (extract32(x, s->start, s->length) == s->mask) {
> -            return extract32(x, 0, s->start);
> +        if (extract32(x, s[i].start, s[i].length) == s[i].mask) {
> +            return extract32(x, 0, s[i].start);
>           }
>       }
>       return -1;
> @@ -82,7 +82,7 @@ static int df_extract_val(DisasContext *ctx, int x, const struct dfe *s)
>   static int df_extract_df(DisasContext *ctx, int x, const struct dfe *s)
>   {
>       for (unsigned i = 0; i < 4; i++) {
> -        if (extract32(x, s->start, s->length) == s->mask) {
> +        if (extract32(x, s[i].start, s[i].length) == s[i].mask) {
>               return i;
>           }
>       }

Cc: phil
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 3/4] target/mips: Fix msa checking condition in trans_msa_elm_fn()

[Richard Henderson]

On 5/3/22 06:07, nihui wrote:
> From: Ni Hui <shuizhuyuanluo@126.com>
> 
> Fix issue that condition of check_msa_enabled(ctx) is reversed
> that causes segfault when msa elm_fn op encountered.
> 
> Signed-off-by: Ni Hui <shuizhuyuanluo@126.com>
> ---
>   target/mips/tcg/msa_translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/mips/tcg/msa_translate.c b/target/mips/tcg/msa_translate.c
> index aa45bae0aa..92ccc6f921 100644
> --- a/target/mips/tcg/msa_translate.c
> +++ b/target/mips/tcg/msa_translate.c
> @@ -599,7 +599,7 @@ static bool trans_msa_elm_fn(DisasContext *ctx, arg_msa_elm_df *a,
>           return false;
>       }
>   
> -    if (check_msa_enabled(ctx)) {
> +    if (!check_msa_enabled(ctx)) {
>           return true;
>       }
>   

Cc: phil
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 1/4] target/mips: Fix SAT_S trans helper

[Philippe Mathieu-Daudé via]

On 3/5/22 17:11, Richard Henderson wrote:
> On 5/3/22 06:07, nihui wrote:
>> From: Ni Hui <shuizhuyuanluo@126.com>
>>
>> Fix the SAT_S and SAT_U trans helper confusion.
>>
>> Signed-off-by: Ni Hui <shuizhuyuanluo@126.com>
>> ---
>>   target/mips/tcg/msa_translate.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/mips/tcg/msa_translate.c 
>> b/target/mips/tcg/msa_translate.c
>> index 7576b3ed86..76307102f2 100644
>> --- a/target/mips/tcg/msa_translate.c
>> +++ b/target/mips/tcg/msa_translate.c
>> @@ -399,7 +399,7 @@ TRANS(BSETI,    trans_msa_bit, 
>> gen_helper_msa_bseti_df);
>>   TRANS(BNEGI,    trans_msa_bit, gen_helper_msa_bnegi_df);
>>   TRANS(BINSLI,   trans_msa_bit, gen_helper_msa_binsli_df);
>>   TRANS(BINSRI,   trans_msa_bit, gen_helper_msa_binsri_df);
>> -TRANS(SAT_S,    trans_msa_bit, gen_helper_msa_sat_u_df);
>> +TRANS(SAT_S,    trans_msa_bit, gen_helper_msa_sat_s_df);
>>   TRANS(SAT_U,    trans_msa_bit, gen_helper_msa_sat_u_df);
>>   TRANS(SRARI,    trans_msa_bit, gen_helper_msa_srari_df);
>>   TRANS(SRLRI,    trans_msa_bit, gen_helper_msa_srlri_df);
> 
> Cc: phil
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Oops my bad...

Fixes: 4701d23aef ("target/mips: Convert MSA BIT instruction format to 
decodetree")

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

And queued to mips-next.

Re: [PATCH 2/4] target/mips: Fix df_extract_val() and df_extract_df() dfe lookup

[Philippe Mathieu-Daudé via]

On 3/5/22 17:12, Richard Henderson wrote:
> On 5/3/22 06:07, nihui wrote:
>> From: Ni Hui <shuizhuyuanluo@126.com>
>>
>> Actually look into dfe structure data so that df_extract_val() and
>> df_extract_df() can return immediate and datafield other than BYTE.

Doh my bad again.

Fixes: 4701d23aef ("target/mips: Convert MSA BIT instruction format to 
decodetree")

>> Signed-off-by: Ni Hui <shuizhuyuanluo@126.com>
>> ---
>>   target/mips/tcg/msa_translate.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/mips/tcg/msa_translate.c 
>> b/target/mips/tcg/msa_translate.c
>> index 76307102f2..aa45bae0aa 100644
>> --- a/target/mips/tcg/msa_translate.c
>> +++ b/target/mips/tcg/msa_translate.c
>> @@ -68,8 +68,8 @@ struct dfe {
>>   static int df_extract_val(DisasContext *ctx, int x, const struct dfe 
>> *s)
>>   {
>>       for (unsigned i = 0; i < 4; i++) {
>> -        if (extract32(x, s->start, s->length) == s->mask) {
>> -            return extract32(x, 0, s->start);
>> +        if (extract32(x, s[i].start, s[i].length) == s[i].mask) {
>> +            return extract32(x, 0, s[i].start);
>>           }
>>       }
>>       return -1;
>> @@ -82,7 +82,7 @@ static int df_extract_val(DisasContext *ctx, int x, 
>> const struct dfe *s)
>>   static int df_extract_df(DisasContext *ctx, int x, const struct dfe *s)
>>   {
>>       for (unsigned i = 0; i < 4; i++) {
>> -        if (extract32(x, s->start, s->length) == s->mask) {
>> +        if (extract32(x, s[i].start, s[i].length) == s[i].mask) {
>>               return i;
>>           }
>>       }
> 
> Cc: phil
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

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

Queued to mips-next tree.

Re: [PATCH 3/4] target/mips: Fix msa checking condition in trans_msa_elm_fn()

[Philippe Mathieu-Daudé via]

On 3/5/22 17:12, Richard Henderson wrote:
> On 5/3/22 06:07, nihui wrote:
>> From: Ni Hui <shuizhuyuanluo@126.com>
>>
>> Fix issue that condition of check_msa_enabled(ctx) is reversed
>> that causes segfault when msa elm_fn op encountered.
>>
>> Signed-off-by: Ni Hui <shuizhuyuanluo@126.com>
>> ---
>>   target/mips/tcg/msa_translate.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/mips/tcg/msa_translate.c 
>> b/target/mips/tcg/msa_translate.c
>> index aa45bae0aa..92ccc6f921 100644
>> --- a/target/mips/tcg/msa_translate.c
>> +++ b/target/mips/tcg/msa_translate.c
>> @@ -599,7 +599,7 @@ static bool trans_msa_elm_fn(DisasContext *ctx, 
>> arg_msa_elm_df *a,
>>           return false;
>>       }
>> -    if (check_msa_enabled(ctx)) {
>> +    if (!check_msa_enabled(ctx)) {
>>           return true;
>>       }
> 
> Cc: phil
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Clearly this path is not exercised in TCG tests.

Ni, could you contribute tests for the ELM MSA opcodes?

Fixes: 2f2745c81a ("target/mips: Convert MSA COPY_U opcode to decodetree")
Fixes: 97fe675519 ("target/mips: Convert MSA COPY_S and INSERT opcodes 
to decodetree")

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

Queued.

Re: [PATCH 4/4] target/mips: Do not treat msa INSERT as NOP when wd is zero

[Philippe Mathieu-Daudé via]

On 3/5/22 15:07, nihui wrote:
> From: Ni Hui <shuizhuyuanluo@126.com>
> 
> Only for msa COPY_U/COPY_S with wd zero, we treat it as NOP.
> 
> Move this special rule into COPY_U and COPY_S trans function.

Fixes: 97fe675519 ("target/mips: Convert MSA COPY_S and INSERT opcodes 
to decodetree")

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

> Signed-off-by: Ni Hui <shuizhuyuanluo@126.com>
> ---
>   target/mips/tcg/msa_translate.c | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/target/mips/tcg/msa_translate.c b/target/mips/tcg/msa_translate.c
> index 92ccc6f921..0b3dd0957c 100644
> --- a/target/mips/tcg/msa_translate.c
> +++ b/target/mips/tcg/msa_translate.c
> @@ -603,11 +603,6 @@ static bool trans_msa_elm_fn(DisasContext *ctx, arg_msa_elm_df *a,
>           return true;
>       }
>   
> -    if (a->wd == 0) {
> -        /* Treat as NOP. */
> -        return true;
> -    }
> -
>       gen_msa_elm[a->df](cpu_env,
>                          tcg_constant_i32(a->wd),
>                          tcg_constant_i32(a->ws),
> @@ -624,6 +619,11 @@ static bool trans_msa_elm_fn(DisasContext *ctx, arg_msa_elm_df *a,
>   
>   static bool trans_COPY_U(DisasContext *ctx, arg_msa_elm_df *a)
>   {
> +    if (a->wd == 0) {
> +        /* Treat as NOP. */
> +        return true;
> +    }
> +
>       static gen_helper_piii * const gen_msa_copy_u[4] = {
>           gen_helper_msa_copy_u_b, gen_helper_msa_copy_u_h,
>           NULL_IF_MIPS32(gen_helper_msa_copy_u_w), NULL
> @@ -634,6 +634,11 @@ static bool trans_COPY_U(DisasContext *ctx, arg_msa_elm_df *a)
>   
>   static bool trans_COPY_S(DisasContext *ctx, arg_msa_elm_df *a)
>   {
> +    if (a->wd == 0) {
> +        /* Treat as NOP. */
> +        return true;
> +    }
> +
>       static gen_helper_piii * const gen_msa_copy_s[4] = {
>           gen_helper_msa_copy_s_b, gen_helper_msa_copy_s_h,
>           gen_helper_msa_copy_s_w, NULL_IF_MIPS32(gen_helper_msa_copy_s_d)