* [Qemu-devel] [PATCH] target-arm/translate: Fix RRX operands
@ 2012-10-16 9:15 Peter Crosthwaite
2012-10-17 16:43 ` Peter Maydell
0 siblings, 1 reply; 3+ messages in thread
From: Peter Crosthwaite @ 2012-10-16 9:15 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: Peter Crosthwaite
Instructions that both use the RRX second operand and update CS were
incorrect, as the Carry flag was updated too early. An example of such an
instruction would be:
ands r12,r13,RRX
Ands, because of the "s" flag will update the carry flag. But the RRX second
operand rotates through the C flag which should happen before the update.
Fixed the ordering of the two, the old carry is read by "r13,RRX" before being
updated.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reported-by: Vinesh Peringat <vineshp@xilinx.com>
---
target-arm/translate.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index c6840b7..daccb15 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -516,10 +516,10 @@ static inline void gen_arm_shift_im(TCGv var, int shiftop, int shift, int flags)
tcg_gen_rotri_i32(var, var, shift); break;
} else {
TCGv tmp = tcg_temp_new_i32();
+ tcg_gen_shli_i32(tmp, cpu_CF, 31);
if (flags)
shifter_out_im(var, 0);
tcg_gen_shri_i32(var, var, 1);
- tcg_gen_shli_i32(tmp, cpu_CF, 31);
tcg_gen_or_i32(var, var, tmp);
tcg_temp_free_i32(tmp);
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] target-arm/translate: Fix RRX operands
2012-10-16 9:15 [Qemu-devel] [PATCH] target-arm/translate: Fix RRX operands Peter Crosthwaite
@ 2012-10-17 16:43 ` Peter Maydell
2012-10-17 18:03 ` Aurelien Jarno
0 siblings, 1 reply; 3+ messages in thread
From: Peter Maydell @ 2012-10-17 16:43 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Crosthwaite, qemu-devel, Aurelien Jarno
On 16 October 2012 10:15, Peter Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> Instructions that both use the RRX second operand and update CS were
> incorrect, as the Carry flag was updated too early. An example of such an
> instruction would be:
>
> ands r12,r13,RRX
>
> Ands, because of the "s" flag will update the carry flag. But the RRX second
> operand rotates through the C flag which should happen before the update.
> Fixed the ordering of the two, the old carry is read by "r13,RRX" before being
> updated.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Reported-by: Vinesh Peringat <vineshp@xilinx.com>
> ---
> target-arm/translate.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index c6840b7..daccb15 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -516,10 +516,10 @@ static inline void gen_arm_shift_im(TCGv var, int shiftop, int shift, int flags)
> tcg_gen_rotri_i32(var, var, shift); break;
> } else {
> TCGv tmp = tcg_temp_new_i32();
> + tcg_gen_shli_i32(tmp, cpu_CF, 31);
> if (flags)
> shifter_out_im(var, 0);
> tcg_gen_shri_i32(var, var, 1);
> - tcg_gen_shli_i32(tmp, cpu_CF, 31);
> tcg_gen_or_i32(var, var, tmp);
> tcg_temp_free_i32(tmp);
> }
Looks like this was broken by Aurelien's commit 66c374de8; previously
we loaded CF into a tmp before doing the shifter_out_im() [which updates CF],
and then used the tmp after the call, rather than directly using CF.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
-- PMM
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] target-arm/translate: Fix RRX operands
2012-10-17 16:43 ` Peter Maydell
@ 2012-10-17 18:03 ` Aurelien Jarno
0 siblings, 0 replies; 3+ messages in thread
From: Aurelien Jarno @ 2012-10-17 18:03 UTC (permalink / raw)
To: Peter Maydell; +Cc: Peter Crosthwaite, Peter Crosthwaite, qemu-devel
On Wed, Oct 17, 2012 at 05:43:55PM +0100, Peter Maydell wrote:
> On 16 October 2012 10:15, Peter Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
> > Instructions that both use the RRX second operand and update CS were
> > incorrect, as the Carry flag was updated too early. An example of such an
> > instruction would be:
> >
> > ands r12,r13,RRX
> >
> > Ands, because of the "s" flag will update the carry flag. But the RRX second
> > operand rotates through the C flag which should happen before the update.
> > Fixed the ordering of the two, the old carry is read by "r13,RRX" before being
> > updated.
> >
> > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> > Reported-by: Vinesh Peringat <vineshp@xilinx.com>
> > ---
> > target-arm/translate.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/target-arm/translate.c b/target-arm/translate.c
> > index c6840b7..daccb15 100644
> > --- a/target-arm/translate.c
> > +++ b/target-arm/translate.c
> > @@ -516,10 +516,10 @@ static inline void gen_arm_shift_im(TCGv var, int shiftop, int shift, int flags)
> > tcg_gen_rotri_i32(var, var, shift); break;
> > } else {
> > TCGv tmp = tcg_temp_new_i32();
> > + tcg_gen_shli_i32(tmp, cpu_CF, 31);
> > if (flags)
> > shifter_out_im(var, 0);
> > tcg_gen_shri_i32(var, var, 1);
> > - tcg_gen_shli_i32(tmp, cpu_CF, 31);
> > tcg_gen_or_i32(var, var, tmp);
> > tcg_temp_free_i32(tmp);
> > }
>
> Looks like this was broken by Aurelien's commit 66c374de8; previously
> we loaded CF into a tmp before doing the shifter_out_im() [which updates CF],
> and then used the tmp after the call, rather than directly using CF.
Yes, I am the culprit here, sorry about that.
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
And applied.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-10-17 18:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-16 9:15 [Qemu-devel] [PATCH] target-arm/translate: Fix RRX operands Peter Crosthwaite
2012-10-17 16:43 ` Peter Maydell
2012-10-17 18:03 ` Aurelien Jarno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).