Re: [RFA] Fix small problems in rs6000-tdep.c:skip_prologue()

[Joel Brobecker <brobecker@gnat.com>]

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

> In this case, the stX 0,N(31) is spilling an argument, even though r0
> is not an argument register.  ('evstdd' is an E500 instruction that
> is definitely an argument spill.)

Dough!

> Clearly, both your function and mine need to go into the test suite...

I can add a new testcase in Ada for the prologue we saw.

> What if we did something like this?  It'd need to be combined with the 
> rest of your change, I'm just sketching:

Attached is a revised version incorporating your changes. Could you
give it a shot against your function, and let me know if it works for
you? It works for me, and doesn't introduce any regression on our
powerpc-aix-5.1 machine.

2004-04-16  Joel Brobecker  <brobecker@gnat.com>

        * rs6000-tdep.c (store_param_on_stack_p): New function,
        an improved version of some code extracted from skip_prologue().
        (skip_prologue): Use store_param_on_stack_p() to detect
        instructions saving a parameter on the stack. Detect when r0
        is used to save a parameter.
        Do not mark "li rx, SIMM" instructions as part of the prologue,
        unless the following instruction is also part of the prologue.
        
I'll followup with a testcase soon.

Thanks,
-- 
Joel

[-- Attachment #2: rs6000-tdep.c.diff --]
[-- Type: text/plain, Size: 5986 bytes --]

Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.191
diff -u -p -r1.191 rs6000-tdep.c
--- rs6000-tdep.c	1 Apr 2004 21:00:59 -0000	1.191
+++ rs6000-tdep.c	17 Apr 2004 04:58:34 -0000
@@ -419,6 +419,76 @@ refine_prologue_limit (CORE_ADDR pc, COR
   return lim_pc;
 }
 
+/* Return nonzero if the given instruction OP can be part of the prologue
+   of a function that saves a parameter on the stack.  FRAMEP should be
+   set if one of the previous instructions in the function has set the
+   Frame Pointer.  */
+
+static int
+store_param_on_stack_p (unsigned long op, int framep, int *r0_contains_arg)
+{
+  /* Move parameters from argument registers to temporary register.  */
+  if ((op & 0xfc0007fe) == 0x7c000378)         /* mr(.)  Rx,Ry */
+    {
+      /* Rx must be scratch register r0.  */
+      const int rx_regno = (op >> 16) & 31;
+      /* Ry: Only r3 - r10 are used for parameter passing.  */
+      const int ry_regno = GET_SRC_REG (op);
+
+      if (rx_regno == 0 && ry_regno >= 3 && ry_regno <= 10)
+        {
+          *r0_contains_arg = 1;
+          return 1;
+        }
+      else
+        return 0;
+    }
+
+  /* Save a General Purpose Register on stack.  */
+
+  if ((op & 0xfc1f0003) == 0xf8010000 ||       /* std  Rx,NUM(r1) */
+      (op & 0xfc1f0000) == 0xd8010000)         /* stfd Rx,NUM(r1) */
+    {
+      /* Rx: Only r3 - r10 are used for parameter passing.  */
+      const int rx_regno = GET_SRC_REG (op);
+
+      return (rx_regno >= 3 && rx_regno <= 10);
+    }
+           
+  /* Save a General Purpose Register on stack via the Frame Pointer.  */
+
+  if (framep &&
+      ((op & 0xfc1f0000) == 0x901f0000 ||     /* st rx,NUM(r31) */
+       (op & 0xfc1f0000) == 0x981f0000 ||     /* stb Rx,NUM(r31) */
+       (op & 0xfc1f0000) == 0xd81f0000))      /* stfd Rx,NUM(r31) */
+    {
+      /* Rx: Usually, only r3 - r10 are used for parameter passing.
+         However, the compiler sometimes uses r0 to hold an argument.  */
+      const int rx_regno = GET_SRC_REG (op);
+
+      return ((rx_regno >= 3 && rx_regno <= 10)
+              || (rx_regno == 0 && *r0_contains_arg));
+    }
+
+  if ((op & 0xfc1f0000) == 0xfc010000)         /* frsp, fp?,NUM(r1) */
+    {
+      /* Only f2 - f8 are used for parameter passing.  */
+      const int src_regno = GET_SRC_REG (op);
+
+      return (src_regno >= 2 && src_regno <= 8);
+    }
+
+  if (framep && ((op & 0xfc1f0000) == 0xfc1f0000))  /* frsp, fp?,NUM(r31) */
+    {
+      /* Only f2 - f8 are used for parameter passing.  */
+      const int src_regno = GET_SRC_REG (op);
+
+      return (src_regno >= 2 && src_regno <= 8);
+    }
+
+  /* Not an insn that saves a parameter on stack.  */
+  return 0;
+}
 
 static CORE_ADDR
 skip_prologue (CORE_ADDR pc, CORE_ADDR lim_pc, struct rs6000_framedata *fdata)
@@ -441,6 +511,7 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
   int minimal_toc_loaded = 0;
   int prev_insn_was_prologue_insn = 1;
   int num_skip_non_prologue_insns = 0;
+  int r0_contains_arg = 0;
   const struct bfd_arch_info *arch_info = gdbarch_bfd_arch_info (current_gdbarch);
   struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
   
@@ -509,11 +580,15 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
 	     ones.  */
 	  if (lr_reg < 0)
 	    lr_reg = (op & 0x03e00000);
+          if (lr_reg == 0)
+            r0_contains_arg = 0;
 	  continue;
 	}
       else if ((op & 0xfc1fffff) == 0x7c000026)
 	{			/* mfcr Rx */
 	  cr_reg = (op & 0x03e00000);
+          if (cr_reg == 0)
+            r0_contains_arg = 0;
 	  continue;
 
 	}
@@ -560,6 +635,7 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
 				   for >= 32k frames */
 	  fdata->offset = (op & 0x0000ffff) << 16;
 	  fdata->frameless = 0;
+          r0_contains_arg = 0;
 	  continue;
 
 	}
@@ -568,6 +644,7 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
 				   lf of >= 32k frames */
 	  fdata->offset |= (op & 0x0000ffff);
 	  fdata->frameless = 0;
+          r0_contains_arg = 0;
 	  continue;
 
 	}
@@ -701,26 +778,7 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
 	  /* store parameters in stack */
 	}
       /* Move parameters from argument registers to temporary register.  */
-      else if ((op & 0xfc0007fe) == 0x7c000378 &&	/* mr(.)  Rx,Ry */
-               (((op >> 21) & 31) >= 3) &&              /* R3 >= Ry >= R10 */
-               (((op >> 21) & 31) <= 10) &&
-               (((op >> 16) & 31) == 0)) /* Rx: scratch register r0 */
-        {
-          continue;
-        }
-      else if ((op & 0xfc1f0003) == 0xf8010000 ||	/* std rx,NUM(r1) */
-	       (op & 0xfc1f0000) == 0xd8010000 ||	/* stfd Rx,NUM(r1) */
-	       (op & 0xfc1f0000) == 0xfc010000)		/* frsp, fp?,NUM(r1) */
-	{
-	  continue;
-
-	  /* store parameters in stack via frame pointer */
-	}
-      else if (framep &&
-	       ((op & 0xfc1f0000) == 0x901f0000 ||     /* st rx,NUM(r31) */
-                (op & 0xfc1f0000) == 0x981f0000 ||     /* stb Rx,NUM(r31) */
-		(op & 0xfc1f0000) == 0xd81f0000 ||     /* stfd Rx,NUM(r31) */
-		(op & 0xfc1f0000) == 0xfc1f0000))      /* frsp, fp?,NUM(r31) */
+      else if (store_param_on_stack_p (op, framep, &r0_contains_arg))
         {
 	  continue;
 
@@ -789,8 +847,15 @@ skip_prologue (CORE_ADDR pc, CORE_ADDR l
       else if ((op & 0xffff0000) == 0x38000000         /* li r0, SIMM */
                || (op & 0xffff0000) == 0x39c00000)     /* li r14, SIMM */
 	{
+          if ((op & 0xffff0000) == 0x38000000)
+            r0_contains_arg = 0;
 	  li_found_pc = pc;
 	  vr_saved_offset = SIGNED_SHORT (op);
+
+          /* This insn by itself is not part of the prologue, unless
+             if part of the pair of insns mentioned above. So do not
+             record this insn as part of the prologue yet.  */
+          prev_insn_was_prologue_insn = 0;
 	}
       /* Store vector register S at (r31+r0) aligned to 16 bytes.  */      
       /* 011111 sssss 11111 00000 00111001110 */