[Qemu-devel] [PATCH target-arm] Don' use T[x] in helper

[Qemu-devel] [PATCH target-arm] Don' use T[x] in helper

[Torbjörn Andersson]

Hi all!

I hit a strange issue with add_cc, can't really explain how it appeared but
it did. The patch below solved my problem.

Can someone make a qualified guess why? I'm compling with gcc 4.

However, T0 and T1 in a helper should not be used, because it creates a
unnecessary dependency between translate.c and op_helper.c. Am I correct?

Further, if we cannot trust T0 and T1 in a helper, what about the global
env-variable?

/Best regards Torbjörn Andersson

Index: op_helper.c
===================================================================
--- op_helper.c	(revision 6883)
+++ op_helper.c	(working copy)
@@ -307,7 +307,7 @@
 uint32_t HELPER (add_cc)(uint32_t a, uint32_t b)
 {
     uint32_t result;
-    result = T0 + T1;
+    result = a + b;
     env->NF = env->ZF = result;
     env->CF = result < a;
     env->VF = (a ^ b ^ -1) & (a ^ result);

Re: [Qemu-devel] [PATCH target-arm] Don' use T[x] in helper

[Laurent Desnogues]

2009/3/24 Torbjörn Andersson <tobbe.tt@home.se>:
>
> I hit a strange issue with add_cc, can't really explain how it appeared but
> it did. The patch below solved my problem.
>
> Can someone make a qualified guess why? I'm compling with gcc 4.
>
> However, T0 and T1 in a helper should not be used, because it creates a
> unnecessary dependency between translate.c and op_helper.c. Am I correct?
>
> Further, if we cannot trust T0 and T1 in a helper, what about the global
> env-variable?

Your patch is correct and the previous code works by accident
since add_cc is called this way:

   gen_helper_add_cc(cpu_T[0], cpu_T[0], cpu_T[1])

What I find strange is that you had issue with the code in svn.  I
didn't have before I fixed it, but it was perhaps using some old
compiler at that time.


Laurent

Acked-by: Laurent Desnogues <laurent.desnogues@gmail.com>

> /Best regards Torbjörn Andersson
>
> Index: op_helper.c
> ===================================================================
> --- op_helper.c (revision 6883)
> +++ op_helper.c (working copy)
> @@ -307,7 +307,7 @@
>  uint32_t HELPER (add_cc)(uint32_t a, uint32_t b)
>  {
>     uint32_t result;
> -    result = T0 + T1;
> +    result = a + b;
>     env->NF = env->ZF = result;
>     env->CF = result < a;
>     env->VF = (a ^ b ^ -1) & (a ^ result);

Re: [Qemu-devel] [PATCH target-arm] Don' use T[x] in helper

[Aurelien Jarno]

On Tue, Mar 24, 2009 at 08:59:16PM +0100, Torbjörn Andersson wrote:
> Hi all!
> 
> I hit a strange issue with add_cc, can't really explain how it appeared but
> it did. The patch below solved my problem.
> 
> Can someone make a qualified guess why? I'm compling with gcc 4.
> 
> However, T0 and T1 in a helper should not be used, because it creates a
> unnecessary dependency between translate.c and op_helper.c. Am I correct?
> 
> Further, if we cannot trust T0 and T1 in a helper, what about the global
> env-variable?

Thanks, I applied your patch.

> /Best regards Torbjörn Andersson
> 
> Index: op_helper.c
> ===================================================================
> --- op_helper.c	(revision 6883)
> +++ op_helper.c	(working copy)
> @@ -307,7 +307,7 @@
>  uint32_t HELPER (add_cc)(uint32_t a, uint32_t b)
>  {
>      uint32_t result;
> -    result = T0 + T1;
> +    result = a + b;
>      env->NF = env->ZF = result;
>      env->CF = result < a;
>      env->VF = (a ^ b ^ -1) & (a ^ result);
> 
> 
> 
> 

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