| Message ID | 1354663750-2163-1-git-send-email-petar.jovanovic@rt-rk.com |
|---|---|
| State | New |
| Headers | show |
On 2012-12-04 17:29, Petar Jovanovic wrote:
> rs5_0 = (int8_t)(rs5_0 << 2) >> 2;
This is more portably written as
rs5_0 = (rs5_0 ^ 0x20) - 0x20;
r~
On 5 December 2012 15:36, Richard Henderson <rth@twiddle.net> wrote: > On 2012-12-04 17:29, Petar Jovanovic wrote: >> rs5_0 = (int8_t)(rs5_0 << 2) >> 2; > > This is more portably written as > > rs5_0 = (rs5_0 ^ 0x20) - 0x20; ...but way more obscurely. If we want to play that kind of game can we have a sign-extension function in a header somewhere? -- PMM
On 2012-12-05 09:49, Peter Maydell wrote: > On 5 December 2012 15:36, Richard Henderson <rth@twiddle.net> wrote: >> On 2012-12-04 17:29, Petar Jovanovic wrote: >>> rs5_0 = (int8_t)(rs5_0 << 2) >> 2; >> >> This is more portably written as >> >> rs5_0 = (rs5_0 ^ 0x20) - 0x20; > > ...but way more obscurely. If we want to play that > kind of game can we have a sign-extension function in > a header somewhere? I dunno about more obscurely. It took me a minute to figure out what was wanted in the original. As for a helper function... sure. r~
On 5 December 2012 15:51, Richard Henderson <rth@twiddle.net> wrote: > On 2012-12-05 09:49, Peter Maydell wrote: >> On 5 December 2012 15:36, Richard Henderson <rth@twiddle.net> wrote: >>> On 2012-12-04 17:29, Petar Jovanovic wrote: >>>> rs5_0 = (int8_t)(rs5_0 << 2) >> 2; >>> >>> This is more portably written as >>> >>> rs5_0 = (rs5_0 ^ 0x20) - 0x20; >> >> ...but way more obscurely. If we want to play that >> kind of game can we have a sign-extension function in >> a header somewhere? > > I dunno about more obscurely. It took me a minute to figure out > what was wanted in the original. > > As for a helper function... sure. I don't think we should block this patch on that general cleanup, though. All the sign extensions in target-mips/translate.c are done in the double-shift way, so this is consistent with the existing code. -- PMM
> -----Original Message----- > From: qemu-devel-bounces+ericj=mips.com@nongnu.org [mailto:qemu-devel- > bounces+ericj=mips.com@nongnu.org] On Behalf Of Petar Jovanovic > Sent: Tuesday, December 04, 2012 3:29 PM > To: qemu-devel@nongnu.org > Cc: blauwirbel@gmail.com; Jovanovic, Petar; rth7680@gmail.com; > afaerber@suse.de; aurelien@aurel32.net > Subject: [Qemu-devel] [PATCH v2] target-mips: Fix incorrect shift for > SHILO and SHILOV > > From: Petar Jovanovic <petarj@mips.com> > > helper_shilo has not been shifting an accumulator value correctly for > negative > values in 'shift' field. Minor optimization for shift=0 case. > This change also adds tests that will trigger issue and check for > regressions. > > Signed-off-by: Petar Jovanovic <petarj@mips.com> > --- > target-mips/dsp_helper.c | 17 +++++++++-------- > tests/tcg/mips/mips32-dsp/shilo.c | 18 ++++++++++++++++++ > tests/tcg/mips/mips32-dsp/shilov.c | 20 ++++++++++++++++++++ > 3 files changed, 47 insertions(+), 8 deletions(-) > > diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c > index e7949c2..44f6dc7 100644 > --- a/target-mips/dsp_helper.c > +++ b/target-mips/dsp_helper.c > @@ -3814,17 +3814,18 @@ void helper_shilo(target_ulong ac, target_ulong > rs, CPUMIPSState *env) > > rs5_0 = rs & 0x3F; > rs5_0 = (int8_t)(rs5_0 << 2) >> 2; > - rs5_0 = MIPSDSP_ABS(rs5_0); > + > + if (unlikely(rs5_0 == 0)) { > + return; > + } > + > acc = (((uint64_t)env->active_tc.HI[ac] << 32) & MIPSDSP_LHI) | > ((uint64_t)env->active_tc.LO[ac] & MIPSDSP_LLO); > - if (rs5_0 == 0) { > - temp = acc; > + > + if (rs5_0 > 0) { > + temp = acc >> rs5_0; > } else { > - if (rs5_0 > 0) { > - temp = acc >> rs5_0; > - } else { > - temp = acc << rs5_0; > - } > + temp = acc << -rs5_0; > } > > env->active_tc.HI[ac] = (target_ulong)(int32_t)((temp & MIPSDSP_LHI) > >> 32); > diff --git a/tests/tcg/mips/mips32-dsp/shilo.c b/tests/tcg/mips/mips32- > dsp/shilo.c > index b686616..ce8ebc6 100644 > --- a/tests/tcg/mips/mips32-dsp/shilo.c > +++ b/tests/tcg/mips/mips32-dsp/shilo.c > @@ -23,5 +23,23 @@ int main() > assert(ach == resulth); > assert(acl == resultl); > > + > + ach = 0x1; > + acl = 0x80000000; > + > + resulth = 0x3; > + resultl = 0x0; > + > + __asm > + ("mthi %0, $ac1\n\t" > + "mtlo %1, $ac1\n\t" > + "shilo $ac1, -1\n\t" > + "mfhi %0, $ac1\n\t" > + "mflo %1, $ac1\n\t" > + : "+r"(ach), "+r"(acl) > + ); > + assert(ach == resulth); > + assert(acl == resultl); > + > return 0; > } > diff --git a/tests/tcg/mips/mips32-dsp/shilov.c b/tests/tcg/mips/mips32- > dsp/shilov.c > index f186032..e1d6cea 100644 > --- a/tests/tcg/mips/mips32-dsp/shilov.c > +++ b/tests/tcg/mips/mips32-dsp/shilov.c > @@ -25,5 +25,25 @@ int main() > assert(ach == resulth); > assert(acl == resultl); > > + > + rs = 0xffffffff; > + ach = 0x1; > + acl = 0x80000000; > + > + resulth = 0x3; > + resultl = 0x0; > + > + __asm > + ("mthi %0, $ac1\n\t" > + "mtlo %1, $ac1\n\t" > + "shilov $ac1, %2\n\t" > + "mfhi %0, $ac1\n\t" > + "mflo %1, $ac1\n\t" > + : "+r"(ach), "+r"(acl) > + : "r"(rs) > + ); > + assert(ach == resulth); > + assert(acl == resultl); > + > return 0; > } > -- > 1.7.5.4 > Reviewed-by: Eric Johnson <ericj@mips.com>
Oops, I forgot. The contents are OK but 'git am' didn't like the patch. patch had to use fuzz. This may need to be rebased to latest master. -Eric > -----Original Message----- > From: qemu-devel-bounces+ericj=mips.com@nongnu.org [mailto:qemu-devel- > bounces+ericj=mips.com@nongnu.org] On Behalf Of Johnson, Eric > Sent: Wednesday, December 05, 2012 12:42 PM > To: Jovanovic, Petar; qemu-devel@nongnu.org > Cc: blauwirbel@gmail.com; rth7680@gmail.com; afaerber@suse.de; > aurelien@aurel32.net > Subject: Re: [Qemu-devel] [PATCH v2] target-mips: Fix incorrect shift for > SHILO and SHILOV > > > -----Original Message----- > > From: qemu-devel-bounces+ericj=mips.com@nongnu.org [mailto:qemu-devel- > > bounces+ericj=mips.com@nongnu.org] On Behalf Of Petar Jovanovic > > Sent: Tuesday, December 04, 2012 3:29 PM > > To: qemu-devel@nongnu.org > > Cc: blauwirbel@gmail.com; Jovanovic, Petar; rth7680@gmail.com; > > afaerber@suse.de; aurelien@aurel32.net > > Subject: [Qemu-devel] [PATCH v2] target-mips: Fix incorrect shift for > > SHILO and SHILOV > > > > From: Petar Jovanovic <petarj@mips.com> > > > > helper_shilo has not been shifting an accumulator value correctly for > > negative > > values in 'shift' field. Minor optimization for shift=0 case. > > This change also adds tests that will trigger issue and check for > > regressions. > > > > Signed-off-by: Petar Jovanovic <petarj@mips.com> > > --- > > target-mips/dsp_helper.c | 17 +++++++++-------- > > tests/tcg/mips/mips32-dsp/shilo.c | 18 ++++++++++++++++++ > > tests/tcg/mips/mips32-dsp/shilov.c | 20 ++++++++++++++++++++ > > 3 files changed, 47 insertions(+), 8 deletions(-) > > > > diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c > > index e7949c2..44f6dc7 100644 > > --- a/target-mips/dsp_helper.c > > +++ b/target-mips/dsp_helper.c > > @@ -3814,17 +3814,18 @@ void helper_shilo(target_ulong ac, target_ulong > > rs, CPUMIPSState *env) > > > > rs5_0 = rs & 0x3F; > > rs5_0 = (int8_t)(rs5_0 << 2) >> 2; > > - rs5_0 = MIPSDSP_ABS(rs5_0); > > + > > + if (unlikely(rs5_0 == 0)) { > > + return; > > + } > > + > > acc = (((uint64_t)env->active_tc.HI[ac] << 32) & MIPSDSP_LHI) | > > ((uint64_t)env->active_tc.LO[ac] & MIPSDSP_LLO); > > - if (rs5_0 == 0) { > > - temp = acc; > > + > > + if (rs5_0 > 0) { > > + temp = acc >> rs5_0; > > } else { > > - if (rs5_0 > 0) { > > - temp = acc >> rs5_0; > > - } else { > > - temp = acc << rs5_0; > > - } > > + temp = acc << -rs5_0; > > } > > > > env->active_tc.HI[ac] = (target_ulong)(int32_t)((temp & > MIPSDSP_LHI) > > >> 32); > > diff --git a/tests/tcg/mips/mips32-dsp/shilo.c b/tests/tcg/mips/mips32- > > dsp/shilo.c > > index b686616..ce8ebc6 100644 > > --- a/tests/tcg/mips/mips32-dsp/shilo.c > > +++ b/tests/tcg/mips/mips32-dsp/shilo.c > > @@ -23,5 +23,23 @@ int main() > > assert(ach == resulth); > > assert(acl == resultl); > > > > + > > + ach = 0x1; > > + acl = 0x80000000; > > + > > + resulth = 0x3; > > + resultl = 0x0; > > + > > + __asm > > + ("mthi %0, $ac1\n\t" > > + "mtlo %1, $ac1\n\t" > > + "shilo $ac1, -1\n\t" > > + "mfhi %0, $ac1\n\t" > > + "mflo %1, $ac1\n\t" > > + : "+r"(ach), "+r"(acl) > > + ); > > + assert(ach == resulth); > > + assert(acl == resultl); > > + > > return 0; > > } > > diff --git a/tests/tcg/mips/mips32-dsp/shilov.c b/tests/tcg/mips/mips32- > > dsp/shilov.c > > index f186032..e1d6cea 100644 > > --- a/tests/tcg/mips/mips32-dsp/shilov.c > > +++ b/tests/tcg/mips/mips32-dsp/shilov.c > > @@ -25,5 +25,25 @@ int main() > > assert(ach == resulth); > > assert(acl == resultl); > > > > + > > + rs = 0xffffffff; > > + ach = 0x1; > > + acl = 0x80000000; > > + > > + resulth = 0x3; > > + resultl = 0x0; > > + > > + __asm > > + ("mthi %0, $ac1\n\t" > > + "mtlo %1, $ac1\n\t" > > + "shilov $ac1, %2\n\t" > > + "mfhi %0, $ac1\n\t" > > + "mflo %1, $ac1\n\t" > > + : "+r"(ach), "+r"(acl) > > + : "r"(rs) > > + ); > > + assert(ach == resulth); > > + assert(acl == resultl); > > + > > return 0; > > } > > -- > > 1.7.5.4 > > > > Reviewed-by: Eric Johnson <ericj@mips.com>
On 2012-12-05 10:38, Peter Maydell wrote: > I don't think we should block this patch on that general > cleanup, though. All the sign extensions in target-mips/translate.c > are done in the double-shift way, so this is consistent with > the existing code. Fair enough. The original can have my Reviewed-by: Richard Henderson <rth@twiddle.net> r~
hey Eric, the patch does not have to be rebased since no changes have been made to these files since I created the patch. You can try it on clean master with: wget -O shilo.diff http://patchwork.ozlabs.org/patch/203744/mbox git am ./shilo.diff You probably have the previous (not yet committed) INSV patch on your branch that creates an issue for you when you run 'git am'. Petar
Sorry mail wrap issue. > -----Original Message----- > From: Jovanovic, Petar > Sent: Wednesday, December 05, 2012 3:31 PM > To: Johnson, Eric; qemu-devel@nongnu.org > Cc: blauwirbel@gmail.com; rth7680@gmail.com; afaerber@suse.de; > aurelien@aurel32.net > Subject: RE: [Qemu-devel] [PATCH v2] target-mips: Fix incorrect shift for > SHILO and SHILOV > > hey Eric, > > the patch does not have to be rebased since no changes have been made to > these files since I created the patch. > > You can try it on clean master with: > > wget -O shilo.diff http://patchwork.ozlabs.org/patch/203744/mbox > git am ./shilo.diff > > You probably have the previous (not yet committed) INSV patch on your > branch > that creates an issue for you when you run 'git am'. > > Petar > ________________________________________ > From: Johnson, Eric > Sent: Wednesday, December 05, 2012 10:36 PM > To: Jovanovic, Petar; qemu-devel@nongnu.org > Cc: blauwirbel@gmail.com; rth7680@gmail.com; afaerber@suse.de; > aurelien@aurel32.net > Subject: RE: [Qemu-devel] [PATCH v2] target-mips: Fix incorrect shift for > SHILO and SHILOV > > Oops, I forgot. The contents are OK but 'git am' didn't like the patch. > patch had to use fuzz. This may need to be rebased to latest master. > > -Eric > > > -----Original Message----- > > From: qemu-devel-bounces+ericj=mips.com@nongnu.org [mailto:qemu-devel- > > bounces+ericj=mips.com@nongnu.org] On Behalf Of Johnson, Eric > > Sent: Wednesday, December 05, 2012 12:42 PM > > To: Jovanovic, Petar; qemu-devel@nongnu.org > > Cc: blauwirbel@gmail.com; rth7680@gmail.com; afaerber@suse.de; > > aurelien@aurel32.net > > Subject: Re: [Qemu-devel] [PATCH v2] target-mips: Fix incorrect shift > for > > SHILO and SHILOV > > > > > -----Original Message----- > > > From: qemu-devel-bounces+ericj=mips.com@nongnu.org [mailto:qemu-devel- > > > bounces+ericj=mips.com@nongnu.org] On Behalf Of Petar Jovanovic > > > Sent: Tuesday, December 04, 2012 3:29 PM > > > To: qemu-devel@nongnu.org > > > Cc: blauwirbel@gmail.com; Jovanovic, Petar; rth7680@gmail.com; > > > afaerber@suse.de; aurelien@aurel32.net > > > Subject: [Qemu-devel] [PATCH v2] target-mips: Fix incorrect shift for > > > SHILO and SHILOV > > > > > > From: Petar Jovanovic <petarj@mips.com> > > > > > > helper_shilo has not been shifting an accumulator value correctly for > > > negative > > > values in 'shift' field. Minor optimization for shift=0 case. > > > This change also adds tests that will trigger issue and check for > > > regressions. > > > > > > Signed-off-by: Petar Jovanovic <petarj@mips.com> > > > --- > > > target-mips/dsp_helper.c | 17 +++++++++-------- > > > tests/tcg/mips/mips32-dsp/shilo.c | 18 ++++++++++++++++++ > > > tests/tcg/mips/mips32-dsp/shilov.c | 20 ++++++++++++++++++++ > > > 3 files changed, 47 insertions(+), 8 deletions(-) > > > > > > diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c > > > index e7949c2..44f6dc7 100644 > > > --- a/target-mips/dsp_helper.c > > > +++ b/target-mips/dsp_helper.c > > > @@ -3814,17 +3814,18 @@ void helper_shilo(target_ulong ac, > target_ulong > > > rs, CPUMIPSState *env) > > > > > > rs5_0 = rs & 0x3F; > > > rs5_0 = (int8_t)(rs5_0 << 2) >> 2; > > > - rs5_0 = MIPSDSP_ABS(rs5_0); > > > + > > > + if (unlikely(rs5_0 == 0)) { > > > + return; > > > + } > > > + > > > acc = (((uint64_t)env->active_tc.HI[ac] << 32) & MIPSDSP_LHI) | > > > ((uint64_t)env->active_tc.LO[ac] & MIPSDSP_LLO); > > > - if (rs5_0 == 0) { > > > - temp = acc; > > > + > > > + if (rs5_0 > 0) { > > > + temp = acc >> rs5_0; > > > } else { > > > - if (rs5_0 > 0) { > > > - temp = acc >> rs5_0; > > > - } else { > > > - temp = acc << rs5_0; > > > - } > > > + temp = acc << -rs5_0; > > > } > > > > > > env->active_tc.HI[ac] = (target_ulong)(int32_t)((temp & > > MIPSDSP_LHI) > > > >> 32); > > > diff --git a/tests/tcg/mips/mips32-dsp/shilo.c > b/tests/tcg/mips/mips32- > > > dsp/shilo.c > > > index b686616..ce8ebc6 100644 > > > --- a/tests/tcg/mips/mips32-dsp/shilo.c > > > +++ b/tests/tcg/mips/mips32-dsp/shilo.c > > > @@ -23,5 +23,23 @@ int main() > > > assert(ach == resulth); > > > assert(acl == resultl); > > > > > > + > > > + ach = 0x1; > > > + acl = 0x80000000; > > > + > > > + resulth = 0x3; > > > + resultl = 0x0; > > > + > > > + __asm > > > + ("mthi %0, $ac1\n\t" > > > + "mtlo %1, $ac1\n\t" > > > + "shilo $ac1, -1\n\t" > > > + "mfhi %0, $ac1\n\t" > > > + "mflo %1, $ac1\n\t" > > > + : "+r"(ach), "+r"(acl) > > > + ); > > > + assert(ach == resulth); > > > + assert(acl == resultl); > > > + > > > return 0; > > > } > > > diff --git a/tests/tcg/mips/mips32-dsp/shilov.c > b/tests/tcg/mips/mips32- > > > dsp/shilov.c > > > index f186032..e1d6cea 100644 > > > --- a/tests/tcg/mips/mips32-dsp/shilov.c > > > +++ b/tests/tcg/mips/mips32-dsp/shilov.c > > > @@ -25,5 +25,25 @@ int main() > > > assert(ach == resulth); > > > assert(acl == resultl); > > > > > > + > > > + rs = 0xffffffff; > > > + ach = 0x1; > > > + acl = 0x80000000; > > > + > > > + resulth = 0x3; > > > + resultl = 0x0; > > > + > > > + __asm > > > + ("mthi %0, $ac1\n\t" > > > + "mtlo %1, $ac1\n\t" > > > + "shilov $ac1, %2\n\t" > > > + "mfhi %0, $ac1\n\t" > > > + "mflo %1, $ac1\n\t" > > > + : "+r"(ach), "+r"(acl) > > > + : "r"(rs) > > > + ); > > > + assert(ach == resulth); > > > + assert(acl == resultl); > > > + > > > return 0; > > > } > > > -- > > > 1.7.5.4 > > > > > > > Reviewed-by: Eric Johnson <ericj@mips.com>
On Wed, Dec 05, 2012 at 12:29:10AM +0100, Petar Jovanovic wrote: > From: Petar Jovanovic <petarj@mips.com> > > helper_shilo has not been shifting an accumulator value correctly for negative > values in 'shift' field. Minor optimization for shift=0 case. > This change also adds tests that will trigger issue and check for regressions. > > Signed-off-by: Petar Jovanovic <petarj@mips.com> > --- > target-mips/dsp_helper.c | 17 +++++++++-------- > tests/tcg/mips/mips32-dsp/shilo.c | 18 ++++++++++++++++++ > tests/tcg/mips/mips32-dsp/shilov.c | 20 ++++++++++++++++++++ > 3 files changed, 47 insertions(+), 8 deletions(-) > > diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c > index e7949c2..44f6dc7 100644 > --- a/target-mips/dsp_helper.c > +++ b/target-mips/dsp_helper.c > @@ -3814,17 +3814,18 @@ void helper_shilo(target_ulong ac, target_ulong rs, CPUMIPSState *env) > > rs5_0 = rs & 0x3F; > rs5_0 = (int8_t)(rs5_0 << 2) >> 2; > - rs5_0 = MIPSDSP_ABS(rs5_0); > + > + if (unlikely(rs5_0 == 0)) { > + return; > + } > + > acc = (((uint64_t)env->active_tc.HI[ac] << 32) & MIPSDSP_LHI) | > ((uint64_t)env->active_tc.LO[ac] & MIPSDSP_LLO); > - if (rs5_0 == 0) { > - temp = acc; > + > + if (rs5_0 > 0) { > + temp = acc >> rs5_0; > } else { > - if (rs5_0 > 0) { > - temp = acc >> rs5_0; > - } else { > - temp = acc << rs5_0; > - } > + temp = acc << -rs5_0; > } > > env->active_tc.HI[ac] = (target_ulong)(int32_t)((temp & MIPSDSP_LHI) >> 32); > diff --git a/tests/tcg/mips/mips32-dsp/shilo.c b/tests/tcg/mips/mips32-dsp/shilo.c > index b686616..ce8ebc6 100644 > --- a/tests/tcg/mips/mips32-dsp/shilo.c > +++ b/tests/tcg/mips/mips32-dsp/shilo.c > @@ -23,5 +23,23 @@ int main() > assert(ach == resulth); > assert(acl == resultl); > > + > + ach = 0x1; > + acl = 0x80000000; > + > + resulth = 0x3; > + resultl = 0x0; > + > + __asm > + ("mthi %0, $ac1\n\t" > + "mtlo %1, $ac1\n\t" > + "shilo $ac1, -1\n\t" > + "mfhi %0, $ac1\n\t" > + "mflo %1, $ac1\n\t" > + : "+r"(ach), "+r"(acl) > + ); > + assert(ach == resulth); > + assert(acl == resultl); > + > return 0; > } > diff --git a/tests/tcg/mips/mips32-dsp/shilov.c b/tests/tcg/mips/mips32-dsp/shilov.c > index f186032..e1d6cea 100644 > --- a/tests/tcg/mips/mips32-dsp/shilov.c > +++ b/tests/tcg/mips/mips32-dsp/shilov.c > @@ -25,5 +25,25 @@ int main() > assert(ach == resulth); > assert(acl == resultl); > > + > + rs = 0xffffffff; > + ach = 0x1; > + acl = 0x80000000; > + > + resulth = 0x3; > + resultl = 0x0; > + > + __asm > + ("mthi %0, $ac1\n\t" > + "mtlo %1, $ac1\n\t" > + "shilov $ac1, %2\n\t" > + "mfhi %0, $ac1\n\t" > + "mflo %1, $ac1\n\t" > + : "+r"(ach), "+r"(acl) > + : "r"(rs) > + ); > + assert(ach == resulth); > + assert(acl == resultl); > + > return 0; > } Thanks, applied. I added a CC: to qemu-stable@nongnu.org, as it is definitely stable material.
On Wed, Dec 05, 2012 at 04:38:22PM +0000, Peter Maydell wrote: > On 5 December 2012 15:51, Richard Henderson <rth@twiddle.net> wrote: > > On 2012-12-05 09:49, Peter Maydell wrote: > >> On 5 December 2012 15:36, Richard Henderson <rth@twiddle.net> wrote: > >>> On 2012-12-04 17:29, Petar Jovanovic wrote: > >>>> rs5_0 = (int8_t)(rs5_0 << 2) >> 2; > >>> > >>> This is more portably written as > >>> > >>> rs5_0 = (rs5_0 ^ 0x20) - 0x20; > >> > >> ...but way more obscurely. If we want to play that > >> kind of game can we have a sign-extension function in > >> a header somewhere? > > > > I dunno about more obscurely. It took me a minute to figure out > > what was wanted in the original. > > > > As for a helper function... sure. > > I don't think we should block this patch on that general > cleanup, though. All the sign extensions in target-mips/translate.c > are done in the double-shift way, so this is consistent with > the existing code. > While it might be a good idea to make QEMU even more portable, it should be noticed that currently QEMU assumes that a left shift followed by the same arithmetic right shift. This is the case in at least the alpha, arm, m68k, mips, ppc, s390 and unicore32 targets, as well as for implementing sign extension and division in TCG.
diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c index e7949c2..44f6dc7 100644 --- a/target-mips/dsp_helper.c +++ b/target-mips/dsp_helper.c @@ -3814,17 +3814,18 @@ void helper_shilo(target_ulong ac, target_ulong rs, CPUMIPSState *env) rs5_0 = rs & 0x3F; rs5_0 = (int8_t)(rs5_0 << 2) >> 2; - rs5_0 = MIPSDSP_ABS(rs5_0); + + if (unlikely(rs5_0 == 0)) { + return; + } + acc = (((uint64_t)env->active_tc.HI[ac] << 32) & MIPSDSP_LHI) | ((uint64_t)env->active_tc.LO[ac] & MIPSDSP_LLO); - if (rs5_0 == 0) { - temp = acc; + + if (rs5_0 > 0) { + temp = acc >> rs5_0; } else { - if (rs5_0 > 0) { - temp = acc >> rs5_0; - } else { - temp = acc << rs5_0; - } + temp = acc << -rs5_0; } env->active_tc.HI[ac] = (target_ulong)(int32_t)((temp & MIPSDSP_LHI) >> 32); diff --git a/tests/tcg/mips/mips32-dsp/shilo.c b/tests/tcg/mips/mips32-dsp/shilo.c index b686616..ce8ebc6 100644 --- a/tests/tcg/mips/mips32-dsp/shilo.c +++ b/tests/tcg/mips/mips32-dsp/shilo.c @@ -23,5 +23,23 @@ int main() assert(ach == resulth); assert(acl == resultl); + + ach = 0x1; + acl = 0x80000000; + + resulth = 0x3; + resultl = 0x0; + + __asm + ("mthi %0, $ac1\n\t" + "mtlo %1, $ac1\n\t" + "shilo $ac1, -1\n\t" + "mfhi %0, $ac1\n\t" + "mflo %1, $ac1\n\t" + : "+r"(ach), "+r"(acl) + ); + assert(ach == resulth); + assert(acl == resultl); + return 0; } diff --git a/tests/tcg/mips/mips32-dsp/shilov.c b/tests/tcg/mips/mips32-dsp/shilov.c index f186032..e1d6cea 100644 --- a/tests/tcg/mips/mips32-dsp/shilov.c +++ b/tests/tcg/mips/mips32-dsp/shilov.c @@ -25,5 +25,25 @@ int main() assert(ach == resulth); assert(acl == resultl); + + rs = 0xffffffff; + ach = 0x1; + acl = 0x80000000; + + resulth = 0x3; + resultl = 0x0; + + __asm + ("mthi %0, $ac1\n\t" + "mtlo %1, $ac1\n\t" + "shilov $ac1, %2\n\t" + "mfhi %0, $ac1\n\t" + "mflo %1, $ac1\n\t" + : "+r"(ach), "+r"(acl) + : "r"(rs) + ); + assert(ach == resulth); + assert(acl == resultl); + return 0; }