qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] 1.4.1 won't build with --enable-debug-tcg (or --enable-debug)
@ 2013-05-03 18:28 Juergen Lock
  2013-05-04 14:23 ` Richard Sandiford
  0 siblings, 1 reply; 4+ messages in thread
From: Juergen Lock @ 2013-05-03 18:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: Richard Sandiford, Aurelien Jarno

Hi!

 The failure is in the mips64-softmmu target: (at least)

[...]
  CC    mips64-softmmu/target-mips/translate.o
 ..qemu-1.4.1/target-mips/translate.c::2780:35 : error: 
      passing 'int' to parameter of incompatible type 'TCGv_i32'
        gen_helper_dmult(cpu_env, acc, t0, t1);
                                  ^~~
[...]

 Looks like this line came in with this patch by Aurelien Jarno: (Cc'd)

	http://patchwork.ozlabs.org/patch/234926/

 Now I wonder is the failure just cosmetic or is the mips emulation
actually broken in 1.4.1?  More so than in 1.4.0?  I guess not but
with all the #defines that code is kinda hard to follow... :)

 Thanx!
	Juergen

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] 1.4.1 won't build with --enable-debug-tcg (or --enable-debug)
  2013-05-03 18:28 [Qemu-devel] 1.4.1 won't build with --enable-debug-tcg (or --enable-debug) Juergen Lock
@ 2013-05-04 14:23 ` Richard Sandiford
  2013-05-05 20:34   ` Juergen Lock
  2013-05-06 17:51   ` Aurelien Jarno
  0 siblings, 2 replies; 4+ messages in thread
From: Richard Sandiford @ 2013-05-04 14:23 UTC (permalink / raw)
  To: Juergen Lock; +Cc: qemu-devel, Aurelien Jarno

[-- Attachment #1: Type: text/plain, Size: 791 bytes --]

Juergen Lock <nox@jelal.kn-bremen.de> writes:
> Hi!
>
>  The failure is in the mips64-softmmu target: (at least)
>
> [...]
>   CC    mips64-softmmu/target-mips/translate.o
>  ..qemu-1.4.1/target-mips/translate.c::2780:35 : error: 
>       passing 'int' to parameter of incompatible type 'TCGv_i32'
>         gen_helper_dmult(cpu_env, acc, t0, t1);
>                                   ^~~
> [...]
>
>  Looks like this line came in with this patch by Aurelien Jarno: (Cc'd)
>
> 	http://patchwork.ozlabs.org/patch/234926/

Ouch.  I can see what Michael means about scary conflicts.  The code
in the 1.4 branch looks different from both the code at the time the
patch was submitted and the code at the time the patch was applied.

Here's one fix, but maybe Aurelien has a better idea.

Richard


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-target-mips-Fix-accumulator-arguments-to-gen_helper_.patch --]
[-- Type: text/x-patch, Size: 2724 bytes --]

>From 61b79e34bc57df0aa0c8086bd86f4c8818618d0e Mon Sep 17 00:00:00 2001
From: Richard Sandiford <rdsandiford@googlemail.com>
Date: Sat, 4 May 2013 15:01:31 +0100
Subject: [PATCH] target-mips: Fix accumulator arguments to gen_helper_dmult(u)

gen_muldiv was passing int accumulator arguments directly
to gen_helper_dmult(u).  This patch fixes it to use TCGs,
via the gen_helper_0e2i wrapper.

Fixes an --enable-debug-tcg build failure reported by Juergen Lock.

Signed-off-by: Richard Sandiford <rdsandiford@googlemail.com>
---
 target-mips/helper.h    | 4 ++--
 target-mips/op_helper.c | 8 ++++----
 target-mips/translate.c | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/target-mips/helper.h b/target-mips/helper.h
index cfe98f1..7aa5f79 100644
--- a/target-mips/helper.h
+++ b/target-mips/helper.h
@@ -24,8 +24,8 @@ DEF_HELPER_FLAGS_1(clz, TCG_CALL_NO_RWG_SE, tl, tl)
 #ifdef TARGET_MIPS64
 DEF_HELPER_FLAGS_1(dclo, TCG_CALL_NO_RWG_SE, tl, tl)
 DEF_HELPER_FLAGS_1(dclz, TCG_CALL_NO_RWG_SE, tl, tl)
-DEF_HELPER_4(dmult, void, env, int, tl, tl)
-DEF_HELPER_4(dmultu, void, env, int, tl, tl)
+DEF_HELPER_4(dmult, void, env, tl, tl, int)
+DEF_HELPER_4(dmultu, void, env, tl, tl, int)
 #endif
 
 DEF_HELPER_3(muls, tl, env, tl, tl)
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index c054300..01df687 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -268,14 +268,14 @@ target_ulong helper_mulshiu(CPUMIPSState *env, target_ulong arg1,
 }
 
 #ifdef TARGET_MIPS64
-void helper_dmult(CPUMIPSState *env, int acc, target_ulong arg1,
-                  target_ulong arg2)
+void helper_dmult(CPUMIPSState *env, target_ulong arg1,
+                  target_ulong arg2, int acc)
 {
     muls64(&(env->active_tc.LO[acc]), &(env->active_tc.HI[acc]), arg1, arg2);
 }
 
-void helper_dmultu(CPUMIPSState *env, int acc, target_ulong arg1,
-                   target_ulong arg2)
+void helper_dmultu(CPUMIPSState *env, target_ulong arg1,
+                   target_ulong arg2, int acc)
 {
     mulu64(&(env->active_tc.LO[acc]), &(env->active_tc.HI[acc]), arg1, arg2);
 }
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 9ed6477..8205456 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -2777,11 +2777,11 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
         opn = "ddivu";
         break;
     case OPC_DMULT:
-        gen_helper_dmult(cpu_env, acc, t0, t1);
+        gen_helper_0e2i(dmult, t0, t1, acc);
         opn = "dmult";
         break;
     case OPC_DMULTU:
-        gen_helper_dmultu(cpu_env, acc, t0, t1);
+        gen_helper_0e2i(dmultu, t0, t1, acc);
         opn = "dmultu";
         break;
 #endif
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] 1.4.1 won't build with --enable-debug-tcg (or --enable-debug)
  2013-05-04 14:23 ` Richard Sandiford
@ 2013-05-05 20:34   ` Juergen Lock
  2013-05-06 17:51   ` Aurelien Jarno
  1 sibling, 0 replies; 4+ messages in thread
From: Juergen Lock @ 2013-05-05 20:34 UTC (permalink / raw)
  To: Juergen Lock, qemu-devel, Aurelien Jarno, rdsandiford

On Sat, May 04, 2013 at 03:23:28PM +0100, Richard Sandiford wrote:
> Juergen Lock <nox@jelal.kn-bremen.de> writes:
> > Hi!
> >
> >  The failure is in the mips64-softmmu target: (at least)
> >
> > [...]
> >   CC    mips64-softmmu/target-mips/translate.o
> >  ..qemu-1.4.1/target-mips/translate.c::2780:35 : error: 
> >       passing 'int' to parameter of incompatible type 'TCGv_i32'
> >         gen_helper_dmult(cpu_env, acc, t0, t1);
> >                                   ^~~
> > [...]
> >
> >  Looks like this line came in with this patch by Aurelien Jarno: (Cc'd)
> >
> > 	http://patchwork.ozlabs.org/patch/234926/
> 
> Ouch.  I can see what Michael means about scary conflicts.  The code
> in the 1.4 branch looks different from both the code at the time the
> patch was submitted and the code at the time the patch was applied.
> 
> Here's one fix, but maybe Aurelien has a better idea.
> 
Thanx, I just updated the FreeBSD port to 1.4.1 and included this fix.

	Juergen

> Richard
> 

> >From 61b79e34bc57df0aa0c8086bd86f4c8818618d0e Mon Sep 17 00:00:00 2001
> From: Richard Sandiford <rdsandiford@googlemail.com>
> Date: Sat, 4 May 2013 15:01:31 +0100
> Subject: [PATCH] target-mips: Fix accumulator arguments to gen_helper_dmult(u)
> 
> gen_muldiv was passing int accumulator arguments directly
> to gen_helper_dmult(u).  This patch fixes it to use TCGs,
> via the gen_helper_0e2i wrapper.
> 
> Fixes an --enable-debug-tcg build failure reported by Juergen Lock.
> 
> Signed-off-by: Richard Sandiford <rdsandiford@googlemail.com>
> ---
>  target-mips/helper.h    | 4 ++--
>  target-mips/op_helper.c | 8 ++++----
>  target-mips/translate.c | 4 ++--
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/target-mips/helper.h b/target-mips/helper.h
> index cfe98f1..7aa5f79 100644
> --- a/target-mips/helper.h
> +++ b/target-mips/helper.h
> @@ -24,8 +24,8 @@ DEF_HELPER_FLAGS_1(clz, TCG_CALL_NO_RWG_SE, tl, tl)
>  #ifdef TARGET_MIPS64
>  DEF_HELPER_FLAGS_1(dclo, TCG_CALL_NO_RWG_SE, tl, tl)
>  DEF_HELPER_FLAGS_1(dclz, TCG_CALL_NO_RWG_SE, tl, tl)
> -DEF_HELPER_4(dmult, void, env, int, tl, tl)
> -DEF_HELPER_4(dmultu, void, env, int, tl, tl)
> +DEF_HELPER_4(dmult, void, env, tl, tl, int)
> +DEF_HELPER_4(dmultu, void, env, tl, tl, int)
>  #endif
>  
>  DEF_HELPER_3(muls, tl, env, tl, tl)
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index c054300..01df687 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -268,14 +268,14 @@ target_ulong helper_mulshiu(CPUMIPSState *env, target_ulong arg1,
>  }
>  
>  #ifdef TARGET_MIPS64
> -void helper_dmult(CPUMIPSState *env, int acc, target_ulong arg1,
> -                  target_ulong arg2)
> +void helper_dmult(CPUMIPSState *env, target_ulong arg1,
> +                  target_ulong arg2, int acc)
>  {
>      muls64(&(env->active_tc.LO[acc]), &(env->active_tc.HI[acc]), arg1, arg2);
>  }
>  
> -void helper_dmultu(CPUMIPSState *env, int acc, target_ulong arg1,
> -                   target_ulong arg2)
> +void helper_dmultu(CPUMIPSState *env, target_ulong arg1,
> +                   target_ulong arg2, int acc)
>  {
>      mulu64(&(env->active_tc.LO[acc]), &(env->active_tc.HI[acc]), arg1, arg2);
>  }
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 9ed6477..8205456 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -2777,11 +2777,11 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
>          opn = "ddivu";
>          break;
>      case OPC_DMULT:
> -        gen_helper_dmult(cpu_env, acc, t0, t1);
> +        gen_helper_0e2i(dmult, t0, t1, acc);
>          opn = "dmult";
>          break;
>      case OPC_DMULTU:
> -        gen_helper_dmultu(cpu_env, acc, t0, t1);
> +        gen_helper_0e2i(dmultu, t0, t1, acc);
>          opn = "dmultu";
>          break;
>  #endif
> -- 
> 1.8.1.4
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] 1.4.1 won't build with --enable-debug-tcg (or --enable-debug)
  2013-05-04 14:23 ` Richard Sandiford
  2013-05-05 20:34   ` Juergen Lock
@ 2013-05-06 17:51   ` Aurelien Jarno
  1 sibling, 0 replies; 4+ messages in thread
From: Aurelien Jarno @ 2013-05-06 17:51 UTC (permalink / raw)
  To: Juergen Lock, qemu-devel, rdsandiford; +Cc: qemu-stable

Hi,

On Sat, May 04, 2013 at 03:23:28PM +0100, Richard Sandiford wrote:
> Juergen Lock <nox@jelal.kn-bremen.de> writes:
> > Hi!
> >
> >  The failure is in the mips64-softmmu target: (at least)
> >
> > [...]
> >   CC    mips64-softmmu/target-mips/translate.o
> >  ..qemu-1.4.1/target-mips/translate.c::2780:35 : error: 
> >       passing 'int' to parameter of incompatible type 'TCGv_i32'
> >         gen_helper_dmult(cpu_env, acc, t0, t1);
> >                                   ^~~
> > [...]
> >
> >  Looks like this line came in with this patch by Aurelien Jarno: (Cc'd)
> >
> > 	http://patchwork.ozlabs.org/patch/234926/
>
> Ouch.  I can see what Michael means about scary conflicts.  The code
> in the 1.4 branch looks different from both the code at the time the
> patch was submitted and the code at the time the patch was applied.

I made this mistake when fixing the conflict which appeared when
backporting the patch to stable. Maybe we should have live with the
bug in the stable version instead. That said, I haven't seen the
problem when booting various guests, it probably works correctly when
not using DSP extensions.

> Here's one fix, but maybe Aurelien has a better idea.
> 

The fix is correct. Thanks.

Aurelien

> From 61b79e34bc57df0aa0c8086bd86f4c8818618d0e Mon Sep 17 00:00:00 2001
> From: Richard Sandiford <rdsandiford@googlemail.com>
> Date: Sat, 4 May 2013 15:01:31 +0100
> Subject: [PATCH] target-mips: Fix accumulator arguments to gen_helper_dmult(u)
> 
> gen_muldiv was passing int accumulator arguments directly
> to gen_helper_dmult(u).  This patch fixes it to use TCGs,
> via the gen_helper_0e2i wrapper.
> 
> Fixes an --enable-debug-tcg build failure reported by Juergen Lock.
> 
> Signed-off-by: Richard Sandiford <rdsandiford@googlemail.com>
> ---
>  target-mips/helper.h    | 4 ++--
>  target-mips/op_helper.c | 8 ++++----
>  target-mips/translate.c | 4 ++--
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/target-mips/helper.h b/target-mips/helper.h
> index cfe98f1..7aa5f79 100644
> --- a/target-mips/helper.h
> +++ b/target-mips/helper.h
> @@ -24,8 +24,8 @@ DEF_HELPER_FLAGS_1(clz, TCG_CALL_NO_RWG_SE, tl, tl)
>  #ifdef TARGET_MIPS64
>  DEF_HELPER_FLAGS_1(dclo, TCG_CALL_NO_RWG_SE, tl, tl)
>  DEF_HELPER_FLAGS_1(dclz, TCG_CALL_NO_RWG_SE, tl, tl)
> -DEF_HELPER_4(dmult, void, env, int, tl, tl)
> -DEF_HELPER_4(dmultu, void, env, int, tl, tl)
> +DEF_HELPER_4(dmult, void, env, tl, tl, int)
> +DEF_HELPER_4(dmultu, void, env, tl, tl, int)
>  #endif
>  
>  DEF_HELPER_3(muls, tl, env, tl, tl)
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index c054300..01df687 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -268,14 +268,14 @@ target_ulong helper_mulshiu(CPUMIPSState *env, target_ulong arg1,
>  }
>  
>  #ifdef TARGET_MIPS64
> -void helper_dmult(CPUMIPSState *env, int acc, target_ulong arg1,
> -                  target_ulong arg2)
> +void helper_dmult(CPUMIPSState *env, target_ulong arg1,
> +                  target_ulong arg2, int acc)
>  {
>      muls64(&(env->active_tc.LO[acc]), &(env->active_tc.HI[acc]), arg1, arg2);
>  }
>  
> -void helper_dmultu(CPUMIPSState *env, int acc, target_ulong arg1,
> -                   target_ulong arg2)
> +void helper_dmultu(CPUMIPSState *env, target_ulong arg1,
> +                   target_ulong arg2, int acc)
>  {
>      mulu64(&(env->active_tc.LO[acc]), &(env->active_tc.HI[acc]), arg1, arg2);
>  }
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 9ed6477..8205456 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -2777,11 +2777,11 @@ static void gen_muldiv(DisasContext *ctx, uint32_t opc,
>          opn = "ddivu";
>          break;
>      case OPC_DMULT:
> -        gen_helper_dmult(cpu_env, acc, t0, t1);
> +        gen_helper_0e2i(dmult, t0, t1, acc);
>          opn = "dmult";
>          break;
>      case OPC_DMULTU:
> -        gen_helper_dmultu(cpu_env, acc, t0, t1);
> +        gen_helper_0e2i(dmultu, t0, t1, acc);
>          opn = "dmultu";
>          break;
>  #endif

Acked-by: Aurelien Jarno <aurelien@aurel32.net>



-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-05-06 17:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-03 18:28 [Qemu-devel] 1.4.1 won't build with --enable-debug-tcg (or --enable-debug) Juergen Lock
2013-05-04 14:23 ` Richard Sandiford
2013-05-05 20:34   ` Juergen Lock
2013-05-06 17:51   ` 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).