ARM sim patch: increase default target memory

ARM sim patch: increase default target memory

[Anthony Green]

This patch increases the default target memory for the ARM sim from 2
to 8MB.  2MB isn't quite enough for the gcj testsuite.  8MB appears
to be more than enough.  We can get through the xscale-elf-gcj testsuite
quite easily with this change.

Ok?



2002-03-17  Anthony Green  <green@redhat.com>

	* wrapper.c (mem_size): Increase the default target memory to 8MB.

Index: sim/arm/wrapper.c
===================================================================
RCS file: /cvs/src/src/sim/arm/wrapper.c,v
retrieving revision 1.17
diff -u -p -r1.17 wrapper.c
--- wrapper.c	2002/02/05 11:22:26	1.17
+++ wrapper.c	2002/03/17 16:40:34
@@ -47,7 +47,7 @@ static SIM_OPEN_KIND sim_kind;
 static char *myname;
 
 /* Memory size in bytes.  */
-static int mem_size = (1 << 21);
+static int mem_size = (1 << 23);
 
 /* Non-zero to display start up banner, and maybe other things.  */
 static int verbosity;

Re: ARM sim patch: increase default target memory

[Andrew Cagney]

> This patch increases the default target memory for the ARM sim from 2
> to 8MB.  2MB isn't quite enough for the gcj testsuite.  8MB appears
> to be more than enough.  We can get through the xscale-elf-gcj testsuite
> quite easily with this change.
> 
> Ok?
> 
> 
> 
> 2002-03-17  Anthony Green  <green@redhat.com>
> 
> * wrapper.c (mem_size): Increase the default target memory to 8MB.
> 

Is there a command line option that you can specify?

Andrew

Re: ARM sim patch: increase default target memory

[Anthony Green]

On Sun, 2002-03-17 at 09:55, Nick Clifton wrote:
> > 2002-03-17  Anthony Green  <green@redhat.com>
> > 
> > 	* wrapper.c (mem_size): Increase the default target memory to 8MB.
> 
> This is fine - Approved.

Thanks Nick.  I checked this in.

BTW - gdb people - I see there is a 5.2 branch.  I assume there may be
some affinity between GCC 3.1 and GDB 5.2.  Would it be OK to move my
patch to the 5.2 branch?

Thanks,

AG

Re: ARM sim patch: increase default target memory

[Andrew Cagney]

> On Sun, 2002-03-17 at 09:55, Nick Clifton wrote:
> 
>> > 2002-03-17  Anthony Green  <green@redhat.com>
>> > 
>> > * wrapper.c (mem_size): Increase the default target memory to 8MB.
> 
>> 
>> This is fine - Approved.

Er, wasn't this going to be properly - add -m to sim_open()?

Andrew


> Thanks Nick.  I checked this in.
> 
> BTW - gdb people - I see there is a 5.2 branch.  I assume there may be
> some affinity between GCC 3.1 and GDB 5.2.  Would it be OK to move my
> patch to the 5.2 branch?
> 
> Thanks,
> 
> AG
> 
> 
> 
> 

Re: ARM sim patch: increase default target memory

[Anthony Green]

On Mon, 2002-03-18 at 17:30, Andrew Cagney wrote:
> Er, wasn't this going to be properly - add -m to sim_open()?

You're implying that the default value must remain 2MB.  Isn't this a
somewhat arbitrary value?

I filed a case against the command line bug.  This seems like a
completely different issue.

AG

Re: ARM sim patch: increase default target memory

[Andrew Cagney]

> On Mon, 2002-03-18 at 17:30, Andrew Cagney wrote:
> 
>> Er, wasn't this going to be properly - add -m to sim_open()?
> 
> 
> You're implying that the default value must remain 2MB.  Isn't this a
> somewhat arbitrary value?

The number is a compromise between a fast simulator startup, sufficient 
memory for a typical simulation and unnecessary VM grab.  It was also 
found to be sufficient for the basic GDB and GCC tests.

 From memory, he Java tests run on the MIPS (and PPC?) simulators and 
yet there haven't been problems.  The MIPS defaults to 2mb, the PPC 1mb.

> I filed a case against the command line bug.  This seems like a
> completely different issue.

I don't think that re-compiling GDB is the correct way for a user to 
change the size of simulator memory.  Instead the user should be able to 
fix it at run time.  I think that is the real bug here.

enjoy,
Andrew

Re: ARM sim patch: increase default target memory

[Anthony Green]

On Tue, 2002-03-19 at 07:46, Andrew Cagney wrote:
> The number is a compromise between a fast simulator startup, sufficient 
> memory for a typical simulation and unnecessary VM grab.  It was also 
> found to be sufficient for the basic GDB and GCC tests.

Unfortunately this isn't true anymore.  gcj is part of GCC and 2MB is
not enough space.

>  From memory, he Java tests run on the MIPS (and PPC?) simulators and 
> yet there haven't been problems.  The MIPS defaults to 2mb, the PPC 1mb.

(just MIPS)  The default runtime requirements have grown since the early
days.  8MB appears to be a very comfortable amount of space, although
with some experimentation this could probably be brought down.

> I don't think that re-compiling GDB is the correct way for a user to 
> change the size of simulator memory.  Instead the user should be able to 
> fix it at run time.  I think that is the real bug here.

Yes, I agree that this is a bug, which is why I filed a case against it.

AG

Re: ARM sim patch: increase default target memory

[Andrew Cagney]

> On Tue, 2002-03-19 at 07:46, Andrew Cagney wrote:
> 
>> The number is a compromise between a fast simulator startup, sufficient 
>> memory for a typical simulation and unnecessary VM grab.  It was also 
>> found to be sufficient for the basic GDB and GCC tests.
> 
> 
> Unfortunately this isn't true anymore.  gcj is part of GCC and 2MB is
> not enough space.

As I said a compromise for the ``basic'' gcc tests.  I recall this value 
being put up once before and people complaining that the basic tests 
slowed down and GDB grabbed too much memory.

>> From memory, he Java tests run on the MIPS (and PPC?) simulators and 
>> yet there haven't been problems.  The MIPS defaults to 2mb, the PPC 1mb.
> 
> 
> (just MIPS)  The default runtime requirements have grown since the early
> days.  8MB appears to be a very comfortable amount of space, although
> with some experimentation this could probably be brought down.
> 
> 
>> I don't think that re-compiling GDB is the correct way for a user to 
>> change the size of simulator memory.  Instead the user should be able to 
>> fix it at run time.  I think that is the real bug here.
> 
> 
> Yes, I agree that this is a bug, which is why I filed a case against it.

We're going to degenerate into semantics :-)

Arm can't change its memory size at run time.  This is the bug that hurt 
you - you had to change a compile time constant to fix your problem and 
that simply shouldn't have been necessary.  Remember, the other 
simulators don't have this problem as -m<blah> can be used.

As a separate issue, it is probably time to review the default memory 
size for all the simulators.  Should they all be increased (to again be 
consistent)?  Should the GDB builder be allowed to 
--enable-sim-memory-size=BLAH this?

Andrew

Re: ARM sim patch: increase default target memory

[Andrew Cagney]

> On Tue, 2002-03-19 at 07:46, Andrew Cagney wrote:
> 
> The number is a compromise between a fast simulator startup, sufficient memory for a typical simulation and unnecessary VM grab.  It was also found to be sufficient for the basic GDB and GCC tests.
> 
> 
> Unfortunately this isn't true anymore.  gcj is part of GCC and 2MB is
> not enough space.
> 
> As I said a compromise for the ``basic'' gcc tests.  I recall this value being put up once before and people complaining that the basic tests slowed down and GDB grabbed too much memory.

Hmm, perhaphs a better way of looking at this is to ask where the line 
should be drawn.

C? Fortran? C++? Java? Ada?

I dread the thought of hardwireing this to 32 mb (:-^) just so that 
standalone C++ or Ada programs can be run without the -m flag.

I suspect C might be a good compromise :-)

enjoy,
Andrew

[rfa] Add -m; Was: ARM sim patch: increase default target memory

[Andrew Cagney]

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

Hello,

The attatched patch (er, hack), adds support for the -m flag from the 
GDB command line (vis (gdb) target sim -m<size>).  It also sets the 
default memory size back to something consistent with the other simulators.

ok?
Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 1412 bytes --]

2002-04-07  Andrew Cagney  <ac131313@redhat.com>

	* wrapper.c (sim_open): Add support for -m<mem-size>.
	(mem_size): Reduce to 2MB.
	Fix PR gdb/433.

Index: wrapper.c
===================================================================
RCS file: /cvs/src/src/sim/arm/wrapper.c,v
retrieving revision 1.18
diff -u -r1.18 wrapper.c
--- wrapper.c	18 Mar 2002 21:43:15 -0000	1.18
+++ wrapper.c	7 Apr 2002 16:59:18 -0000
@@ -47,7 +47,7 @@
 static char *myname;
 
 /* Memory size in bytes.  */
-static int mem_size = (1 << 23);
+static int mem_size = (1 << 21);
 
 /* Non-zero to display start up banner, and maybe other things.  */
 static int verbosity;
@@ -448,7 +448,7 @@
     {
       int i;
 
-      /* Scan for endian-ness switch.  */
+      /* Scan for endian-ness and memory-size switches.  */
       for (i = 0; (argv[i] != NULL) && (argv[i][0] != 0); i++)
 	if (argv[i][0] == '-' && argv[i][1] == 'E')
 	  {
@@ -482,6 +482,23 @@
 		  (sim_callback, "Unrecognised argument to -E option\n");
 		break;
 	      }
+	  }
+	else if (argv[i][0] == '-' && argv[i][1] == 'm')
+	  {
+	    if (argv[i][2] != '\0')
+	      sim_size (atoi (&argv[i][2]));
+	    else if (argv[i + 1] != NULL)
+	      {
+		sim_size (atoi (argv[i + 1]));
+		i++;
+	      }
+	    else
+	      {
+		sim_callback->printf_filtered (sim_callback,
+					       "Missing argument to -m option\n");
+		return NULL;
+	      }
+	      
 	  }
     }
 

Re: [rfa] Add -m; Was: ARM sim patch: increase default target memory

[Nick Clifton]

Hi Andrew,

> The attatched patch (er, hack), adds support for the -m flag from
> the GDB command line (vis (gdb) target sim -m<size>).

Hmm, this looks generally OK to me, except that I would recommend
using "strtoul" instead of "atoi" to convert the string into a
number, so that hex values can be specified as well as decimal.  I
would also suggest that you consider adding support for a megabyte
postfix so that the user could do:

        -m 6M

to set up a 6 megabyte virtual memory, instead of having to type:

        -m 6291456

> It also sets the default memory size back to something consistent
> with the other simulators.

This will stop the java tests from running, so maybe you should also
add the new switch to the java test harness so that they do not fail.

Cheers
        Nick

Re: [rfa] Add -m; Was: ARM sim patch: increase default target memory

[Andrew Cagney]

> Hi Andrew,
> 
> 
>> The attatched patch (er, hack), adds support for the -m flag from
>> the GDB command line (vis (gdb) target sim -m<size>).
> 
> 
> Hmm, this looks generally OK to me, except that I would recommend
> using "strtoul" instead of "atoi" to convert the string into a
> number, so that hex values can be specified as well as decimal. 

Good point.

 > I
> would also suggest that you consider adding support for a megabyte
> postfix so that the user could do:
> 
>         -m 6M

Good idea, all the sims should do this.  Any pointers for the code? 
I'll likely bug report it for now.

> to set up a 6 megabyte virtual memory, instead of having to type:
> 
>         -m 6291456
> 
> 
>> It also sets the default memory size back to something consistent
>> with the other simulators.
> 
> 
> This will stop the java tests from running, so maybe you should also
> add the new switch to the java test harness so that they do not fail.

Do you know which file?

Andrew

Re: [rfa] Add -m; Was: ARM sim patch: increase default target memory

[Nick Clifton]

Hi Andrew,

> > I would also suggest that you consider adding support for a
> > megabyte  postfix so that the user could do:
> >         -m 6M
> 
> Good idea, all the sims should do this.  Any pointers for the code?

Nope - it was just an idea that popped into my head whilst reading
your code.  It should be very straightforward though.  Something like
the code attached to the end of this email maybe ?

> >> It also sets the default memory size back to something consistent
> >> with the other simulators.
> > This will stop the java tests from running, so maybe you should also
> > add the new switch to the java test harness so that they do not fail.
> 
> Do you know which file?

Umm, not sure - libjava.exp maybe ?  Anthony ?

Cheers
        Nick

-------------------------------------------------------------------------
  else if (argv[i][0] == '-' && argv[i][1] == 'm')
    {
      char * end_ptr;
      const char * ptr;
      unsigned long size;
	
      if (argv[i][2] != '\0')
	ptr = argv[i] + 2;
      else
	{
	  ptr = argv [++i];

	  if (ptr == NULL)
	    {
	      sim_callback->printf_filtered
		(sim_callback,
		 "Missing argument to -m option\n");
	      return NULL;
	    }
	}

      size = strtoul (ptr, & end_ptr, 0);
      if (end_ptr != ptr)
	{
	  switch (* end_ptr)
	    {
	    case 'k':
	    case 'K':
	      size *= 1024;
	      break;
	    case 'm':
	    case 'M':
	      size *= 1024 * 1024;
	      break;
	    case 'g':
	    case 'G':
	      size *= 1024 * 1024 * 1024;
	      break;
	    case 0:
	      break;
	    default:
	      sim_callback->printf_filtered
		(sim_callback,
		 "Unexpected characters at end of -m option\n");
	      break;
	    }
	}

      sim_size (size);
    }

Re: [rfa] Add -m; Was: ARM sim patch: increase default target memory

[Andrew Cagney]

> Hi Andrew,
> 
> 
>> > I would also suggest that you consider adding support for a
>> > megabyte  postfix so that the user could do:
>> >         -m 6M
> 
>> 
>> Good idea, all the sims should do this.  Any pointers for the code?
> 
> 
> Nope - it was just an idea that popped into my head whilst reading
> your code.  It should be very straightforward though.  Something like
> the code attached to the end of this email maybe ?

Given I've not got to this I've created a bug report.

The original ``-m'' fix is committed.

Andrew


>> >> It also sets the default memory size back to something consistent
>> >> with the other simulators.
> 
>> > This will stop the java tests from running, so maybe you should also
>> > add the new switch to the java test harness so that they do not fail.
> 
>> 
>> Do you know which file?
> 
> 
> Umm, not sure - libjava.exp maybe ?  Anthony ?
> 
> Cheers
>         Nick
> 
> -------------------------------------------------------------------------
>   else if (argv[i][0] == '-' && argv[i][1] == 'm')
>     {
>       char * end_ptr;
>       const char * ptr;
>       unsigned long size;
> 	
>       if (argv[i][2] != '\0')
> 	ptr = argv[i] + 2;
>       else
> 	{
> 	  ptr = argv [++i];
> 
> 	  if (ptr == NULL)
> 	    {
> 	      sim_callback->printf_filtered
> 		(sim_callback,
> 		 "Missing argument to -m option\n");
> 	      return NULL;
> 	    }
> 	}
> 
>       size = strtoul (ptr, & end_ptr, 0);
>       if (end_ptr != ptr)
> 	{
> 	  switch (* end_ptr)
> 	    {
> 	    case 'k':
> 	    case 'K':
> 	      size *= 1024;
> 	      break;
> 	    case 'm':
> 	    case 'M':
> 	      size *= 1024 * 1024;
> 	      break;
> 	    case 'g':
> 	    case 'G':
> 	      size *= 1024 * 1024 * 1024;
> 	      break;
> 	    case 0:
> 	      break;
> 	    default:
> 	      sim_callback->printf_filtered
> 		(sim_callback,
> 		 "Unexpected characters at end of -m option\n");
> 	      break;
> 	    }
> 	}
> 
>       sim_size (size);
>     }
> 
>