[patch] fix profiling in glibc for Linux/MIPS

[patch] fix profiling in glibc for Linux/MIPS

[Daniel Jacobowitz]

_mcount was doing awful things to its caller's stack frame.

Theoretically, we can get by with 16 bytes less of stack than I now
allocate, but GCC still considers functions that call _mcount to be leaf
functions, so that doesn't work.  I think this is close enough; it only adds
one instruction.  Is this OK?  Do I need a "nop" after the subu?  My MIPS
assembly knowledge is not that thorough.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

--- glibc-2.2.3/sysdeps/mips/machine-gmon.h.orig	Wed Jul 25 20:09:20 2001
+++ glibc-2.2.3/sysdeps/mips/machine-gmon.h	Wed Jul 25 21:14:09 2001
@@ -35,22 +35,23 @@
         ".set noreorder;" \
         ".set noat;" \
         CPLOAD \
-        "sw $4,8($29);" \
-        "sw $5,12($29);" \
-        "sw $6,16($29);" \
-        "sw $7,20($29);" \
-        "sw $1,0($29);" \
-        "sw $31,4($29);" \
+	"subu $29,$29,40;" \
+        "sw $4,24($29);" \
+        "sw $5,28($29);" \
+        "sw $6,32($29);" \
+        "sw $7,36($29);" \
+        "sw $1,16($29);" \
+        "sw $31,20($29);" \
         "move $5,$31;" \
         "jal __mcount;" \
         "move $4,$1;" \
-        "lw $4,8($29);" \
-        "lw $5,12($29);" \
-        "lw $6,16($29);" \
-        "lw $7,20($29);" \
-        "lw $31,4($29);" \
-        "lw $1,0($29);" \
-        "addu $29,$29,8;" \
+        "lw $4,24($29);" \
+        "lw $5,28($29);" \
+        "lw $6,32($29);" \
+        "lw $7,36($29);" \
+        "lw $31,20($29);" \
+        "lw $1,16($29);" \
+        "addu $29,$29,48;" \
         "j $31;" \
         "move $31,$1;" \
         ".set reorder;" \

Re: [patch] fix profiling in glibc for Linux/MIPS

[Thiemo Seufer]

Daniel Jacobowitz wrote:
> _mcount was doing awful things to its caller's stack frame.

Maybe I'm missing something, but both the old and the new code
add 8 byte more to sp than they subtracted before. How is this
supposed to work?

[snip]
> I think this is close enough; it only adds
> one instruction.  Is this OK?

Why do you save and restore $6, $7, seemingly without using them?

> Do I need a "nop" after the subu?

It works here since it is expanded in an

	addiu $29,$29,-40

which is executed in one cycle. The usual suspects for hazards
to be NOPed are load/store insns and branches.

[snip]
> +        "sw $31,20($29);" \
>          "move $5,$31;" \
>          "jal __mcount;" \
>          "move $4,$1;" \
            ^
Some stylistic issue: In ".set noreorder" assembly it helps to
indent the insns in a branch delay slot by one blank to avoid
confusion about their non-sequential nature.


Thiemo

Re: [patch] fix profiling in glibc for Linux/MIPS

[Daniel Jacobowitz]

On Fri, Jul 27, 2001 at 02:48:20AM +0200, Thiemo Seufer wrote:
> Daniel Jacobowitz wrote:
> > _mcount was doing awful things to its caller's stack frame.
> 
> Maybe I'm missing something, but both the old and the new code
> add 8 byte more to sp than they subtracted before. How is this
> supposed to work?

It's supposed to do that, according to GCC.  Build something with -S
-pg and look at it.

> [snip]
> > I think this is close enough; it only adds
> > one instruction.  Is this OK?
> 
> Why do you save and restore $6, $7, seemingly without using them?

Because they were already there; I was trying to keep this patch
minimal.  My MIPS assembly knowledge, as I said, is a little scanty.

> > Do I need a "nop" after the subu?
> 
> It works here since it is expanded in an
> 
> 	addiu $29,$29,-40
> 
> which is executed in one cycle. The usual suspects for hazards
> to be NOPed are load/store insns and branches.

Ahh, OK.  I see.

> [snip]
> > +        "sw $31,20($29);" \
> >          "move $5,$31;" \
> >          "jal __mcount;" \
> >          "move $4,$1;" \
>             ^
> Some stylistic issue: In ".set noreorder" assembly it helps to
> indent the insns in a branch delay slot by one blank to avoid
> confusion about their non-sequential nature.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [patch] fix profiling in glibc for Linux/MIPS

[Thiemo Seufer]

Daniel Jacobowitz wrote:
[snip]
> > Maybe I'm missing something, but both the old and the new code
> > add 8 byte more to sp than they subtracted before. How is this
> > supposed to work?
> 
> It's supposed to do that, according to GCC.  Build something with -S
> -pg and look at it.

Well, I don't have a 32bit compiler here ATM, only a highly
experimental 64bit one. :-)  But I found in the GCC Code this
snippet in /config/mips.mips.h:

/* Output assembler code to FILE to increment profiler label # LABELNO
   for profiling a function entry.  */
   
#define FUNCTION_PROFILER(FILE, LABELNO)                                \
{                                                                       \
  if (TARGET_MIPS16)                                                    \
    sorry ("mips16 function profiling");                                \
  fprintf (FILE, "\t.set\tnoreorder\n");                                \
  fprintf (FILE, "\t.set\tnoat\n");                                     \
  fprintf (FILE, "\tmove\t%s,%s\t\t# save current return address\n",    \
           reg_names[GP_REG_FIRST + 1], reg_names[GP_REG_FIRST + 31]);  \
  fprintf (FILE, "\tjal\t_mcount\n");                                   \
  fprintf (FILE,                                                        \
           "\t%s\t%s,%s,%d\t\t# _mcount pops 2 words from  stack\n",    \
           TARGET_64BIT ? "dsubu" : "subu",                             \
           reg_names[STACK_POINTER_REGNUM],                             \
           reg_names[STACK_POINTER_REGNUM],                             \
           Pmode == DImode ? 16 : 8);                                   \
  fprintf (FILE, "\t.set\treorder\n");                                  \
  fprintf (FILE, "\t.set\tat\n");                                       \
}

This means, 8 byte is indeed ok for 32bit targets, a 64bit one
would differ by 16 byte (and won't work with the code you've
changed anyway).

Nevertheless, IHMO it would be a good idea to support both targets.

[snip]
> > Why do you save and restore $6, $7, seemingly without using them?
> 
> Because they were already there; I was trying to keep this patch
> minimal.  My MIPS assembly knowledge, as I said, is a little scanty.

Hm, and I have too little knowledge about the profiler to give
helpful advice here.


Thiemo

Re: [patch] fix profiling in glibc for Linux/MIPS

[Ralf Baechle]

On Fri, Jul 27, 2001 at 02:48:20AM +0200, Thiemo Seufer wrote:
> Date: Fri, 27 Jul 2001 02:48:20 +0200
> To: linux-mips@oss.sgi.com
> Subject: Re: [patch] fix profiling in glibc for Linux/MIPS
> From: Thiemo Seufer <ica2_ts@csv.ica.uni-stuttgart.de>
> 
> Daniel Jacobowitz wrote:
> > _mcount was doing awful things to its caller's stack frame.
> 
> Maybe I'm missing something, but both the old and the new code
> add 8 byte more to sp than they subtracted before. How is this
> supposed to work?

_mcount has some odd special calling convention.  I don't recall any official
standard that defines _mcount's calling convention but gcc uses it the
same way as the proprietary compiler I tried.

  Ralf