This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

var-tracking and s390's LM(G)


Sorry, looks like I missed the stage 3 deadline by a day, but:
we'd like to add support for shrink-wrapping and conditional returns
to s390(x) for 4.9.  Doing this showed up a problem with the way that
var-tracking handles the load-multiple instruction that s390 uses in
many function epilogues.

stack_adjust_offset_pre_post punts on any assignment to the stack
pointer that it doesn't understand and assumes that no adjustment
is made.  And the only kind of direct set that it understands is
(reasonably enough) one of the form (plus (sp) (const_int X)).
(There's a MINUS case too but it should never trigger.)

But if s390 needs to save and restore several registers, it will
use LM(G) to restore the stack pointer too, rather than generating
a separate addition.  stack_adjust_offset_pre_post doesn't understand
the load and so the stack_adjust after the epilogue is the same as
before it, rather than the correct post-epilogue value.

When shrink-wrapping is enabled, we have other edges to the exit block
that have the correct stack_adjust and so we fail the test:

	  /* Check whether the adjustments on the edges are the same.  */
	  if (VTI (dest)->in.stack_adjust != VTI (src)->out.stack_adjust)
	    {
	      free (stack);
	      return false;
	    }

No var-tracking is then performed on the function, leading to some new
guality.exp failures.

I wondered whether we could model the load of the stack pointer as an
addition in cases where that is safe (e.g. it would require !calls_eh_return
at least).  But that would only work in functions that don't need a frame
pointer.  In functions that do need a frame pointer we'd presumably have
(plus (hfp) (const_int X)) instead, which stack_adjust_offset_pre_post
would again not understand.

Another option would be to work out the offset from REG_CFA_DEF_CFA notes,
where present, but I'm a bit uncomfortable with the idea of mixing two
different methods of calculating the stack offset.

The simplest fix seems to be to disable this check for the exit block.
We never use its stack_adjust anyway, and dwarf2cfi already checks
(using CFA information) that the offsets in a shrink-wrapped function
are consistent.

Tested on s390-linux-gnu and s390x-linux-gnu.  OK to install?

Thanks,
Richard


gcc/
	* var-tracking.c (vt_stack_adjustments): Don't require stack_adjusts
	to match for the exit block.

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c	2014-02-03 12:16:49.009149939 +0000
+++ gcc/var-tracking.c	2014-02-04 09:58:59.924381497 +0000
@@ -886,8 +886,25 @@ vt_stack_adjustments (void)
 	}
       else
 	{
-	  /* Check whether the adjustments on the edges are the same.  */
-	  if (VTI (dest)->in.stack_adjust != VTI (src)->out.stack_adjust)
+	  /* We can end up with different stack adjustments for the exit block
+	     of a shrink-wrapped function if stack_adjust_offset_pre_post
+	     doesn't understand the rtx pattern used to restore the stack
+	     pointer in the epilogue.  For example, on s390(x), the stack
+	     pointer is often restored via a load-multiple instruction
+	     and so no stack_adjust offset is recorded for it.  This means
+	     that the stack offset at the end of the epilogue block is the
+	     the same as the offset before the epilogue, whereas other paths
+	     to the exit block will have the correct stack_adjust.
+
+	     It is safe to ignore these differences because (a) we never
+	     use the stack_adjust for the exit block in this pass and
+	     (b) dwarf2cfi checks whether the CFA notes in a shrink-wrapped
+	     function are correct.
+
+	     We must check whether the adjustments on other edges are
+	     the same though.  */
+	  if (dest != EXIT_BLOCK_PTR_FOR_FN (cfun)
+	      && VTI (dest)->in.stack_adjust != VTI (src)->out.stack_adjust)
 	    {
 	      free (stack);
 	      return false;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]