| Message ID | 20171107145546.767-1-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/s390x: Finish implementing RISBGN | expand |
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.
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;
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(-) -- 2.13.6