Re: [patch] sim/erc32/ max simulation time extended by using 64bit ints

[Doug Evans <dje at google dot com>]

On Fri, Apr 23, 2010 at 6:25 AM, Tiemen Schut <T.Schut@sron.nl> wrote:
> Hey all,
>
> This patch solves the problem that the sparc instruction simulator (SIS) would hang after a few minutes of simulation time (time depending on speed of host pc), because of the use of 32 bit counters internally.
>
> This patch doesn't change anything to simulator behaviour, except that it allows for longer simulation times.
>
> There may be a problem with the use of 64 bit integers, but that was also there before this patch.
>
> Thanks,
>
> Tiemen Schut

Hi.

The patch is ok with me, with a few changes.
I'd leave it a week to see if anyone else has something to say.

1)

-#define	VAL(x)	strtol(x,(char **)NULL,0)
+#define	VAL(x)	strtoull(x,(char **)NULL,0)

I realize VAL is only used once in interf.c but it's also defined in
other files as well.
While one could consolidate them, having the macro at all is probably
less preferable to just calling strtoul{,l} directly.
I would just remove it from interf.c and call strtoull directly.

2)

@@ -338,10 +338,10 @@

 int
 sim_fetch_register(sd, regno, buf, length)
-     SIM_DESC sd;
-    int             regno;
-    unsigned char  *buf;
-     int length;
+    SIM_DESC sd;
+    int regno;
+    unsigned char *buf;
+    int length;
 {
     get_regi(&sregs, regno, buf);
     return -1;
@@ -349,10 +349,10 @@

 int
 sim_write(sd, mem, buf, length)
-     SIM_DESC sd;
-    SIM_ADDR             mem;
+    SIM_DESC sd;
+    SIM_ADDR mem;
     const unsigned char  *buf;
-    int             length;
+    int length;
 {
     return (sis_memory_write(mem, buf, length));
 }

Generally, formatting changes/fixes should be separate.  I noticed a
few, can you remove them?

3)

+#include "stdint.h"

That should be <stdint.h>

4)

-    uint32          ildreg;	/* Destination of last load instruction */
+    uint64          ildreg;	/* Destination of last load instruction */

No point in making this uint64, leave it as uint32.

5)

+	* sis.c, func.c, sis.h, interf.c: Increase max simulation time
+	by using uint64 for relevant counters.

I realize sim/erc32/ChangeLog doesn't always follow the GNU
conventions for ChangeLog entries - it's software obtained from
elsewhere.  It's ok with me to leave as is, but I defer to someone
else with an opinion.

6) I'm assuming this change has been well tested.