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
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>
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
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).
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~
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.
© 2016 - 2024 Red Hat, Inc.