[RFC] frv-tdep.c: Use refine_prologue_limit() instead of skip_prologue_using_sal()

[RFC] frv-tdep.c: Use refine_prologue_limit() instead of skip_prologue_using_sal()

[Kevin Buettner]

On FR-V, I've encountered an optimized prologue in which
skip_prologue_using_sal() fails to making it past the first
prologue -> body transition.  The prologue in question is not all that
unusual (for an optimized prologue) which leads me to wonder about the
efficacy of skip_prologue_using_sal() with regard to optimized
prologues in general.  I've read the comments and code in
skip_prologue_using_sal(), however, and it does seem to be operating
as intended.

That said, it seems like a shame to introduce yet another private copy
of refine_prologue_limit().  (Though I was careful to keep this one
the same as that defined in rs6000-tdep.c.  The IA-64 version differs
slightly.)  Perhaps it's time to move refine_prologue_limit to
symtab.c?  Or perhaps skip_prologue_using_sal() should be revised
somewhat so that it provides more useful results?

Comments?

	* frv-tdep.c (max_skip_non_prologue_insns): New static global.
	(refine_prologue_limit): New function copied verbatim from
	rs6000-tdep.c.
	(frv_analyze_prologue): Use refine_prologue_limit() instead of
	skip_prologue_using_sal().

Index: frv-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/frv-tdep.c,v
retrieving revision 1.87
diff -u -p -r1.87 frv-tdep.c
--- frv-tdep.c	20 Jul 2004 19:45:05 -0000	1.87
+++ frv-tdep.c	30 Jul 2004 21:25:10 -0000
@@ -504,6 +504,64 @@ is_argument_reg (int reg)
   return (8 <= reg && reg <= 13);
 }
 
+/* Limit the number of skipped non-prologue instructions, as the examining
+   of the prologue is expensive.  */
+static int max_skip_non_prologue_insns = 10;
+
+/* Given PC representing the starting address of a function, and
+   LIM_PC which is the (sloppy) limit to which to scan when looking
+   for a prologue, attempt to further refine this limit by using
+   the line data in the symbol table.  If successful, a better guess
+   on where the prologue ends is returned, otherwise the previous
+   value of lim_pc is returned.  */
+
+/* Note: cagney/2004-02-14: This function and logic have largely been
+   superseded by skip_prologue_using_sal.  
+   Note: kevinb/2004-07-29: Not true.  On FR-V, I've encountered a
+   case where skip_prologue_using_sal() doesn't make it past the
+   first prologue -> body transition.  */
+
+static CORE_ADDR
+refine_prologue_limit (CORE_ADDR pc, CORE_ADDR lim_pc)
+{
+  struct symtab_and_line prologue_sal;
+
+  prologue_sal = find_pc_line (pc, 0);
+  if (prologue_sal.line != 0)
+    {
+      int i;
+      CORE_ADDR addr = prologue_sal.end;
+
+      /* Handle the case in which compiler's optimizer/scheduler
+         has moved instructions into the prologue.  We scan ahead
+	 in the function looking for address ranges whose corresponding
+	 line number is less than or equal to the first one that we
+	 found for the function.  (It can be less than when the
+	 scheduler puts a body instruction before the first prologue
+	 instruction.)  */
+      for (i = 2 * max_skip_non_prologue_insns; 
+           i > 0 && (lim_pc == 0 || addr < lim_pc);
+	   i--)
+        {
+	  struct symtab_and_line sal;
+
+	  sal = find_pc_line (addr, 0);
+	  if (sal.line == 0)
+	    break;
+	  if (sal.line <= prologue_sal.line 
+	      && sal.symtab == prologue_sal.symtab)
+	    {
+	      prologue_sal = sal;
+	    }
+	  addr = sal.end;
+	}
+
+      if (lim_pc == 0 || prologue_sal.end < lim_pc)
+	lim_pc = prologue_sal.end;
+    }
+  return lim_pc;
+}
+
 /* Scan an FR-V prologue, starting at PC, until frame->PC.
    If FRAME is non-zero, fill in its saved_regs with appropriate addresses.
    We assume FRAME's saved_regs array has already been allocated and cleared.
@@ -573,17 +631,18 @@ frv_analyze_prologue (CORE_ADDR pc, stru
 
   last_prologue_pc = pc;
 
-  /* Try to compute an upper limit (on how far to scan) based on the
-     line number info.  */
-  lim_pc = skip_prologue_using_sal (pc);
-  /* If there's no line number info, lim_pc will be 0.  In that case,
-     set the limit to be 100 instructions away from pc.  Hopefully, this
-     will be far enough away to account for the entire prologue.  Don't
-     worry about overshooting the end of the function.  The scan loop
-     below contains some checks to avoid scanning unreasonably far.  */
+  /* Set the initial limit to be 100 instructions away from pc. 
+     Hopefully, this will be far enough away to account for the entire
+     prologue.  Don't worry about overshooting the end of the
+     function.  The scan loop below contains some checks to avoid
+     scanning unreasonably far.  */
   if (lim_pc == 0)
     lim_pc = pc + 400;
 
+  /* Attempt to refine (reduce) the limit based on available line number
+     info.  */
+  lim_pc = refine_prologue_limit (pc, lim_pc);
+
   /* If we have a frame, we don't want to scan past the frame's pc.  This
      will catch those cases where the pc is in the prologue.  */
   if (next_frame)

Re: [RFC] frv-tdep.c: Use refine_prologue_limit() instead of skip_prologue_using_sal()

[Andrew Cagney]

> +   Note: kevinb/2004-07-29: Not true.  On FR-V, I've encountered a
> +   case where skip_prologue_using_sal() doesn't make it past the
> +   first prologue -> body transition.  */

The rs6000 contains this:
   if (lim_pc == 0)
     lim_pc = refine_prologue_limit (pc, lim_pc);
which I believe scores as a ``Not not true'' :-)

More seriously, what's the case you've encountered?

GDB should be using the dwarf2 debug info when setting the prologue 
breakpoint[s], how hard is it to do that?

Andrew

PS: I'd keep the two comments separate, M-w or indent will merge them :-)

Re: [RFC] frv-tdep.c: Use refine_prologue_limit() instead of skip_prologue_using_sal()

[Kevin Buettner]

On Sat, 31 Jul 2004 13:37:55 -0400
Andrew Cagney <cagney@gnu.org> wrote:

> More seriously, what's the case you've encountered?

It's pthread_start_thread() from glibc.  Here's the code split up into
SAL units with the line number at the right.  (I synthesized this by hand
from "x/i" and "info line" output.  I can provide you with the raw data
if you want...)

Code                                            Line Number     Ref
---------------------------                     -----------     ---
0x2e020  addi sp,-200,sp                        256             A
0x2e024  sti.p fp,@(sp,184)

0x2e028  sethi 0xffff,gr4                       273             B

0x2e02c  addi.p sp,184,fp                       256             C

0x2e030  setlo 0xfd80,gr4                       273             D

0x2e034  movsg lr,gr5                           256             E
0x2e038  stdi.p gr18,@(sp,0)
0x2e03c  ori gr15,0,gr18
0x2e040  sti.p gr5,@(fp,8)

0x2e044  ori gr8,0,gr29                         265             F

0x2e048  stdi.p gr20,@(sp,8)                    256             G
0x2e04c  ori gr8,0,gr19
0x2e050  stdi gr22,@(sp,16)

0x2e054  ldd @(gr4,gr15),gr14                   273             H
0x2e058  calll @(gr14,gr0)

0x2e05c  sethi.p 0xffff,gr4                     276             I
0x2e060  setlo 0xfb20,gr4

0x2e064  sti.p gr8,@(gr19,84)                   273             J

0x2e068  addi gr19,148,gr9                      276             K
0x2e06c  ldd.p @(gr4,gr18),gr14
0x2e070  setlos 0x2,gr8
0x2e074  calll.p @(gr14,gr0)
0x2e078  setlos lo(0x0),gr10

The last prologue SAL is marked "G" in the "Ref" column.

When these SALs are scanned using skip_prologue_using_sal(),
prologue_sal will be initialized to SAL "A".  When the loop
is entered, ``sal'' (local to the loop) is set to "B".  The
following test causes the loop to terminate on the first
iteration:

          if (sal.line >= prologue_sal.line)
            break;

Here, sal.line is 273 and prologue_sal.line is 256.

Thus, skip_prologue_using_sal() returns the end address corresponding
to SAL "A" (which is actually 0x2e028 since the end address is actually
the start address for the next SAL).

By way of contrast, refine_prologue_limit() starts out the same way
initializing ``prologue_sal'' to SAL "A".  Execution of the inner loop
will cause ``prologue_sal'' will be set successively to "C", "E", and
finally "G".  (Note that there will be some/many iterations where
``prologue_sal'' doesn't change.)

As far as I can tell, the only interesting case that
skip_prologue_using_sal() handles is when the line numbers for
successive SALs are monotonically decreasing up to the prologue ->
body transition.

> GDB should be using the dwarf2 debug info when setting the prologue 
> breakpoint[s], how hard is it to do that?

I don't know.  When I became aware of this problem, I considered
just turning on dwarf2 CFI, but I wanted to understand why prologue
analysis was failing first.

Kevin

Re: [RFC] frv-tdep.c: Use refine_prologue_limit() instead of skip_prologue_using_sal()

[Andrew Cagney]

> On Sat, 31 Jul 2004 13:37:55 -0400
> Andrew Cagney <cagney@gnu.org> wrote:
> 
> 
>>> More seriously, what's the case you've encountered?
> 
> 
> It's pthread_start_thread() from glibc.  Here's the code split up into
> SAL units with the line number at the right.  (I synthesized this by hand
> from "x/i" and "info line" output.  I can provide you with the raw data
> if you want...)
> 
> Code                                            Line Number     Ref
> ---------------------------                     -----------     ---
> 0x2e020  addi sp,-200,sp                        256             A
> 0x2e024  sti.p fp,@(sp,184)
> 
> 0x2e028  sethi 0xffff,gr4                       273             B
> 
> 0x2e02c  addi.p sp,184,fp                       256             C
> 
> 0x2e030  setlo 0xfd80,gr4                       273             D
> 
> 0x2e034  movsg lr,gr5                           256             E
> 0x2e038  stdi.p gr18,@(sp,0)
> 0x2e03c  ori gr15,0,gr18
> 0x2e040  sti.p gr5,@(fp,8)
> 
> 0x2e044  ori gr8,0,gr29                         265             F
> 
> 0x2e048  stdi.p gr20,@(sp,8)                    256             G
> 0x2e04c  ori gr8,0,gr19
> 0x2e050  stdi gr22,@(sp,16)
> 
> 0x2e054  ldd @(gr4,gr15),gr14                   273             H
> 0x2e058  calll @(gr14,gr0)
> 
> 0x2e05c  sethi.p 0xffff,gr4                     276             I
> 0x2e060  setlo 0xfb20,gr4
> 
> 0x2e064  sti.p gr8,@(gr19,84)                   273             J
> 
> 0x2e068  addi gr19,148,gr9                      276             K
> 0x2e06c  ldd.p @(gr4,gr18),gr14
> 0x2e070  setlos 0x2,gr8
> 0x2e074  calll.p @(gr14,gr0)
> 0x2e078  setlos lo(0x0),gr10

What about the corresponding C code?

> The last prologue SAL is marked "G" in the "Ref" column.
> 
> When these SALs are scanned using skip_prologue_using_sal(),
> prologue_sal will be initialized to SAL "A".  When the loop
> is entered, ``sal'' (local to the loop) is set to "B".  The
> following test causes the loop to terminate on the first
> iteration:
> 
>           if (sal.line >= prologue_sal.line)
>             break;

For reference:

       while (prologue_sal.end < end_pc)
         {
           struct symtab_and_line sal;

           sal = find_pc_line (prologue_sal.end, 0);
           if (sal.line == 0)
             break;
           /* Assume that a consecutive SAL for the same (or larger)
              line mark the prologue -> body transition.  */
           if (sal.line >= prologue_sal.line)
             break;

As in:
   28    int foo (int i) { return i * 2; };
would have two SALs at line 28.

           /* The case in which compiler's optimizer/scheduler has
              moved instructions into the prologue.  We look ahead in
              the function looking for address ranges whose
              corresponding line number is less the first one that we
              found for the function.  This is more conservative then
              refine_prologue_limit which scans a large number of SALs
              looking for any in the prologue */
           prologue_sal = sal;
         }

> Here, sal.line is 273 and prologue_sal.line is 256.
> 
> Thus, skip_prologue_using_sal() returns the end address corresponding
> to SAL "A" (which is actually 0x2e028 since the end address is actually
> the start address for the next SAL).
> 
> By way of contrast, refine_prologue_limit() starts out the same way
> initializing ``prologue_sal'' to SAL "A".  Execution of the inner loop
> will cause ``prologue_sal'' will be set successively to "C", "E", and
> finally "G".  (Note that there will be some/many iterations where
> ``prologue_sal'' doesn't change.)

For reference:

       for (i = 2 * max_skip_non_prologue_insns;
            i > 0 && (lim_pc == 0 || addr < lim_pc);
            i--)
         {
           struct symtab_and_line sal;

           sal = find_pc_line (addr, 0);
           if (sal.line == 0)
             break;
           if (sal.line <= prologue_sal.line
               && sal.symtab == prologue_sal.symtab)
             {
               prologue_sal = sal;
             }
           addr = sal.end;
         }


> As far as I can tell, the only interesting case that
> skip_prologue_using_sal() handles is when the line numbers for
> successive SALs are monotonically decreasing up to the prologue ->
> body transition.

Sounds like it should be tweaked.

>>> GDB should be using the dwarf2 debug info when setting the prologue 
>>> breakpoint[s], how hard is it to do that?
> 
> 
> I don't know.  When I became aware of this problem, I considered
> just turning on dwarf2 CFI, but I wanted to understand why prologue
> analysis was failing first.

dwarf2's prologue info is separate to CFI - its part of the line number 
information - and lets us implement breakpoints past the prologue in a 
portable way.

Andrew