[Qemu-devel] [PATCH] target/s390x: Finish implementing RISBGN

Richard Henderson posted 1 patch 6 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171107145546.767-1-richard.henderson@linaro.org
Test checkpatch passed
Test docker passed
Test ppc passed
Test s390x passed
target/s390x/translate.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
[Qemu-devel] [PATCH] target/s390x: Finish implementing RISBGN
Posted by Richard Henderson 6 years, 4 months ago
We added the entry to insn-data.def, but failed to update op_risbg
to match.  No need to special-case the imask inversion, since that
is already ~0 for RISBG (and now RISBGN).

Fixes: 375ee58bedcda359011fe7fa99e0647f66f9ffa0
Fixes: https://bugs.launchpad.net/qemu/+bug/1701798 (s390x part)
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/translate.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index dee72a787d..85d0a6c3af 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3432,6 +3432,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
     /* Adjust the arguments for the specific insn.  */
     switch (s->fields->op2) {
     case 0x55: /* risbg */
+    case 0x59: /* risbgn */
         i3 &= 63;
         i4 &= 63;
         pmask = ~0;
@@ -3447,7 +3448,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
         pmask = 0x00000000ffffffffull;
         break;
     default:
-        abort();
+        g_assert_not_reached();
     }
 
     /* MASK is the set of bits to be inserted from R2.
@@ -3464,11 +3465,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
        insns, we need to keep the other half of the register.  */
     imask = ~mask | ~pmask;
     if (do_zero) {
-        if (s->fields->op2 == 0x55) {
-            imask = 0;
-        } else {
-            imask = ~pmask;
-        }
+        imask = ~pmask;
     }
 
     len = i4 - i3 + 1;
-- 
2.13.6


Re: [Qemu-devel] [qemu-s390x] [PATCH] target/s390x: Finish implementing RISBGN
Posted by Thomas Huth 6 years, 4 months ago
On 07.11.2017 15:55, Richard Henderson wrote:
> We added the entry to insn-data.def, but failed to update op_risbg
> to match.  No need to special-case the imask inversion, since that
> is already ~0 for RISBG (and now RISBGN).
> 
> Fixes: 375ee58bedcda359011fe7fa99e0647f66f9ffa0
> Fixes: https://bugs.launchpad.net/qemu/+bug/1701798 (s390x part)
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/translate.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index dee72a787d..85d0a6c3af 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -3432,6 +3432,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>      /* Adjust the arguments for the specific insn.  */
>      switch (s->fields->op2) {
>      case 0x55: /* risbg */
> +    case 0x59: /* risbgn */
>          i3 &= 63;
>          i4 &= 63;
>          pmask = ~0;
> @@ -3447,7 +3448,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>          pmask = 0x00000000ffffffffull;
>          break;
>      default:
> -        abort();
> +        g_assert_not_reached();
>      }
>  
>      /* MASK is the set of bits to be inserted from R2.
> @@ -3464,11 +3465,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>         insns, we need to keep the other half of the register.  */
>      imask = ~mask | ~pmask;
>      if (do_zero) {
> -        if (s->fields->op2 == 0x55) {
> -            imask = 0;
> -        } else {
> -            imask = ~pmask;
> -        }
> +        imask = ~pmask;
>      }
>  
>      len = i4 - i3 + 1;
> 

Looks good.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [qemu-s390x] [PATCH] target/s390x: Finish implementing RISBGN
Posted by Peter Maydell 6 years, 4 months ago
On 7 November 2017 at 16:00, Thomas Huth <thuth@redhat.com> wrote:
> On 07.11.2017 15:55, Richard Henderson wrote:
>> We added the entry to insn-data.def, but failed to update op_risbg
>> to match.  No need to special-case the imask inversion, since that
>> is already ~0 for RISBG (and now RISBGN).
>>
>> Fixes: 375ee58bedcda359011fe7fa99e0647f66f9ffa0
>> Fixes: https://bugs.launchpad.net/qemu/+bug/1701798 (s390x part)
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  target/s390x/translate.c | 9 +++------
>>  1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
>> index dee72a787d..85d0a6c3af 100644
>> --- a/target/s390x/translate.c
>> +++ b/target/s390x/translate.c
>> @@ -3432,6 +3432,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>>      /* Adjust the arguments for the specific insn.  */
>>      switch (s->fields->op2) {
>>      case 0x55: /* risbg */
>> +    case 0x59: /* risbgn */
>>          i3 &= 63;
>>          i4 &= 63;
>>          pmask = ~0;
>> @@ -3447,7 +3448,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>>          pmask = 0x00000000ffffffffull;
>>          break;
>>      default:
>> -        abort();
>> +        g_assert_not_reached();
>>      }
>>
>>      /* MASK is the set of bits to be inserted from R2.
>> @@ -3464,11 +3465,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>>         insns, we need to keep the other half of the register.  */
>>      imask = ~mask | ~pmask;
>>      if (do_zero) {
>> -        if (s->fields->op2 == 0x55) {
>> -            imask = 0;
>> -        } else {
>> -            imask = ~pmask;
>> -        }
>> +        imask = ~pmask;
>>      }
>>
>>      len = i4 - i3 + 1;
>>
>
> Looks good.
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

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

...at least it fixes the simple test case from the bug report.

-- PMM

Re: [Qemu-devel] [qemu-s390x] [PATCH] target/s390x: Finish implementing RISBGN
Posted by Cornelia Huck 6 years, 4 months ago
On Tue,  7 Nov 2017 15:55:46 +0100
Richard Henderson <richard.henderson@linaro.org> wrote:

> We added the entry to insn-data.def, but failed to update op_risbg
> to match.  No need to special-case the imask inversion, since that
> is already ~0 for RISBG (and now RISBGN).
> 
> Fixes: 375ee58bedcda359011fe7fa99e0647f66f9ffa0
> Fixes: https://bugs.launchpad.net/qemu/+bug/1701798 (s390x part)
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/translate.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index dee72a787d..85d0a6c3af 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -3432,6 +3432,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>      /* Adjust the arguments for the specific insn.  */
>      switch (s->fields->op2) {
>      case 0x55: /* risbg */
> +    case 0x59: /* risbgn */
>          i3 &= 63;
>          i4 &= 63;
>          pmask = ~0;
> @@ -3447,7 +3448,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>          pmask = 0x00000000ffffffffull;
>          break;
>      default:
> -        abort();
> +        g_assert_not_reached();
>      }
>  
>      /* MASK is the set of bits to be inserted from R2.
> @@ -3464,11 +3465,7 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>         insns, we need to keep the other half of the register.  */
>      imask = ~mask | ~pmask;
>      if (do_zero) {
> -        if (s->fields->op2 == 0x55) {
> -            imask = 0;
> -        } else {
> -            imask = ~pmask;
> -        }
> +        imask = ~pmask;
>      }
>  
>      len = i4 - i3 + 1;

I can queue this to s390-fixes (unless there are other takers).

Re: [Qemu-devel] [qemu-s390x] [PATCH] target/s390x: Finish implementing RISBGN
Posted by Richard Henderson 6 years, 4 months ago
On 11/08/2017 12:50 PM, Cornelia Huck wrote:
> I can queue this to s390-fixes (unless there are other takers).

Please do.  Thanks,


r~

Re: [Qemu-devel] [qemu-s390x] [PATCH] target/s390x: Finish implementing RISBGN
Posted by Cornelia Huck 6 years, 4 months ago
On Tue,  7 Nov 2017 15:55:46 +0100
Richard Henderson <richard.henderson@linaro.org> wrote:

> We added the entry to insn-data.def, but failed to update op_risbg
> to match.  No need to special-case the imask inversion, since that
> is already ~0 for RISBG (and now RISBGN).
> 
> Fixes: 375ee58bedcda359011fe7fa99e0647f66f9ffa0
> Fixes: https://bugs.launchpad.net/qemu/+bug/1701798 (s390x part)
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/s390x/translate.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)

Thanks, applied to s390-fixes.