Fix var-tracking WRT location list for DW_AT_frame_base

Fix var-tracking WRT location list for DW_AT_frame_base

[Josef Zlomek]

Hello,

when a location list is used for DW_AT_frame_base and a location expr
(i.e. the old debug info) is used for a variable which is located in
memory, GDB does not print the value of variable correctly
as Ulrich Weigand discovered on s390 (but the problem is generic
with -fomit-frame-pointer).

The problem is that the offset in location expr is offseted from
the stack pointer after prologue but the location list for
DW_AT_frame_base is offseted from the stack pointer at function entry
with -fomit-frame-pointer.

The attached patch makes var-tracking to offset DW_AT_frame_base from
the stack pointer after prologue and thus fixes the problem.

Bootstrapped/regtested x86-64 and ia64.

Josef

2004-06-22  Josef Zlomek  <zlomekj@suse.cz>

	* var-tracking.c: Fix some comments.
	(frame_stack_adjust): New.
	(vt_stack_adjustments): Init stack_adjust of entry block to
	minus stack adjustment of function prologue.
	(vt_add_function_parameters): Do not adjust locations of
	function arguments.
	(vt_initialize): Compute the stack adjustment of function
	prologue and offset the initial "location" of frame_base_decl
	from the stack pointer after prologue.

Index: var-tracking.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/var-tracking.c,v
retrieving revision 2.17
diff -c -p -r2.17 var-tracking.c
*** var-tracking.c	15 Jun 2004 18:02:40 -0000	2.17
--- var-tracking.c	22 Jun 2004 08:00:21 -0000
*************** static bool emit_notes;
*** 267,272 ****
--- 267,275 ----
  /* Fake variable for stack pointer.  */
  tree frame_base_decl;
  
+ /* Stack adjust caused by function prologue.  */
+ HOST_WIDE_INT frame_stack_adjust;
+ 
  /* Local function prototypes.  */
  static void stack_adjust_offset_pre_post (rtx, HOST_WIDE_INT *,
  					  HOST_WIDE_INT *);
*************** bb_stack_adjust_offset (basic_block bb)
*** 484,490 ****
    VTI (bb)->out.stack_adjust = offset;
  }
  
! /* Compute stack adjustment caused by function prolog.  */
  
  static HOST_WIDE_INT
  prologue_stack_adjust (void)
--- 487,493 ----
    VTI (bb)->out.stack_adjust = offset;
  }
  
! /* Compute stack adjustment caused by function prologue.  */
  
  static HOST_WIDE_INT
  prologue_stack_adjust (void)
*************** vt_stack_adjustments (void)
*** 528,534 ****
  
    /* Initialize entry block.  */
    VTI (ENTRY_BLOCK_PTR)->visited = true;
!   VTI (ENTRY_BLOCK_PTR)->out.stack_adjust = 0;
  
    /* Allocate stack for back-tracking up CFG.  */
    stack = xmalloc ((n_basic_blocks + 1) * sizeof (edge));
--- 531,537 ----
  
    /* Initialize entry block.  */
    VTI (ENTRY_BLOCK_PTR)->visited = true;
!   VTI (ENTRY_BLOCK_PTR)->out.stack_adjust = frame_stack_adjust;
  
    /* Allocate stack for back-tracking up CFG.  */
    stack = xmalloc ((n_basic_blocks + 1) * sizeof (edge));
*************** variable_was_changed (variable var, htab
*** 1918,1926 ****
  }
  
  /* Set the location of frame_base_decl to LOC in dataflow set SET.  This
!    function expects that
!    frame_base_decl has already one location for offset 0 in the variable table.
!  */
  
  static void
  set_frame_base_location (dataflow_set *set, rtx loc)
--- 1921,1928 ----
  }
  
  /* Set the location of frame_base_decl to LOC in dataflow set SET.  This
!    function expects that frame_base_decl has already one location for offset 0
!    in the variable table.  */
  
  static void
  set_frame_base_location (dataflow_set *set, rtx loc)
*************** static void
*** 2490,2500 ****
  vt_add_function_parameters (void)
  {
    tree parm;
-   HOST_WIDE_INT stack_adjust = 0;
    
-   if (!frame_pointer_needed)
-     stack_adjust = prologue_stack_adjust ();
- 
    for (parm = DECL_ARGUMENTS (current_function_decl);
         parm; parm = TREE_CHAIN (parm))
      {
--- 2492,2498 ----
*************** vt_add_function_parameters (void)
*** 2529,2536 ****
  #endif
  
        incoming = eliminate_regs (incoming, 0, NULL_RTX);
-       if (!frame_pointer_needed && GET_CODE (incoming) == MEM)
- 	incoming = adjust_stack_reference (incoming, -stack_adjust);
        out = &VTI (ENTRY_BLOCK_PTR)->out;
  
        if (REG_P (incoming))
--- 2527,2532 ----
*************** vt_initialize (void)
*** 2696,2701 ****
--- 2692,2699 ----
      {
        rtx base;
  
+       frame_stack_adjust = -prologue_stack_adjust ();
+ 
        /* Create fake variable for tracking stack pointer changes.  */
        frame_base_decl = make_node (VAR_DECL);
        DECL_NAME (frame_base_decl) = get_identifier ("___frame_base_decl");
*************** vt_initialize (void)
*** 2703,2709 ****
        DECL_ARTIFICIAL (frame_base_decl) = 1;
  
        /* Set its initial "location".  */
!       base = gen_rtx_MEM (Pmode, stack_pointer_rtx);
        set_variable_part (&VTI (ENTRY_BLOCK_PTR)->out, base, frame_base_decl, 0);
      }
    else
--- 2701,2709 ----
        DECL_ARTIFICIAL (frame_base_decl) = 1;
  
        /* Set its initial "location".  */
!       base = gen_rtx_MEM (Pmode,
! 			  gen_rtx_PLUS (Pmode, stack_pointer_rtx,
! 					GEN_INT (frame_stack_adjust)));
        set_variable_part (&VTI (ENTRY_BLOCK_PTR)->out, base, frame_base_decl, 0);
      }
    else

Re: Fix var-tracking WRT location list for DW_AT_frame_base

[Roger Sayle]

On Tue, 22 Jun 2004, Josef Zlomek wrote:
> 2004-06-22  Josef Zlomek  <zlomekj@suse.cz>
>
> 	* var-tracking.c: Fix some comments.
> 	(frame_stack_adjust): New.
> 	(vt_stack_adjustments): Init stack_adjust of entry block to
> 	minus stack adjustment of function prologue.
> 	(vt_add_function_parameters): Do not adjust locations of
> 	function arguments.
> 	(vt_initialize): Compute the stack adjustment of function
> 	prologue and offset the initial "location" of frame_base_decl
> 	from the stack pointer after prologue.

The new global variable "frame_stack_adjust" needs to be made static
as it isn't used outside of var-tracking.c.

And how about changing

>         /* Set its initial "location".  */
> !       base = gen_rtx_MEM (Pmode,
> ! 			  gen_rtx_PLUS (Pmode, stack_pointer_rtx,
> ! 					GEN_INT (frame_stack_adjust)));

to

	base = frame_stack_adjust
	       ? gen_rtx_PLUS (Pmode, stack_pointer_rtx,
			       GEN_INT (frame_stack_adjust))
	       : stack_pointer_rtx;
	base = gen_rtx_MEM (Pmode, base);

This avoids generating a dubious PLUS rtx in the common/current case
where frame_stack_adjust is zero.

Ok for mainline with those changes, after the usual bootstrapping
and regression testing.

Roger
--

Re: Fix var-tracking WRT location list for DW_AT_frame_base

[Richard Sandiford]

Roger Sayle <roger@eyesopen.com> writes:
> And how about changing
>
>>         /* Set its initial "location".  */
>> !       base = gen_rtx_MEM (Pmode,
>> ! 			  gen_rtx_PLUS (Pmode, stack_pointer_rtx,
>> ! 					GEN_INT (frame_stack_adjust)));
>
> to
>
> 	base = frame_stack_adjust
> 	       ? gen_rtx_PLUS (Pmode, stack_pointer_rtx,
> 			       GEN_INT (frame_stack_adjust))
> 	       : stack_pointer_rtx;
> 	base = gen_rtx_MEM (Pmode, base);

Maybe it'd be better to use plus_constant.  I.e.:

        base = gen_rtx_MEM (Pmode, plus_constant (stack_pointer_rtx,
                                                  frame_stack_adjust);

Richard

Re: Fix var-tracking WRT location list for DW_AT_frame_base

[Josef Zlomek]

> > And how about changing
> >
> >>         /* Set its initial "location".  */
> >> !       base = gen_rtx_MEM (Pmode,
> >> ! 			  gen_rtx_PLUS (Pmode, stack_pointer_rtx,
> >> ! 					GEN_INT (frame_stack_adjust)));
> >
> > to
> >
> > 	base = frame_stack_adjust
> > 	       ? gen_rtx_PLUS (Pmode, stack_pointer_rtx,
> > 			       GEN_INT (frame_stack_adjust))
> > 	       : stack_pointer_rtx;
> > 	base = gen_rtx_MEM (Pmode, base);
> 
> Maybe it'd be better to use plus_constant.  I.e.:
> 
>         base = gen_rtx_MEM (Pmode, plus_constant (stack_pointer_rtx,
>                                                   frame_stack_adjust);

IMHO plus_constant is too heavyweight, all I need is to generate a
(mem (stackreg)) or (mem (plus (stackreg) (const)))

What about using the following function instead of those gen_rtx_MEMs
which would probably get inlined?

static rtx 
frame_base_location_rtx (HOST_WIDE_INT offset)
{
  rtx x;

  if (offset == 0)
    return gen_rtx_MEM (Pmode, stack_pointer_rtx);

  return gen_rtx_MEM (Pmode, gen_rtx_PLUS (Pmode, stack_pointer_rtx,
                                           GEN_INT (offset)));
}

Regards,

Josef

Re: Fix var-tracking WRT location list for DW_AT_frame_base

[Josef Zlomek]

> static rtx 
> frame_base_location_rtx (HOST_WIDE_INT offset)
> {
>   rtx x;

Oops, "rtx x;" should not be here.

>   if (offset == 0)
>     return gen_rtx_MEM (Pmode, stack_pointer_rtx);
> 
>   return gen_rtx_MEM (Pmode, gen_rtx_PLUS (Pmode, stack_pointer_rtx,
>                                            GEN_INT (offset)));
> }

Regards,

Josef

Re: Fix var-tracking WRT location list for DW_AT_frame_base

[Roger Sayle]

On Sun, 4 Jul 2004, Josef Zlomek wrote:
> What about using the following function instead of those gen_rtx_MEMs
> which would probably get inlined?
>
> static rtx
> frame_base_location_rtx (HOST_WIDE_INT offset)
> {
>   if (offset == 0)
>     return gen_rtx_MEM (Pmode, stack_pointer_rtx);
>
>   return gen_rtx_MEM (Pmode, gen_rtx_PLUS (Pmode, stack_pointer_rtx,
>                                            GEN_INT (offset)));
> }

Sure.  I appreciate that if performance is a concern that both
plus_constant and simplify_gen_binary are probably overkill.
The issue is that if you don't use either of these functions,
you manually have to check for the "zero" case.

Your function above looks fine, provided you remember to add a
suitable comment above it.

Thanks,

Roger
--

Re: Fix var-tracking WRT location list for DW_AT_frame_base

[Josef Zlomek]

> Josef, could you go with the variant using plus_constant?

Yes. The patch is attached.
Bootstrapped/regtested ia64; clean mainline does not build on ppc and ppc64.
I'm commiting it as pre-approved.

Josef

2004-07-05  Josef Zlomek  <zlomekj@suse.cz>

	* var-tracking.c: Fix some comments.
	(frame_stack_adjust): New.
	(vt_stack_adjustments): Init stack_adjust of entry block to
	minus stack adjustment of function prologue.
	(adjust_stack_reference): Do not adjust if adjustment == 0.
	(compute_bb_dataflow): Use plus_constant instead of gen_rtx_PLUS.
	(emit_notes_in_bb): Likewise.
	(vt_add_function_parameters): Do not adjust locations of
	function arguments.
	(vt_initialize): Compute the stack adjustment of function
	prologue and offset the initial "location" of frame_base_decl
	from the stack pointer after prologue.

Index: var-tracking.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/var-tracking.c,v
retrieving revision 2.18
diff -c -p -r2.18 var-tracking.c
*** var-tracking.c	1 Jul 2004 12:52:53 -0000	2.18
--- var-tracking.c	5 Jul 2004 04:34:56 -0000
*************** static bool emit_notes;
*** 267,272 ****
--- 267,275 ----
  /* Fake variable for stack pointer.  */
  tree frame_base_decl;
  
+ /* Stack adjust caused by function prologue.  */
+ static HOST_WIDE_INT frame_stack_adjust;
+ 
  /* Local function prototypes.  */
  static void stack_adjust_offset_pre_post (rtx, HOST_WIDE_INT *,
  					  HOST_WIDE_INT *);
*************** bb_stack_adjust_offset (basic_block bb)
*** 484,490 ****
    VTI (bb)->out.stack_adjust = offset;
  }
  
! /* Compute stack adjustment caused by function prolog.  */
  
  static HOST_WIDE_INT
  prologue_stack_adjust (void)
--- 487,493 ----
    VTI (bb)->out.stack_adjust = offset;
  }
  
! /* Compute stack adjustment caused by function prologue.  */
  
  static HOST_WIDE_INT
  prologue_stack_adjust (void)
*************** vt_stack_adjustments (void)
*** 528,534 ****
  
    /* Initialize entry block.  */
    VTI (ENTRY_BLOCK_PTR)->visited = true;
!   VTI (ENTRY_BLOCK_PTR)->out.stack_adjust = 0;
  
    /* Allocate stack for back-tracking up CFG.  */
    stack = xmalloc ((n_basic_blocks + 1) * sizeof (edge));
--- 531,537 ----
  
    /* Initialize entry block.  */
    VTI (ENTRY_BLOCK_PTR)->visited = true;
!   VTI (ENTRY_BLOCK_PTR)->out.stack_adjust = frame_stack_adjust;
  
    /* Allocate stack for back-tracking up CFG.  */
    stack = xmalloc ((n_basic_blocks + 1) * sizeof (edge));
*************** adjust_stack_reference (rtx mem, HOST_WI
*** 590,595 ****
--- 593,601 ----
    rtx adjusted_mem;
    rtx tmp;
  
+   if (adjustment == 0)
+     return mem;
+ 
    adjusted_mem = copy_rtx (mem);
    XEXP (adjusted_mem, 0) = replace_rtx (XEXP (adjusted_mem, 0),
  					stack_pointer_rtx,
*************** compute_bb_dataflow (basic_block bb)
*** 1653,1661 ****
  	      rtx base;
  
  	      out->stack_adjust += VTI (bb)->mos[i].u.adjust;
! 	      base = gen_rtx_MEM (Pmode,
! 				  gen_rtx_PLUS (Pmode, stack_pointer_rtx,
! 						GEN_INT (out->stack_adjust)));
  	      set_frame_base_location (out, base);
  	    }
  	    break;
--- 1659,1666 ----
  	      rtx base;
  
  	      out->stack_adjust += VTI (bb)->mos[i].u.adjust;
! 	      base = gen_rtx_MEM (Pmode, plus_constant (stack_pointer_rtx,
! 							out->stack_adjust));
  	      set_frame_base_location (out, base);
  	    }
  	    break;
*************** variable_was_changed (variable var, htab
*** 1918,1926 ****
  }
  
  /* Set the location of frame_base_decl to LOC in dataflow set SET.  This
!    function expects that
!    frame_base_decl has already one location for offset 0 in the variable table.
!  */
  
  static void
  set_frame_base_location (dataflow_set *set, rtx loc)
--- 1923,1930 ----
  }
  
  /* Set the location of frame_base_decl to LOC in dataflow set SET.  This
!    function expects that frame_base_decl has already one location for offset 0
!    in the variable table.  */
  
  static void
  set_frame_base_location (dataflow_set *set, rtx loc)
*************** emit_notes_in_bb (basic_block bb)
*** 2409,2417 ****
  	      rtx base;
  
  	      set.stack_adjust += VTI (bb)->mos[i].u.adjust;
! 	      base = gen_rtx_MEM (Pmode,
! 				  gen_rtx_PLUS (Pmode, stack_pointer_rtx,
! 						GEN_INT (set.stack_adjust)));
  	      set_frame_base_location (&set, base);
  	      emit_notes_for_changes (insn, EMIT_NOTE_AFTER_INSN);
  	    }
--- 2413,2420 ----
  	      rtx base;
  
  	      set.stack_adjust += VTI (bb)->mos[i].u.adjust;
! 	      base = gen_rtx_MEM (Pmode, plus_constant (stack_pointer_rtx,
! 							set.stack_adjust));
  	      set_frame_base_location (&set, base);
  	      emit_notes_for_changes (insn, EMIT_NOTE_AFTER_INSN);
  	    }
*************** static void
*** 2490,2500 ****
  vt_add_function_parameters (void)
  {
    tree parm;
-   HOST_WIDE_INT stack_adjust = 0;
    
-   if (!frame_pointer_needed)
-     stack_adjust = prologue_stack_adjust ();
- 
    for (parm = DECL_ARGUMENTS (current_function_decl);
         parm; parm = TREE_CHAIN (parm))
      {
--- 2493,2499 ----
*************** vt_add_function_parameters (void)
*** 2529,2536 ****
  #endif
  
        incoming = eliminate_regs (incoming, 0, NULL_RTX);
-       if (!frame_pointer_needed && MEM_P (incoming))
- 	incoming = adjust_stack_reference (incoming, -stack_adjust);
        out = &VTI (ENTRY_BLOCK_PTR)->out;
  
        if (REG_P (incoming))
--- 2528,2533 ----
*************** vt_initialize (void)
*** 2703,2709 ****
        DECL_ARTIFICIAL (frame_base_decl) = 1;
  
        /* Set its initial "location".  */
!       base = gen_rtx_MEM (Pmode, stack_pointer_rtx);
        set_variable_part (&VTI (ENTRY_BLOCK_PTR)->out, base, frame_base_decl, 0);
      }
    else
--- 2700,2708 ----
        DECL_ARTIFICIAL (frame_base_decl) = 1;
  
        /* Set its initial "location".  */
!       frame_stack_adjust = -prologue_stack_adjust ();
!       base = gen_rtx_MEM (Pmode, plus_constant (stack_pointer_rtx,
! 						frame_stack_adjust));
        set_variable_part (&VTI (ENTRY_BLOCK_PTR)->out, base, frame_base_decl, 0);
      }
    else