Patchwork [for-1.2] target-ppc: fix altivec instructions

login
register
mail settings
Submitter Aurelien Jarno
Date Aug. 26, 2012, 2:14 p.m.
Message ID <1345990448-15587-1-git-send-email-aurelien@aurel32.net>
Download mbox | patch
Permalink /patch/180056/
State New
Headers show

Comments

Aurelien Jarno - Aug. 26, 2012, 2:14 p.m.
Altivec instructions are not working anymore in PowerPC emulation,
following commit d15f74fb, which inverted two registers in the call
to helper. Fix that.

Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Andreas Färber <afaerber@suse.de>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-ppc/translate.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Peter Maydell - Aug. 26, 2012, 3:25 p.m.
On 26 August 2012 15:14, Aurelien Jarno <aurelien@aurel32.net> wrote:
> Altivec instructions are not working anymore in PowerPC emulation,
> following commit d15f74fb, which inverted two registers in the call
> to helper. Fix that.
>
> Cc: Blue Swirl <blauwirbel@gmail.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target-ppc/translate.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 91eb7a0..ac915cc 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -6530,7 +6530,7 @@ static void glue(gen_, name)(DisasContext *ctx)                         \
>      ra = gen_avr_ptr(rA(ctx->opcode));                                  \
>      rb = gen_avr_ptr(rB(ctx->opcode));                                  \
>      rd = gen_avr_ptr(rD(ctx->opcode));                                  \
> -    gen_helper_##name(rd, cpu_env, ra, rb);                             \
> +    gen_helper_##name(cpu_env, rd, ra, rb);                             \
>      tcg_temp_free_ptr(ra);                                              \
>      tcg_temp_free_ptr(rb);                                              \
>      tcg_temp_free_ptr(rd);                                              \

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

(For these helpers, rd is an input to the helper, not an output,
which is why the cpu_env has to go first, unlike eg gen_helper_mulldo().)

-- PMM
Andreas Färber - Aug. 26, 2012, 3:27 p.m.
Am 26.08.2012 16:14, schrieb Aurelien Jarno:
> Altivec instructions are not working anymore in PowerPC emulation,
> following commit d15f74fb, which inverted two registers in the call
> to helper. Fix that.
> 
> Cc: Blue Swirl <blauwirbel@gmail.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

Reviewed-by: Andreas Färber <afaerber@suse.de>

This looks right, but do you have a particular test case I can check?

Andreas

> ---
>  target-ppc/translate.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 91eb7a0..ac915cc 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -6530,7 +6530,7 @@ static void glue(gen_, name)(DisasContext *ctx)                         \
>      ra = gen_avr_ptr(rA(ctx->opcode));                                  \
>      rb = gen_avr_ptr(rB(ctx->opcode));                                  \
>      rd = gen_avr_ptr(rD(ctx->opcode));                                  \
> -    gen_helper_##name(rd, cpu_env, ra, rb);                             \
> +    gen_helper_##name(cpu_env, rd, ra, rb);                             \
>      tcg_temp_free_ptr(ra);                                              \
>      tcg_temp_free_ptr(rb);                                              \
>      tcg_temp_free_ptr(rd);                                              \
>
Aurelien Jarno - Aug. 26, 2012, 3:46 p.m.
On Sun, Aug 26, 2012 at 05:27:59PM +0200, Andreas Färber wrote:
> Am 26.08.2012 16:14, schrieb Aurelien Jarno:
> > Altivec instructions are not working anymore in PowerPC emulation,
> > following commit d15f74fb, which inverted two registers in the call
> > to helper. Fix that.
> > 
> > Cc: Blue Swirl <blauwirbel@gmail.com>
> > Cc: Alexander Graf <agraf@suse.de>
> > Cc: Andreas Färber <afaerber@suse.de>
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> 
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> 
> This looks right, but do you have a particular test case I can check?
> 

The Gwenole Beauchesne testsuite (attached).
Blue Swirl - Aug. 26, 2012, 5:56 p.m.
On Sun, Aug 26, 2012 at 2:14 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> Altivec instructions are not working anymore in PowerPC emulation,
> following commit d15f74fb, which inverted two registers in the call
> to helper. Fix that.
>
> Cc: Blue Swirl <blauwirbel@gmail.com>

Acked-by: Blue Swirl <blauwirbel@gmail.com>

I wonder why TCG debug did not catch this.

> Cc: Alexander Graf <agraf@suse.de>
> Cc: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target-ppc/translate.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 91eb7a0..ac915cc 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -6530,7 +6530,7 @@ static void glue(gen_, name)(DisasContext *ctx)                         \
>      ra = gen_avr_ptr(rA(ctx->opcode));                                  \
>      rb = gen_avr_ptr(rB(ctx->opcode));                                  \
>      rd = gen_avr_ptr(rD(ctx->opcode));                                  \
> -    gen_helper_##name(rd, cpu_env, ra, rb);                             \
> +    gen_helper_##name(cpu_env, rd, ra, rb);                             \
>      tcg_temp_free_ptr(ra);                                              \
>      tcg_temp_free_ptr(rb);                                              \
>      tcg_temp_free_ptr(rd);                                              \
> --
> 1.7.10.4
>
Peter Maydell - Aug. 26, 2012, 6:17 p.m.
On 26 August 2012 18:56, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Sun, Aug 26, 2012 at 2:14 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
>> Altivec instructions are not working anymore in PowerPC emulation,
>> following commit d15f74fb, which inverted two registers in the call
>> to helper. Fix that.

> I wonder why TCG debug did not catch this.

Because all of ra, rb, rd and cpu_env are TCGv_ptr. Debug only
catches mismatches between _i32, _i64 and _ptr. It might be
possible to add support for enforcing that you pass a cpu_env
in where your DEF_HELPER_* had an 'env' parameter, but it would
be slightly different from the current checks because you want
to support passing a cpu_env TCGv in where a TCGv_ptr is OK
as well as the places which require exactly a TCGv_env.

-- PMM

Patch

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 91eb7a0..ac915cc 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -6530,7 +6530,7 @@  static void glue(gen_, name)(DisasContext *ctx)                         \
     ra = gen_avr_ptr(rA(ctx->opcode));                                  \
     rb = gen_avr_ptr(rB(ctx->opcode));                                  \
     rd = gen_avr_ptr(rD(ctx->opcode));                                  \
-    gen_helper_##name(rd, cpu_env, ra, rb);                             \
+    gen_helper_##name(cpu_env, rd, ra, rb);                             \
     tcg_temp_free_ptr(ra);                                              \
     tcg_temp_free_ptr(rb);                                              \
     tcg_temp_free_ptr(rd);                                              \