PR70674: S/390: Add memory barrier to stack pointer restore

PR70674: S/390: Add memory barrier to stack pointer restore
 from fpr.

This patches fixes a problem with stack variable accesses being
scheduled after the stack pointer restore instructions.  In the
testcase this happened with the stack variable 'a' accessed through the
frame pointer.

The existing stack_tie we have in the backend is basically useless
when trying to block stack variable accesses from being scheduled
across an insn.  The alias set of stack variables and the frame alias
set usually differ and hence aren't in conflict with each other.  The
solution appears to be a magic MEM term with a scratch register which
is handled as a full memory barrier when analyzing scheduling
dependencies.

With the patch a (clobber (mem:BLK (scratch))) is being added to the
restore instruction in order to prevent any memory operations to be
scheduled across the insn.  The patch does that only for the one case
where the stack pointer is restored from an FPR.  Theoretically this
might happen also in the case where the stack pointer gets restored
using a load multiple.  However, triggering that problem with
load-multiple appears to be much harder since the load-multiple will
restore the frame pointer as well.  So in order to see the problem a
different call-clobbered register would need to be used as temporary
stack pointer.

Another case which needs to be handled some day is the stack pointer
allocation part.  It needs to be a memory barrier as well.

gcc/ChangeLog:

2016-04-21  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>

	Backport from mainline
	2016-04-20  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>

	PR target/70674
	* config/s390/s390.c (s390_restore_gprs_from_fprs): Pick the new
	stack_restore_from_fpr pattern when restoring r15.
	(s390_optimize_prologue): Strip away the memory barrier in the
	parallel when trying to get rid of restore insns.
	* config/s390/s390.md ("stack_restore_from_fpr"): New insn
	definition for loading the stack pointer from an FPR.  Compared to
	the normal move insn this pattern includes a full memory barrier.

gcc/testsuite/ChangeLog:

2016-04-21  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>

	Backport from mainline
	2016-04-20  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>

	PR target/70674
	* gcc.target/s390/pr70674.c: New test.

From-SVN: r235334

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 4f31fc743301aa43d0b4792ef6c0fe807b002cea..d6d083faddde785af3e8b6c83f429af562368f23 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,17 @@
+2016-04-21  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>
+
+       Backport from mainline
+       2016-04-20  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>
+
+       PR target/70674
+       * config/s390/s390.c (s390_restore_gprs_from_fprs): Pick the new
+       stack_restore_from_fpr pattern when restoring r15.
+       (s390_optimize_prologue): Strip away the memory barrier in the
+       parallel when trying to get rid of restore insns.
+       * config/s390/s390.md ("stack_restore_from_fpr"): New insn
+       definition for loading the stack pointer from an FPR.  Compared to
+       the normal move insn this pattern includes a full memory barrier.
+
 2016-04-13  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
 
        Backport from mainline

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 2a52db9e9e4f65cdeffd27f42dd73cfd6c515adc..af910b9cacfb2b215d48c95e28c123b1742256a4 100644 (file)
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -8615,19 +8615,25 @@ s390_restore_gprs_from_fprs (void)
 
   for (i = 6; i < 16; i++)
     {
-      if (FP_REGNO_P (cfun_gpr_save_slot (i)))
-       {
-         rtx insn =
-           emit_move_insn (gen_rtx_REG (DImode, i),
-                           gen_rtx_REG (DImode, cfun_gpr_save_slot (i)));
-         df_set_regs_ever_live (i, true);
-         add_reg_note (insn, REG_CFA_RESTORE, gen_rtx_REG (DImode, i));
-         if (i == STACK_POINTER_REGNUM)
-           add_reg_note (insn, REG_CFA_DEF_CFA,
-                         plus_constant (Pmode, stack_pointer_rtx,
-                                        STACK_POINTER_OFFSET));
-         RTX_FRAME_RELATED_P (insn) = 1;
-       }
+      rtx insn;
+
+      if (!FP_REGNO_P (cfun_gpr_save_slot (i)))
+       continue;
+
+      rtx fpr = gen_rtx_REG (DImode, cfun_gpr_save_slot (i));
+
+      if (i == STACK_POINTER_REGNUM)
+       insn = emit_insn (gen_stack_restore_from_fpr (fpr));
+      else
+       insn = emit_move_insn (gen_rtx_REG (DImode, i), fpr);
+
+      df_set_regs_ever_live (i, true);
+      add_reg_note (insn, REG_CFA_RESTORE, gen_rtx_REG (DImode, i));
+      if (i == STACK_POINTER_REGNUM)
+       add_reg_note (insn, REG_CFA_DEF_CFA,
+                     plus_constant (Pmode, stack_pointer_rtx,
+                                    STACK_POINTER_OFFSET));
+      RTX_FRAME_RELATED_P (insn) = 1;
     }
 }
 
@@ -10912,37 +10918,46 @@ s390_optimize_prologue (void)
 
       /* Remove ldgr/lgdr instructions used for saving and restore
         GPRs if possible.  */
-      if (TARGET_Z10
-         && GET_CODE (pat) == SET
-         && GET_MODE (SET_SRC (pat)) == DImode
-         && REG_P (SET_SRC (pat))
-         && REG_P (SET_DEST (pat)))
+      if (TARGET_Z10)
        {
-         int src_regno = REGNO (SET_SRC (pat));
-         int dest_regno = REGNO (SET_DEST (pat));
-         int gpr_regno;
-         int fpr_regno;
+         rtx tmp_pat = pat;
 
-         if (!((GENERAL_REGNO_P (src_regno) && FP_REGNO_P (dest_regno))
-               || (FP_REGNO_P (src_regno) && GENERAL_REGNO_P (dest_regno))))
-           continue;
+         if (INSN_CODE (insn) == CODE_FOR_stack_restore_from_fpr)
+           tmp_pat = XVECEXP (pat, 0, 0);
 
-         gpr_regno = GENERAL_REGNO_P (src_regno) ? src_regno : dest_regno;
-         fpr_regno = FP_REGNO_P (src_regno) ? src_regno : dest_regno;
+         if (GET_CODE (tmp_pat) == SET
+             && GET_MODE (SET_SRC (tmp_pat)) == DImode
+             && REG_P (SET_SRC (tmp_pat))
+             && REG_P (SET_DEST (tmp_pat)))
+           {
+             int src_regno = REGNO (SET_SRC (tmp_pat));
+             int dest_regno = REGNO (SET_DEST (tmp_pat));
+             int gpr_regno;
+             int fpr_regno;
+
+             if (!((GENERAL_REGNO_P (src_regno)
+                    && FP_REGNO_P (dest_regno))
+                   || (FP_REGNO_P (src_regno)
+                       && GENERAL_REGNO_P (dest_regno))))
+               continue;
 
-         /* GPR must be call-saved, FPR must be call-clobbered.  */
-         if (!call_really_used_regs[fpr_regno]
-             || call_really_used_regs[gpr_regno])
-           continue;
+             gpr_regno = GENERAL_REGNO_P (src_regno) ? src_regno : dest_regno;
+             fpr_regno = FP_REGNO_P (src_regno) ? src_regno : dest_regno;
 
-         /* It must not happen that what we once saved in an FPR now
-            needs a stack slot.  */
-         gcc_assert (cfun_gpr_save_slot (gpr_regno) != -1);
+             /* GPR must be call-saved, FPR must be call-clobbered.  */
+             if (!call_really_used_regs[fpr_regno]
+                 || call_really_used_regs[gpr_regno])
+               continue;
 
-         if (cfun_gpr_save_slot (gpr_regno) == 0)
-           {
-             remove_insn (insn);
-             continue;
+             /* It must not happen that what we once saved in an FPR now
+                needs a stack slot.  */
+             gcc_assert (cfun_gpr_save_slot (gpr_regno) != -1);
+
+             if (cfun_gpr_save_slot (gpr_regno) == 0)
+               {
+                 remove_insn (insn);
+                 continue;
+               }
            }
        }
 

diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index 986b0c65b9a3e44e929ce0cf79af7a23cafa1e3d..cc9e39ef6c43b6bf8ccf6da4a2361c2c62b9e1fa 100644 (file)
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -183,6 +183,8 @@
    (BASE_REGNUM                        13)
    ; Return address register.
    (RETURN_REGNUM              14)
+   ; Stack pointer register.
+   (STACK_REGNUM               15)
    ; Condition code register.
    (CC_REGNUM                  33)
    ; Thread local storage pointer register.
@@ -9896,6 +9898,14 @@
   [(set_attr "length" "0")])
 
 
+(define_insn "stack_restore_from_fpr"
+  [(set (reg:DI STACK_REGNUM)
+       (match_operand:DI 0 "register_operand" "f"))
+   (clobber (mem:BLK (scratch)))]
+  "TARGET_Z10"
+  "lgdr\t%%r15,%0"
+  [(set_attr "op_type"  "RRE")])
+
 ;
 ; Data prefetch patterns
 ;

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 324810d7bb465335ad289146ebaa726fca27154c..868b415670f7f04c5eb487f8bde78b02a7db1dce 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2016-04-21  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>
+
+       Backport from mainline
+       2016-04-20  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>
+
+       PR target/70674
+       * gcc.target/s390/pr70674.c: New test.
+
 2016-04-13  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
 
        Backport from mainline

diff --git a/gcc/testsuite/gcc.target/s390/pr70674.c b/gcc/testsuite/gcc.target/s390/pr70674.c
new file mode 100644 (file)
index 0000000..13bf271
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/pr70674.c
@@ -0,0 +1,13 @@
+/* Test case for PR/70674.  */
+
+/* { dg-do compile { target s390x-*-* } } */
+/* { dg-options "-march=z10 -mtune=z196 -O2 -fno-omit-frame-pointer -fno-asynchronous-unwind-tables" } */
+
+void
+foo (void)
+{
+  volatile int a = 5;
+  (void) a;
+}
+
+/* { dg-final { scan-assembler-not "^.*lgdr.*\n.*\\(%r11\\)" } } */