Re: [patch] Fix var-tracking with dynamic stack realignment on x86

[Eric Botcazou <ebotcazou@adacore.com>]

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

> Here is an alternative, almost completely untested, patch, which
> uses DW_OP_fbreg <N> for the arguments of dynamically realigned functions.
> draptest.c now works...

Thanks for the tip.  I completely overlooked this CFA-based machinery and now 
agree that it's the most sensible approach.  For my defense, I would point out 
that it's quite lightly documented. :-)  I've attached a patch that adds a 
blurb about it (as well as contains the fp_cfa_offset adjustment we both have 
in our patches).  I'll install it as obvious if you don't have any objection.

Now on to a few questions:

> 2011-05-23  Jakub Jelinek  <jakub@redhat.com>
>
> 	* var-tracking.c (vt_add_function_parameter): Remap incoming
> 	MEMs with crtl->args.internal_arg_pointer based address
> 	if stack_realign_drap to arg_pointer_rtx.
> 	(vt_init_cfa_base): Add equate argument, don't equate cfa_base_rtx
> 	to hfp/sp if it is false.
> 	(vt_initialize): Adjust vt_init_cfa_base callers, don't call
> 	vt_init_cfa_base if frame_pointer_needed, but fp_cfa_offset is -1,
> 	set fp_cfa_offset to -1 if fp/argp hasn't been eliminated.

Why setting CFA_BASE_RTX here?  The remapping in vt_add_function_parameter 
doesn't seem to need it.  Is that necessary so that the processing of MEMs is 
short-circuited elsewhere?  If so, would this be correct in the whole function 
given that the MEMs aren't adjusted in this case (i.e. compute_cfa_pointer is 
never called)?


	* var-tracking.c (compute_cfa_pointer): Adjust head comment.
	(vt_initialize):  Set PROLOGUE_BB unconditionally.
	Add block comment about CFA_BASE_RTX machinery.
	Reset FP_CFA_OFFSET to -1 on all invalid paths.
	Call vt_init_cfa_base only if FP_CFA_OFFSET isn't equal to -1.


-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-diff, Size: 2282 bytes --]

Index: var-tracking.c
===================================================================
--- var-tracking.c	(revision 174058)
+++ var-tracking.c	(working copy)
@@ -705,7 +705,8 @@ vt_stack_adjustments (void)
 static rtx cfa_base_rtx;
 static HOST_WIDE_INT cfa_base_offset;
 
-/* Compute a CFA-based value for the stack pointer.  */
+/* Compute a CFA-based value for an ADJUSTMENT made to stack_pointer_rtx
+   or hard_frame_pointer_rtx.  */
 
 static inline rtx
 compute_cfa_pointer (HOST_WIDE_INT adjustment)
@@ -8664,7 +8665,7 @@ vt_init_cfa_base (void)
 static bool
 vt_initialize (void)
 {
-  basic_block bb, prologue_bb = NULL;
+  basic_block bb, prologue_bb = single_succ (ENTRY_BLOCK_PTR);
   HOST_WIDE_INT fp_cfa_offset = -1;
 
   alloc_aux_for_blocks (sizeof (struct variable_tracking_info_def));
@@ -8722,6 +8723,15 @@ vt_initialize (void)
 
   CLEAR_HARD_REG_SET (argument_reg_set);
 
+  /* In order to shield ourselves from adjustments to the stack pointer or to
+     the hard frame pointer that aren't meant to change the base location of
+     variables, we're going to rewrite MEMs based on them into MEMs based on
+     the CFA by de-eliminating stack_pointer_rtx or hard_frame_pointer_rtx to
+     the virtual CFA pointer frame_pointer_rtx resp. arg_pointer_rtx.  We can
+     do this either when there is no frame pointer in the function and stack
+     adjustments are consistent for all basic blocks or when there is a frame
+     pointer and no stack realignment.  In any case, we first have to check
+     that frame_pointer_rtx resp. arg_pointer_rtx has been eliminated.  */
   if (!frame_pointer_needed)
     {
       rtx reg, elim;
@@ -8764,10 +8774,11 @@ vt_initialize (void)
 	    }
 	  if (elim != hard_frame_pointer_rtx)
 	    fp_cfa_offset = -1;
-	  else
-	    prologue_bb = single_succ (ENTRY_BLOCK_PTR);
 	}
+      else
+	fp_cfa_offset = -1;
     }
+
   if (frame_pointer_needed)
     {
       rtx insn;
@@ -8870,7 +8881,8 @@ vt_initialize (void)
 		  if (bb == prologue_bb
 		      && hard_frame_pointer_adjustment == -1
 		      && RTX_FRAME_RELATED_P (insn)
-		      && fp_setter (insn))
+		      && fp_setter (insn)
+		      && fp_cfa_offset != -1)
 		    {
 		      vt_init_cfa_base ();
 		      hard_frame_pointer_adjustment = fp_cfa_offset;