From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4817 invoked by alias); 4 May 2010 21:16:18 -0000 Received: (qmail 4804 invoked by uid 22791); 4 May 2010 21:16:15 -0000 X-SWARE-Spam-Status: No, hits=-0.5 required=5.0 tests=AWL,BAYES_50,TW_RQ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from oarmail.oarcorp.com (HELO OARmail.OARCORP.com) (67.63.146.244) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 04 May 2010 21:16:10 +0000 Received: from localhost.localdomain (216.186.145.118) by OARmail.OARCORP.com (192.168.2.2) with Microsoft SMTP Server id 8.1.436.0; Tue, 4 May 2010 16:16:06 -0500 Message-ID: <4BE08E95.5040500@oarcorp.com> Date: Tue, 04 May 2010 21:16:00 -0000 From: Joel Sherrill User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.9) Gecko/20100330 Fedora/3.0.4-1.fc12 Thunderbird/3.0.4 MIME-Version: 1.0 To: Doug Evans CC: Tiemen Schut , "gdb-patches@sourceware.org" Subject: Re: [patch] sim/erc32/ max simulation time extended by using 64bit ints References: <4BD1BBE3020000520000FC62@pluto.sron.nl> In-Reply-To: Content-Type: multipart/mixed; boundary="------------000705090705010306000005" Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-05/txt/msg00080.txt.bz2 --------------000705090705010306000005 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Content-length: 3691 Attached is V4. Comments interspersed. --joel On 04/23/2010 03:28 PM, Doug Evans wrote: > On Fri, Apr 23, 2010 at 6:25 AM, Tiemen Schut 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. > > I am trying to help address these. > 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. > I have fixed this in interf.c. But didn't touch the other files. Is this OK? I can do a second patch after this is merged if you want VAL killed. > 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? > The ones you are citing above are my fault. I used a very new version of gcc to compile this when checking his patch and it complained. I got near the code and fixed it. Sorry. > 3) > > +#include "stdint.h" > > That should be > Fixed. > 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. > > Fixed. > 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. > > What specifically are you noticing here? I looked around and didn't spot anything too offensive. FWIW Doug I think you have a script which is helping with your ChangeLog entries and it has (or had) a bug. Look at this one from sim/ChangeLog: 2010-02-11 Doug Evans * cris/cpuv10.h, * cris/cpuv32.h, * cris/cris-desc.c, * cris/cris-desc.h, * cris/decodev10.c, * cris/decodev32.c, * cris/modelv10.c, * cris/modelv32.c, * cris/semcrisv10f-switch.c, * cris/semcrisv32f-switch.c: Regenerate. I don't think there should be a ", *" between the files. :-D My helper script has its own oddities. LOL > 6) I'm assuming this change has been well tested. > I've used it for RTEMS testing. --joel RTEMS --------------000705090705010306000005 Content-Type: text/plain; name="sis-V4.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="sis-V4.diff" Content-length: 10187 Index: sim/erc32/ChangeLog =================================================================== RCS file: /cvs/src/src/sim/erc32/ChangeLog,v retrieving revision 1.31 diff -u -r1.31 ChangeLog --- sim/erc32/ChangeLog 14 Apr 2010 07:38:04 -0000 1.31 +++ sim/erc32/ChangeLog 4 May 2010 21:14:55 -0000 @@ -1,3 +1,9 @@ +2010-04-20 Tiemen Schut + + * erc32.c, sis.c, func.c, sis.h, interf.c: Increase max simulation + time by using uint64 for relevant counters. Change prototype + of sis_memory_write() to const unsigned char *. + 2010-04-14 Mike Frysinger * interp.c (sim_write): Add const to buf arg. Index: sim/erc32/erc32.c =================================================================== RCS file: /cvs/src/src/sim/erc32/erc32.c,v retrieving revision 1.2 diff -u -r1.2 erc32.c --- sim/erc32/erc32.c 11 Nov 2008 22:20:54 -0000 1.2 +++ sim/erc32/erc32.c 4 May 2010 21:14:55 -0000 @@ -1860,9 +1860,9 @@ int sis_memory_write(addr, data, length) - uint32 addr; - char *data; - uint32 length; + uint32 addr; + const unsigned char *data; + uint32 length; { char *mem; Index: sim/erc32/func.c =================================================================== RCS file: /cvs/src/src/sim/erc32/func.c,v retrieving revision 1.4 diff -u -r1.4 func.c --- sim/erc32/func.c 8 Jul 2005 08:04:54 -0000 1.4 +++ sim/erc32/func.c 4 May 2010 21:14:55 -0000 @@ -421,7 +421,7 @@ } } else if (strncmp(cmd1, "cont", clen) == 0) { if ((cmd1 = strtok(NULL, " \t\n\r")) == NULL) { - stat = run_sim(sregs, -1, 0); + stat = run_sim(sregs, UINT64_MAX, 0); } else { stat = run_sim(sregs, VAL(cmd1), 0); } @@ -472,7 +472,7 @@ if ((cmd2 = strtok(NULL, " \t\n\r")) != NULL) { stat = run_sim(sregs, VAL(cmd2), 0); } else { - stat = run_sim(sregs, -1, 0); + stat = run_sim(sregs, UINT64_MAX, 0); } daddr = sregs->pc; sim_halt(); @@ -544,7 +544,7 @@ reset_all(); reset_stat(sregs); if ((cmd1 = strtok(NULL, " \t\n\r")) == NULL) { - stat = run_sim(sregs, -1, 0); + stat = run_sim(sregs, UINT64_MAX, 0); } else { stat = run_sim(sregs, VAL(cmd1), 0); } @@ -560,7 +560,7 @@ sim_halt(); } else if (strncmp(cmd1, "tcont", clen) == 0) { sregs->tlimit = limcalc(sregs->freq); - stat = run_sim(sregs, -1, 0); + stat = run_sim(sregs, UINT64_MAX, 0); daddr = sregs->pc; sim_halt(); } else if (strncmp(cmd1, "tgo", clen) == 0) { @@ -573,7 +573,7 @@ sregs->pc = len & ~3; sregs->npc = sregs->pc + 4; printf("resuming at 0x%08x\n",sregs->pc); - stat = run_sim(sregs, -1, 0); + stat = run_sim(sregs, UINT64_MAX, 0); daddr = sregs->pc; sim_halt(); } else if (strncmp(cmd1, "tlimit", clen) == 0) { @@ -583,7 +583,7 @@ sregs->tlimit / sregs->freq / 1000); } else if (strncmp(cmd1, "tra", clen) == 0) { if ((cmd1 = strtok(NULL, " \t\n\r")) == NULL) { - stat = run_sim(sregs, -1, 1); + stat = run_sim(sregs, UINT64_MAX, 1); } else { stat = run_sim(sregs, VAL(cmd1), 1); } @@ -595,7 +595,7 @@ reset_all(); reset_stat(sregs); sregs->tlimit = limcalc(sregs->freq); - stat = run_sim(sregs, -1, 0); + stat = run_sim(sregs, UINT64_MAX, 0); daddr = sregs->pc; sim_halt(); } else @@ -833,7 +833,7 @@ event(cfunc, arg, delta) void (*cfunc) (); int32 arg; - uint32 delta; + uint64 delta; { struct evcell *ev1, *evins; @@ -900,7 +900,8 @@ struct evcell *evrem; void (*cfunc) (); - uint32 arg, endtime; + uint32 arg; + uint64 endtime; #ifdef STAT sregs->fholdt += sregs->fhold; @@ -938,7 +939,8 @@ { struct evcell *evrem; void (*cfunc) (); - int32 arg, endtime; + int32 arg; + uint64 endtime; if (ebase.eq.nxt == NULL) printf("Warning: event queue empty - power-down mode not entered\n"); Index: sim/erc32/interf.c =================================================================== RCS file: /cvs/src/src/sim/erc32/interf.c,v retrieving revision 1.7 diff -u -r1.7 interf.c --- sim/erc32/interf.c 14 Apr 2010 07:38:04 -0000 1.7 +++ sim/erc32/interf.c 4 May 2010 21:14:55 -0000 @@ -37,8 +37,6 @@ #define PSR_CWP 0x7 -#define VAL(x) strtol(x,(char **)NULL,0) - extern struct disassemble_info dinfo; extern struct pstate sregs; extern struct estate ebase; @@ -69,7 +67,7 @@ int run_sim(sregs, icount, dis) struct pstate *sregs; - unsigned int icount; + uint64 icount; int dis; { int mexc, irq; @@ -234,7 +232,7 @@ } else if (strcmp(argv[stat], "-freq") == 0) { if ((stat + 1) < argc) { - freq = VAL(argv[++stat]); + freq = strtol(argv[++stat], (char **)NULL, 0); } } else { (*sim_callback->printf_filtered) (sim_callback, @@ -338,10 +336,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 +347,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)); } @@ -461,7 +459,7 @@ void sim_resume(SIM_DESC sd, int step, int siggnal) { - simstat = run_sim(&sregs, -1, 0); + simstat = run_sim(&sregs, UINT64_MAX, 0); if (sis_gdb_break) flush_windows (); } Index: sim/erc32/sis.c =================================================================== RCS file: /cvs/src/src/sim/erc32/sis.c,v retrieving revision 1.3 diff -u -r1.3 sis.c --- sim/erc32/sis.c 9 Jun 2008 22:55:27 -0000 1.3 +++ sim/erc32/sis.c 4 May 2010 21:14:55 -0000 @@ -82,7 +82,7 @@ int run_sim(sregs, icount, dis) struct pstate *sregs; - unsigned int icount; + uint64 icount; int dis; { int irq, mexc, deb, asi; Index: sim/erc32/sis.h =================================================================== RCS file: /cvs/src/src/sim/erc32/sis.h,v retrieving revision 1.2 diff -u -r1.2 sis.h --- sim/erc32/sis.h 9 Jun 2002 15:45:46 -0000 1.2 +++ sim/erc32/sis.h 4 May 2010 21:14:55 -0000 @@ -23,6 +23,7 @@ #include "ansidecl.h" #include "gdb/callback.h" #include "gdb/remote-sim.h" +#include #include "end.h" @@ -52,8 +53,8 @@ typedef double float64; /* 64-bit float */ /* FIXME: what about host compilers that don't support 64-bit ints? */ -typedef unsigned long long uint64; /* 64-bit unsigned int */ -typedef long long int64; /* 64-bit signed int */ +typedef uint64_t uint64; /* 64-bit unsigned int */ +typedef int64_t int64; /* 64-bit signed int */ struct pstate { @@ -108,22 +109,22 @@ float32 freq; /* Simulated processor frequency */ - uint32 tottime; - uint32 ninst; - uint32 fholdt; - uint32 holdt; - uint32 icntt; - uint32 finst; - uint32 simstart; - uint32 starttime; - uint32 tlimit; /* Simulation time limit */ - uint32 pwdtime; /* Cycles in power-down mode */ - uint32 nstore; /* Number of load instructions */ - uint32 nload; /* Number of store instructions */ - uint32 nannul; /* Number of annuled instructions */ - uint32 nbranch; /* Number of branch instructions */ + uint64 tottime; + uint64 ninst; + uint64 fholdt; + uint64 holdt; + uint64 icntt; + uint64 finst; + uint64 simstart; + uint64 starttime; + uint64 tlimit; /* Simulation time limit */ + uint64 pwdtime; /* Cycles in power-down mode */ + uint64 nstore; /* Number of load instructions */ + uint64 nload; /* Number of store instructions */ + uint64 nannul; /* Number of annuled instructions */ + uint64 nbranch; /* Number of branch instructions */ uint32 ildreg; /* Destination of last load instruction */ - uint32 ildtime; /* Last time point for load dependency */ + uint64 ildtime; /* Last time point for load dependency */ int rett_err; /* IU in jmpl/restore error state (Rev.0) */ int jmpltime; @@ -132,14 +133,14 @@ struct evcell { void (*cfunc) (); int32 arg; - uint32 time; + uint64 time; struct evcell *nxt; }; struct estate { struct evcell eq; struct evcell *freeq; - uint32 simtime; + uint64 simtime; }; struct irqcell { @@ -168,8 +169,8 @@ int32 sz, int32 *ws)); extern int memory_write PARAMS ((int32 asi, uint32 addr, uint32 *data, int32 sz, int32 *ws)); -extern int sis_memory_write PARAMS ((uint32 addr, char *data, - uint32 length)); +extern int sis_memory_write PARAMS ((uint32 addr, + const unsigned char *data, uint32 length)); extern int sis_memory_read PARAMS ((uint32 addr, char *data, uint32 length)); @@ -186,7 +187,7 @@ struct disassemble_info; extern void dis_mem PARAMS ((uint32 addr, uint32 len, struct disassemble_info *info)); -extern void event PARAMS ((void (*cfunc) (), int32 arg, uint32 delta)); +extern void event PARAMS ((void (*cfunc) (), int32 arg, uint64 delta)); extern void set_int PARAMS ((int32 level, void (*callback) (), int32 arg)); extern void advance_time PARAMS ((struct pstate *sregs)); extern uint32 now PARAMS ((void)); @@ -205,7 +206,7 @@ /* interf.c */ extern int run_sim PARAMS ((struct pstate *sregs, - unsigned int icount, int dis)); + uint64 icount, int dis)); /* float.c */ extern int get_accex PARAMS ((void)); --------------000705090705010306000005--