[PATCH] S/390: Emit a barrier INSN between stack pointer decrement and stack slot writes

["Andreas Krebbel" <Andreas dot Krebbel at de dot ibm dot com>]

Hello,

on 64bit (s390x) we store the floating point registers f8-f15 below
the normal register save area.  In order to access that save area we
set up a copy of the old stack pointer in a different register and
write them after the original stack pointer has been decremented.

Now it might happen that the scheduler moves the decrement of the
stack pointer over the instructions writing the stack slots using a
copy of the stack pointer.  Although this is valid from a data flow
perspective it is wrong since it would mean that unallocated stack
slots are accessed i.e. stack slots *below* the stack pointer.

I've got aware of the problem due to a crash in C++ exception
handling.  The move of the stack pointer decrement led to corrupted
dwarf 2 register offsets which in turn made the unwinder crash.

The testcase I've is only triggered with a modification to the s390
pipeline description.


The attached patch implements the rs6000 solution for s390.  We now
emit a "stack tie" between the stack pointer decrement and the insns
writing the floating point stack slots on 64bit.  The stack tie is a
pattern writing a stack slot using the stack pointer with an UNSPEC
having the stack pointer register as parameter.  Neither the stack
pointer decrement nor the stack writing INSNs can be moved over the
"stack tie".  Therefore there is also no way one of these is moved
over each other with the stack tie sitting in between.


Bootstrapped and regtested on s390 and s390x.  I've verified that the
patch fixes the problem I had with the modified pipeline description.

I'll apply the patch in two days in order to wait for any comments.

Bye,

-Andreas-


2007-12-04  Andreas Krebbel  <krebbel1@de.ibm.com>

	* config/s390/s390.c (s390_emit_stack_tie): New function.
	(s390_emit_prologue): Emit a stack tie when writing to the
	f8-f15 stack slots.
	* config/s390/s390.md ("stack_tie"): New insn definition.


Index: gcc/config/s390/s390.c
===================================================================
*** gcc/config/s390/s390.c.orig	2007-12-03 17:22:04.000000000 +0100
--- gcc/config/s390/s390.c	2007-12-03 17:35:23.000000000 +0100
*************** s390_load_got (void)
*** 7162,7167 ****
--- 7162,7179 ----
    return insns;
  }
  
+ /* This ties together stack memory (MEM with an alias set of frame_alias_set)
+    and the change to the stack pointer.  */
+ 
+ static void
+ s390_emit_stack_tie (void)
+ {
+   rtx mem = gen_frame_mem (BLKmode,
+ 			   gen_rtx_REG (Pmode, STACK_POINTER_REGNUM));
+ 
+   emit_insn (gen_stack_tie (mem));
+ }
+ 
  /* Expand the prologue into a bunch of separate insns.  */
  
  void
*************** s390_emit_prologue (void)
*** 7390,7395 ****
--- 7402,7412 ----
  
    if (cfun_save_high_fprs_p && next_fpr)
      {
+       /* If the stack might be accessed through a different register
+ 	 we have to make sure that the stack pointer decrement is not
+ 	 moved below the use of the stack slots.  */
+       s390_emit_stack_tie ();
+ 
        insn = emit_insn (gen_add2_insn (temp_reg, 
  				       GEN_INT (cfun_frame_layout.f8_offset)));
  
Index: gcc/config/s390/s390.md
===================================================================
*** gcc/config/s390/s390.md.orig	2007-11-23 15:35:37.000000000 +0100
--- gcc/config/s390/s390.md	2007-12-03 17:35:59.000000000 +0100
***************
*** 61,66 ****
--- 61,67 ----
     (UNSPEC_CCU_TO_INT		2)
     (UNSPEC_CCZ_TO_INT		3)
     (UNSPEC_ICM			10)
+    (UNSPEC_TIE                  11)
  
     ; GOT/PLT and lt-relative accesses
     (UNSPEC_LTREL_OFFSET		100)
***************
*** 7971,7973 ****
--- 7972,7984 ----
    ""
    "clc\t%O0(%G0,%R0),%S1"
    [(set_attr "op_type" "SS")])
+ 
+ ; This is used in s390_emit_prologue in order to prevent insns
+ ; adjusting the stack pointer to be moved over insns writing stack
+ ; slots using a copy of the stack pointer in a different register.
+ (define_insn "stack_tie"
+   [(set (match_operand:BLK 0 "memory_operand" "+m")
+         (unspec:BLK [(match_dup 0)] UNSPEC_TIE))]
+   ""
+   ""
+   [(set_attr "length" "0")])