[Qemu-devel] [PATCH] risu-m68k: update fpregs

[Qemu-devel] [PATCH] risu-m68k: update fpregs

[Laurent Vivier]

f_fpregs is a 2d array, not 1d:

 typedef struct fpregset
 {
   int f_pcr;
   int f_psr;
   int f_fpiaddr;
 #ifdef __mcoldfire__
   int f_fpregs[8][2];
 #else
   int f_fpregs[8][3];
 #endif
 } fpregset_t;

For the moment, we don't manage ColdFire case, only 680x0.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 risu_reginfo_m68k.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/risu_reginfo_m68k.c b/risu_reginfo_m68k.c
index c9d21cc..d0d47d9 100644
--- a/risu_reginfo_m68k.c
+++ b/risu_reginfo_m68k.c
@@ -31,9 +31,9 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
     ri->fpregs.f_psr = uc->uc_mcontext.fpregs.f_psr;
     ri->fpregs.f_fpiaddr = uc->uc_mcontext.fpregs.f_fpiaddr;
     for (i = 0; i < 8; i++) {
-        memcpy(&ri->fpregs.f_fpregs[i * 3],
-               &uc->uc_mcontext.fpregs.f_fpregs[i * 3],
-               3 * sizeof(int));
+        memcpy(ri->fpregs.f_fpregs[i],
+               uc->uc_mcontext.fpregs.f_fpregs[i],
+               sizeof(ri->fpregs.f_fpregs[0]));
     }
 }
 
@@ -64,9 +64,9 @@ int reginfo_is_eq(struct reginfo *m, struct reginfo *a, ucontext_t *uc)
     }
 
     for (i = 0; i < 8; i++) {
-        if (m->fpregs.f_fpregs[i * 3] != a->fpregs.f_fpregs[i * 3] ||
-            m->fpregs.f_fpregs[i * 3 + 1] != a->fpregs.f_fpregs[i * 3 + 1] ||
-            m->fpregs.f_fpregs[i * 3 + 2] != a->fpregs.f_fpregs[i * 3 + 2]) {
+        if (m->fpregs.f_fpregs[i][0] != a->fpregs.f_fpregs[i][0] ||
+            m->fpregs.f_fpregs[i][1] != a->fpregs.f_fpregs[i][1] ||
+            m->fpregs.f_fpregs[i][2] != a->fpregs.f_fpregs[i][2]) {
             return 0;
         }
     }
@@ -93,8 +93,8 @@ void reginfo_dump(struct reginfo *ri, int is_master)
 
     for (i = 0; i < 8; i++) {
         fprintf(stderr, "\tFP%d: %08x %08x %08x\n", i,
-                ri->fpregs.f_fpregs[i * 3], ri->fpregs.f_fpregs[i * 3 + 1],
-                ri->fpregs.f_fpregs[i * 3 + 2]);
+                ri->fpregs.f_fpregs[i][0], ri->fpregs.f_fpregs[i][1],
+                ri->fpregs.f_fpregs[i][2]);
     }
 
     fprintf(stderr, "\n");
@@ -134,15 +134,14 @@ int reginfo_dump_mismatch(struct reginfo *m, struct reginfo *a, FILE *f)
     }
 
     for (i = 0; i < 8; i++) {
-        if (m->fpregs.f_fpregs[i * 3] != a->fpregs.f_fpregs[i * 3] ||
-            m->fpregs.f_fpregs[i * 3 + 1] != a->fpregs.f_fpregs[i * 3 + 1] ||
-            m->fpregs.f_fpregs[i * 3 + 2] != a->fpregs.f_fpregs[i * 3 + 2]) {
+        if (m->fpregs.f_fpregs[i][0] != a->fpregs.f_fpregs[i][0] ||
+            m->fpregs.f_fpregs[i][1] != a->fpregs.f_fpregs[i][1] ||
+            m->fpregs.f_fpregs[i][2] != a->fpregs.f_fpregs[i][2]) {
             fprintf(f, "Mismatch: Register FP%d\n", i);
             fprintf(f, "m: [%08x %08x %08x] != a: [%08x %08x %08x]\n",
-                    m->fpregs.f_fpregs[i * 3], m->fpregs.f_fpregs[i * 3 + 1],
-                    m->fpregs.f_fpregs[i * 3 + 2], a->fpregs.f_fpregs[i * 3],
-                    a->fpregs.f_fpregs[i * 3 + 1],
-                    a->fpregs.f_fpregs[i * 3 + 2]);
+                    m->fpregs.f_fpregs[i][0], m->fpregs.f_fpregs[i][1],
+                    m->fpregs.f_fpregs[i][2], a->fpregs.f_fpregs[i][0],
+                    a->fpregs.f_fpregs[i][1], a->fpregs.f_fpregs[i][2]);
         }
     }
 
-- 
2.9.3

Re: [Qemu-devel] [PATCH] risu-m68k: update fpregs

[Peter Maydell]

On 19 February 2017 at 20:02, Laurent Vivier <laurent@vivier.eu> wrote:
> f_fpregs is a 2d array, not 1d:
>
>  typedef struct fpregset
>  {
>    int f_pcr;
>    int f_psr;
>    int f_fpiaddr;
>  #ifdef __mcoldfire__
>    int f_fpregs[8][2];
>  #else
>    int f_fpregs[8][3];
>  #endif
>  } fpregset_t;
>
> For the moment, we don't manage ColdFire case, only 680x0.
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>

Thanks; applied to target-arm.next.

I've also pushed out support for building risu into a separate
build directory, and a 'build-all-archs' script that will
build every target CPU arch that you have a cross compiler
installed for (Debian and Ubuntu package cross compilers for
everything, helpfully).

thanks
-- PMM

Re: [Qemu-devel] [PATCH] risu-m68k: update fpregs

[Laurent Vivier]

Le 20/02/2017 à 13:14, Peter Maydell a écrit :
> On 19 February 2017 at 20:02, Laurent Vivier <laurent@vivier.eu> wrote:
>> f_fpregs is a 2d array, not 1d:
>>
>>  typedef struct fpregset
>>  {
>>    int f_pcr;
>>    int f_psr;
>>    int f_fpiaddr;
>>  #ifdef __mcoldfire__
>>    int f_fpregs[8][2];
>>  #else
>>    int f_fpregs[8][3];
>>  #endif
>>  } fpregset_t;
>>
>> For the moment, we don't manage ColdFire case, only 680x0.
>>
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> 
> Thanks; applied to target-arm.next.
> 
> I've also pushed out support for building risu into a separate
> build directory, and a 'build-all-archs' script that will
> build every target CPU arch that you have a cross compiler
> installed for (Debian and Ubuntu package cross compilers for
> everything, helpfully).

Thank you.

I have some problems with risugen since some functions have been moved
to common:

$ ./risugen --numinsns 10000 --pattern ABCD m68k.risu ABCD.out
Generating code using patterns: ABCD M68000...
Syntax error detected evaluating ABCD M68000 constraints string:
    ]
{                         write_movb_di($Dx, rand(10) | (rand(10) <<
4));                         write_movb_di($Dy, rand(10) | (rand(10) <<
4));                         1;                      }
Undefined subroutine &risugen_common::write_movb_di called at (eval 5)
line 1.

If I add "risugen_m68k::" to the function name, it works. But is there
better solution to fix that instead of updating all the calls in
m68k.risu for functions found in risugen_m68k.pm?

Thanks,
Laurent

Re: [Qemu-devel] [PATCH] risu-m68k: update fpregs

[Peter Maydell]

On 20 February 2017 at 12:41, Laurent Vivier <laurent@vivier.eu> wrote:
> I have some problems with risugen since some functions have been moved
> to common:
>
> $ ./risugen --numinsns 10000 --pattern ABCD m68k.risu ABCD.out
> Generating code using patterns: ABCD M68000...
> Syntax error detected evaluating ABCD M68000 constraints string:
>     ]
> {                         write_movb_di($Dx, rand(10) | (rand(10) <<
> 4));                         write_movb_di($Dy, rand(10) | (rand(10) <<
> 4));                         1;                      }
> Undefined subroutine &risugen_common::write_movb_di called at (eval 5)
> line 1.
>
> If I add "risugen_m68k::" to the function name, it works. But is there
> better solution to fix that instead of updating all the calls in
> m68k.risu for functions found in risugen_m68k.pm?

Oops, that was unintended. We definitely don't want to require
decorating all the function calls in the .risu files. Reverting
commit 6a3647ae8918 should fix this, but I'll see if there's
a way to avoid the code duplication without the problem with
evaluation happening in the "wrong" module scope.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] risu-m68k: update fpregs

[Laurent Vivier]

Le 20/02/2017 à 13:49, Peter Maydell a écrit :
> On 20 February 2017 at 12:41, Laurent Vivier <laurent@vivier.eu> wrote:
>> I have some problems with risugen since some functions have been moved
>> to common:
>>
>> $ ./risugen --numinsns 10000 --pattern ABCD m68k.risu ABCD.out
>> Generating code using patterns: ABCD M68000...
>> Syntax error detected evaluating ABCD M68000 constraints string:
>>     ]
>> {                         write_movb_di($Dx, rand(10) | (rand(10) <<
>> 4));                         write_movb_di($Dy, rand(10) | (rand(10) <<
>> 4));                         1;                      }
>> Undefined subroutine &risugen_common::write_movb_di called at (eval 5)
>> line 1.
>>
>> If I add "risugen_m68k::" to the function name, it works. But is there
>> better solution to fix that instead of updating all the calls in
>> m68k.risu for functions found in risugen_m68k.pm?
> 
> Oops, that was unintended. We definitely don't want to require
> decorating all the function calls in the .risu files. Reverting
> commit 6a3647ae8918 should fix this, but I'll see if there's
> a way to avoid the code duplication without the problem with
> evaluation happening in the "wrong" module scope.

Reverting 6a3647ae8918 actually fixes the problem.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH] risu-m68k: update fpregs

[Peter Maydell]

On 20 February 2017 at 13:08, Laurent Vivier <laurent@vivier.eu> wrote:
> Le 20/02/2017 à 13:49, Peter Maydell a écrit :
>> Oops, that was unintended. We definitely don't want to require
>> decorating all the function calls in the .risu files. Reverting
>> commit 6a3647ae8918 should fix this, but I'll see if there's
>> a way to avoid the code duplication without the problem with
>> evaluation happening in the "wrong" module scope.
>
> Reverting 6a3647ae8918 actually fixes the problem.

I found a better fix (adding a package statement to the eval'd
string) and pushed it to master.

The ideal solution to this problem would be to tighten up the
way we evaluate strings from risu files, so that they're run
in their own package which only has imported into it the specific
functions that we want to give them access to. That's a bit
beyond my somewhat rudimentary Perl skills though, so I've
settled for just adding a comment to that effect.

thanks
-- PMM