diff mbox

pdp11: Fix bad code for no frame pointer case

Message ID AAB094E5-0065-4D44-AAF7-5DF9982F6979@dell.com
State New
Headers show

Commit Message

Paul Koning Nov. 19, 2010, 2:04 a.m. UTC
FIRST_PARM_OFFSET was set on the assumption that there is always a frame pointer saved in the stack frame, which is incorrect.  This patch fixes that, which corrects testsuite failures caused by picking up function arguments at the wrong stack offset.

Tested by built, check of the failing testcase (abs-1.c).  Committed.

	paul

ChangeLog:

2010-11-18  Paul Koning  <ni1d@arrl.net>

	* config/pdp11/pdp11.h (FIRST_PARM_OFFSET): Fix case of no frame
	pointer.

Comments

Richard Henderson Nov. 19, 2010, 5:33 p.m. UTC | #1
On 11/18/2010 06:04 PM, Paul Koning wrote:
> -#define FIRST_PARM_OFFSET(FNDECL) 4
> +#define FIRST_PARM_OFFSET(FNDECL) ((frame_pointer_needed) ? 4 : 2)

This is a sign of broken register elimination.  This macro is used very early
in the rtl optimization path -- virtual register instantiation.  At that point,
the final value of frame_pointer_needed has not been set.

The position of ARG_POINTER_REGNUM needs to be completely above the local stack
frame in order to properly address the arguments.  So either you need to position
FRAME_POINTER_REGNUM at the top of the frame, or you need to create a "soft"
ARG_POINTER_REGNUM that is always eliminated to either the FRAME_POINTER_REGNUM
or to STACK_POINTER_REGNUM.

Quite often, ports also create a "soft" FRAME_POINTER_REGNUM, which is always
eliminated to HARD_FRAME_POINTER_REGNUM.  The soft frame pointer will always
point to the area for the local variables, whereas the hard frame pointer is
somewhere else, generally within the area for the saved registers.

For instance, for i386:

  void f(int a, int b)
  {
    int x[10];
    int y[a];
    g(a, x, y);
  }


	b
        a
	---------------	arg_pointer
	return addr
        save ebp
        --------------- hard_frame_pointer
	save ebx
	save esi
	save edi
	--------------- frame_pointer
	x[10]		(local variable area)
	---------------
	y[a]		(alloca area)
	---------------
	&y
	&x		(outgoing arguments)
	a
	--------------- stack_pointer

Before register allocation, the fixed positions that we have are
arg_pointer, frame_pointer, and stack_pointer (for outgoing arguments).

During register allocation, the ELIMINABLE_REGS, TARGET_CAN_ELIMINATE
and INITIAL_ELIMINATION_OFFSET macros determine which eliminations are
possible, and what offsets to apply when doing so.

The pieces of information needed in order to compute these offsets are

  (1) whether frame pointer is required (frame_pointer_needed),
  (2) the size of the register save area (some cpu.c function),
  (3) the size of the local stack frame (get_frame_size ()),
  (4) the size of the outgoing arguments (ctrl->outgoing_args_size).

Exactly how you apply these depends on exactly how you lay out the
entire local stack frame.


r~
Paul Koning Nov. 19, 2010, 5:44 p.m. UTC | #2
On Nov 19, 2010, at 12:33 PM, Richard Henderson wrote:

> On 11/18/2010 06:04 PM, Paul Koning wrote:
>> -#define FIRST_PARM_OFFSET(FNDECL) 4
>> +#define FIRST_PARM_OFFSET(FNDECL) ((frame_pointer_needed) ? 4 : 2)
> 
> This is a sign of broken register elimination.  This macro is used very early
> in the rtl optimization path -- virtual register instantiation.  At that point,
> the final value of frame_pointer_needed has not been set.
> 
> The position of ARG_POINTER_REGNUM needs to be completely above the local stack
> frame in order to properly address the arguments.  So either you need to position
> FRAME_POINTER_REGNUM at the top of the frame, or you need to create a "soft"
> ARG_POINTER_REGNUM that is always eliminated to either the FRAME_POINTER_REGNUM
> or to STACK_POINTER_REGNUM.
> 
> Quite often, ports also create a "soft" FRAME_POINTER_REGNUM, which is always
> eliminated to HARD_FRAME_POINTER_REGNUM.  The soft frame pointer will always
> point to the area for the local variables, whereas the hard frame pointer is
> somewhere else, generally within the area for the saved registers.
> 
> For instance, for i386:
> 
>  void f(int a, int b)
>  {
>    int x[10];
>    int y[a];
>    g(a, x, y);
>  }
> 
> 
> 	b
>        a
> 	---------------	arg_pointer
> 	return addr
>        save ebp
>        --------------- hard_frame_pointer
> 	save ebx
> 	save esi
> 	save edi
> 	--------------- frame_pointer
> 	x[10]		(local variable area)
> 	---------------
> 	y[a]		(alloca area)
> 	---------------
> 	&y
> 	&x		(outgoing arguments)
> 	a
> 	--------------- stack_pointer
> 
> Before register allocation, the fixed positions that we have are
> arg_pointer, frame_pointer, and stack_pointer (for outgoing arguments).
> 
> During register allocation, the ELIMINABLE_REGS, TARGET_CAN_ELIMINATE
> and INITIAL_ELIMINATION_OFFSET macros determine which eliminations are
> possible, and what offsets to apply when doing so.
> 
> The pieces of information needed in order to compute these offsets are
> 
>  (1) whether frame pointer is required (frame_pointer_needed),
>  (2) the size of the register save area (some cpu.c function),
>  (3) the size of the local stack frame (get_frame_size ()),
>  (4) the size of the outgoing arguments (ctrl->outgoing_args_size).
> 
> Exactly how you apply these depends on exactly how you lay out the
> entire local stack frame.

That's helpful.  I noticed that this "fix" helps some cases and breaks others, depending on whether a frame pointer appears or not.  I'll dig into i386 and those other macros to make this right.

	paul
diff mbox

Patch

Index: config/pdp11/pdp11.h
===================================================================
--- config/pdp11/pdp11.h	(revision 166927)
+++ config/pdp11/pdp11.h	(working copy)
@@ -372,11 +372,10 @@ 
 /* Offset of first parameter from the argument pointer register value.  
    For the pdp11, this is nonzero to account for the return address.
 	1 - return address
-	2 - frame pointer (always saved, even when not used!!!!)
-		-- change some day !!!:q!
+	2 - frame pointer, if needed
 
 */
-#define FIRST_PARM_OFFSET(FNDECL) 4
+#define FIRST_PARM_OFFSET(FNDECL) ((frame_pointer_needed) ? 4 : 2)
 
 /* Define how to find the value returned by a function.
    VALTYPE is the data type of the value (as a tree).