[PATCH] S390: Hotpatching fixes.

[PATCH] S390: Hotpatching fixes.

[Dominik Vogt]

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

S390: Hotpatching fixes.

 * Properly align function labels with -mhotpatch and add test cases.
 * Include the nops after the function label in the area covered by cfi and
   debug information.
 * Correct a typo in the documentation.
 * Fix formatting in the generated 6-byte-NOP and adapt the test cases.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

[-- Attachment #2: 0001-S390-Hotpatching-fixes.patch --]
[-- Type: text/x-diff, Size: 19898 bytes --]

From 2d42c989a83fac102294ebdff6e68ca4bd571915 Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Mon, 23 Feb 2015 13:48:26 +0100
Subject: [PATCH] S390: Hotpatching fixes.

 * Properly align function labels with -mhotpatch and add test cases.
 * Include the nops after the function label in the area covered by cfi and
   debug information.
 * Correct a typo in the documentation.
 * Fix formatting in the generated 6-byte-NOP and adapt the test cases.
---
 gcc/config/s390/s390.c                      | 65 ++++++++++++++++++-----------
 gcc/config/s390/s390.md                     | 28 +++++++++++++
 gcc/doc/invoke.texi                         |  4 +-
 gcc/testsuite/gcc.target/s390/hotpatch-1.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-10.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-11.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-12.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-13.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-14.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-15.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-16.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-17.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-18.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-19.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-2.c  |  3 +-
 gcc/testsuite/gcc.target/s390/hotpatch-21.c | 14 +++++++
 gcc/testsuite/gcc.target/s390/hotpatch-22.c | 14 +++++++
 gcc/testsuite/gcc.target/s390/hotpatch-23.c | 14 +++++++
 gcc/testsuite/gcc.target/s390/hotpatch-24.c | 14 +++++++
 gcc/testsuite/gcc.target/s390/hotpatch-3.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-4.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-5.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-6.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-7.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-8.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-9.c  |  2 +-
 26 files changed, 146 insertions(+), 46 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/hotpatch-21.c
 create mode 100644 gcc/testsuite/gcc.target/s390/hotpatch-22.c
 create mode 100644 gcc/testsuite/gcc.target/s390/hotpatch-23.c
 create mode 100644 gcc/testsuite/gcc.target/s390/hotpatch-24.c

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 1924f2a..bac3555 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -5295,6 +5295,7 @@ s390_asm_output_function_label (FILE *asm_out_file, const char *fname,
 
   if (hotpatch_p)
     {
+      int function_alignment;
       int i;
 
       /* Add a trampoline code area before the function label and initialize it
@@ -5308,34 +5309,13 @@ s390_asm_output_function_label (FILE *asm_out_file, const char *fname,
 	 stored directly before the label without crossing a cacheline
 	 boundary.  All this is necessary to make sure the trampoline code can
 	 be changed atomically.  */
+      function_alignment = MAX (8, DECL_ALIGN (decl) / BITS_PER_UNIT);
+      if (! DECL_USER_ALIGN (decl))
+	function_alignment = MAX (function_alignment, align_functions);
+      ASM_OUTPUT_ALIGN (asm_out_file, floor_log2 (function_alignment));
     }
 
   ASM_OUTPUT_LABEL (asm_out_file, fname);
-
-  /* Output a series of NOPs after the function label.  */
-  if (hotpatch_p)
-    {
-      while (hw_after > 0)
-	{
-	  if (hw_after >= 3 && TARGET_CPU_ZARCH)
-	    {
-	      asm_fprintf (asm_out_file, "\tbrcl\t\t0,0\n");
-	      hw_after -= 3;
-	    }
-	  else if (hw_after >= 2)
-	    {
-	      gcc_assert (hw_after == 2 || !TARGET_CPU_ZARCH);
-	      asm_fprintf (asm_out_file, "\tnop\t0\n");
-	      hw_after -= 2;
-	    }
-	  else
-	    {
-	      gcc_assert (hw_after == 1);
-	      asm_fprintf (asm_out_file, "\tnopr\t%%r7\n");
-	      hw_after -= 1;
-	    }
-	}
-    }
 }
 
 /* Output machine-dependent UNSPECs occurring in address constant X
@@ -11368,6 +11348,7 @@ static void
 s390_reorg (void)
 {
   bool pool_overflow = false;
+  int hw_before, hw_after;
 
   /* Make sure all splits have been performed; splits after
      machine_dependent_reorg might confuse insn length counts.  */
@@ -11503,6 +11484,40 @@ s390_reorg (void)
       if (insn_added_p)
 	shorten_branches (get_insns ());
     }
+
+  s390_function_num_hotpatch_hw (current_function_decl, &hw_before, &hw_after);
+  if (hw_after > 0)
+    {
+      rtx_insn *insn;
+
+      /* Inject nops for hotpatching. */
+      for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
+	{
+	  if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_FUNCTION_BEG)
+	    break;
+	}
+      gcc_assert (insn);
+      /* Output a series of NOPs after the NOTE_INSN_FUNCTION_BEG.  */
+      while (hw_after > 0)
+	{
+	  if (hw_after >= 3 && TARGET_CPU_ZARCH)
+	    {
+	      insn = emit_insn_after (gen_nop_6_byte (), insn);
+	      hw_after -= 3;
+	    }
+	  else if (hw_after >= 2)
+	    {
+	      insn = emit_insn_after (gen_nop_4_byte (), insn);
+	      hw_after -= 2;
+	    }
+	  else
+	    {
+	      insn = emit_insn_after (gen_nop_2_byte (), insn);
+	      hw_after -= 1;
+	    }
+	}
+      gcc_assert (hw_after == 0);
+    }
 }
 
 /* Return true if INSN is a fp load insn writing register REGNO.  */
diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index bf698b1..afc418b 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -153,6 +153,11 @@
    UNSPECV_CAS
    UNSPECV_ATOMIC_OP
 
+   ; Hotpatching (unremovable NOPs)
+   UNSPECV_NOP_2_BYTE
+   UNSPECV_NOP_4_BYTE
+   UNSPECV_NOP_6_BYTE
+
    ; Transactional Execution support
    UNSPECV_TBEGIN
    UNSPECV_TBEGIN_TDB
@@ -10094,6 +10099,29 @@
 
 
 ;;
+;;- Undeletable nops (used for hotpatching)
+;;
+
+(define_insn "nop_2_byte"
+  [(unspec_volatile [(const_int 0)] UNSPECV_NOP_2_BYTE)]
+  ""
+  "nopr\t%%r7"
+  [(set_attr "op_type" "RR")])
+
+(define_insn "nop_4_byte"
+  [(unspec_volatile [(const_int 0)] UNSPECV_NOP_4_BYTE)]
+  ""
+  "nop\t0"
+  [(set_attr "op_type" "RX")])
+
+(define_insn "nop_6_byte"
+  [(unspec_volatile [(const_int 0)] UNSPECV_NOP_6_BYTE)]
+  ""
+  "brcl\t0, 0"
+  [(set_attr "op_type" "RIL")])
+
+
+;;
 ;;- Transactional execution instructions
 ;;
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index a87376e..3133c01 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -20045,9 +20045,9 @@ The @var{stack-guard} option can only be used in conjunction with @var{stack-siz
 If the hotpatch option is enabled, a ``hot-patching'' function
 prologue is generated for all functions in the compilation unit.
 The funtion label is prepended with the given number of two-byte
-Nop instructions (@var{pre-halfwords}, maximum 1000000).  After
+NOP instructions (@var{pre-halfwords}, maximum 1000000).  After
 the label, 2 * @var{post-halfwords} bytes are appended, using the
-larges nop like instructions the architecture allows (maximum
+largest NOP like instructions the architecture allows (maximum
 1000000).
 
 If both arguments are zero, hotpatching is disabled.
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-1.c b/gcc/testsuite/gcc.target/s390/hotpatch-1.c
index 61d8f39..4e29817 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-1.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-1.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-10.c b/gcc/testsuite/gcc.target/s390/hotpatch-10.c
index 1097ab4..352c9d5 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-10.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-10.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-11.c b/gcc/testsuite/gcc.target/s390/hotpatch-11.c
index be08acc..beb8738 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-11.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-11.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-12.c b/gcc/testsuite/gcc.target/s390/hotpatch-12.c
index 6e909c0..d109478 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-12.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-12.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 999 } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-13.c b/gcc/testsuite/gcc.target/s390/hotpatch-13.c
index 959789c..9ac9f84 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-13.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-13.c
@@ -14,4 +14,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-14.c b/gcc/testsuite/gcc.target/s390/hotpatch-14.c
index 0f1f41f..209e4ac 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-14.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-14.c
@@ -14,4 +14,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-15.c b/gcc/testsuite/gcc.target/s390/hotpatch-15.c
index e306495..109034e 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-15.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-15.c
@@ -14,4 +14,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-16.c b/gcc/testsuite/gcc.target/s390/hotpatch-16.c
index 66f13fe..b01c1c3 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-16.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-16.c
@@ -14,4 +14,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-17.c b/gcc/testsuite/gcc.target/s390/hotpatch-17.c
index b301c5c..ce0b950 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-17.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-17.c
@@ -14,4 +14,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-18.c b/gcc/testsuite/gcc.target/s390/hotpatch-18.c
index 1c06783..9a24d5e 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-18.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-18.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-19.c b/gcc/testsuite/gcc.target/s390/hotpatch-19.c
index a1ec9c7..3586da2 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-19.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-19.c
@@ -20,4 +20,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-2.c b/gcc/testsuite/gcc.target/s390/hotpatch-2.c
index 0c82a5a..18be5aa 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-2.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-2.c
@@ -13,4 +13,5 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
+/* { dg-final { scan-assembler-times "\.align\t8" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-21.c b/gcc/testsuite/gcc.target/s390/hotpatch-21.c
new file mode 100644
index 0000000..d72dee3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-21.c
@@ -0,0 +1,14 @@
+/* Functional tests for the function hotpatching feature.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 -mzarch -mhotpatch=0,1" } */
+
+#include <stdio.h>
+
+void __attribute__ ((aligned(512))) hp1(void)
+{
+  printf("hello, world!\n");
+}
+
+/* Check number of occurences of certain instructions.  */
+/* { dg-final { scan-assembler-times "\.align\t512" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-22.c b/gcc/testsuite/gcc.target/s390/hotpatch-22.c
new file mode 100644
index 0000000..6507466
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-22.c
@@ -0,0 +1,14 @@
+/* Functional tests for the function hotpatching feature.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 -mzarch -mhotpatch=0,1 -falign-functions=1024" } */
+
+#include <stdio.h>
+
+void hp1(void)
+{
+  printf("hello, world!\n");
+}
+
+/* Check number of occurences of certain instructions.  */
+/* { dg-final { scan-assembler-times "\.align\t1024" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-23.c b/gcc/testsuite/gcc.target/s390/hotpatch-23.c
new file mode 100644
index 0000000..7dce5b8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-23.c
@@ -0,0 +1,14 @@
+/* Functional tests for the function hotpatching feature.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 -mzarch -mhotpatch=0,1 -falign-functions=4096" } */
+
+#include <stdio.h>
+
+void __attribute__ ((aligned(2048))) hp1(void)
+{
+  printf("hello, world!\n");
+}
+
+/* Check number of occurences of certain instructions.  */
+/* { dg-final { scan-assembler-times "\.align\t2048" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-24.c b/gcc/testsuite/gcc.target/s390/hotpatch-24.c
new file mode 100644
index 0000000..4d6815c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-24.c
@@ -0,0 +1,14 @@
+/* Functional tests for the function hotpatching feature.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 -mzarch -mhotpatch=0,1 -falign-functions=2048" } */
+
+#include <stdio.h>
+
+void __attribute__ ((aligned(4096))) hp1(void)
+{
+  printf("hello, world!\n");
+}
+
+/* Check number of occurences of certain instructions.  */
+/* { dg-final { scan-assembler-times "\.align\t4096" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-3.c b/gcc/testsuite/gcc.target/s390/hotpatch-3.c
index 041ef9c..8a2ab53 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-3.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-3.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-4.c b/gcc/testsuite/gcc.target/s390/hotpatch-4.c
index 050a065..9a69459 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-4.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-4.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-times "brcl\t\t0,0" 1 } } */
+/* { dg-final { scan-assembler-times "brcl\t0, 0" 1 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-5.c b/gcc/testsuite/gcc.target/s390/hotpatch-5.c
index 785dba7..b3d990f 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-5.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-5.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-times "brcl\t\t0,0" 1 } } */
+/* { dg-final { scan-assembler-times "brcl\t0, 0" 1 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-6.c b/gcc/testsuite/gcc.target/s390/hotpatch-6.c
index da96e43..10ba558 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-6.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-6.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-times "brcl\t\t0,0" 1 } } */
+/* { dg-final { scan-assembler-times "brcl\t0, 0" 1 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-7.c b/gcc/testsuite/gcc.target/s390/hotpatch-7.c
index b7c19d7..edf67d3 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-7.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-7.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-times "brcl\t\t0,0" 2 } } */
+/* { dg-final { scan-assembler-times "brcl\t0, 0" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-8.c b/gcc/testsuite/gcc.target/s390/hotpatch-8.c
index 25edd9b..a179c4a 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-8.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-8.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-9.c b/gcc/testsuite/gcc.target/s390/hotpatch-9.c
index 2143f9d..c4e21f2 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-9.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-9.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-times "nop\t0" 2 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
-- 
1.9.3


[-- Attachment #3: ChangeLog --]
[-- Type: text/plain, Size: 607 bytes --]

2015-03-02  Dominik Vogt  <vogt@linux.vnet.ibm.com>

	* config/s390/s390.c (s390_reorg): Move code to output nops after label
	to s390_reorg ().
	(s390_asm_output_function_label): Likewise.
	* config/s390/s390.c (s390_asm_output_function_label):
	Fix function label alignment with -mhtopatch.
	* config/s390/s390.md ("unspecv"): New values UNSPECV_NOP_2_BYTE,
	UNSPECV_NOP_4_BYTE and UNSPECV_NOP_6_BYTE
	("nop_2_byte"): New define_insn.
	("nop_4_byte"): Likewise.
	("nop_6_byte"): Likewise.
	* doc/extend.texi (hotpatch): hotpatch attribute doc fixes.
	* doc/invoke.texi (-mhotpatch): -mhotpatch doc fixes.

Re: [PATCH] S390: Hotpatching fixes.

[Dominik Vogt]

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

Updated patch after internal review:

 * Moved the hotpatch specific NOP patterns to the normal NOP
   patterns in the .md file.
 * Make function_alignment unsigned and cast align_function
   instead.

(ChangeLog is still the same.)

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

[-- Attachment #2: 5.0-0001-S390-Hotpatching-fixes.patch --]
[-- Type: text/x-diff, Size: 19967 bytes --]

From e0083a3044797bf13ebdc9294fb0ebc117cbed4b Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Mon, 23 Feb 2015 13:48:26 +0100
Subject: [PATCH] S390: Hotpatching fixes.

 * Properly align function labels with -mhotpatch and add test cases.
 * Include the nops after the function label in the area covered by cfi and
   debug information.
 * Correct a typo in the documentation.
 * Fix formatting in the generated 6-byte-NOP and adapt the test cases.
---
 gcc/config/s390/s390.c                      | 66 ++++++++++++++++++-----------
 gcc/config/s390/s390.md                     | 25 +++++++++++
 gcc/doc/invoke.texi                         |  4 +-
 gcc/testsuite/gcc.target/s390/hotpatch-1.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-10.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-11.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-12.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-13.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-14.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-15.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-16.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-17.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-18.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-19.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-2.c  |  3 +-
 gcc/testsuite/gcc.target/s390/hotpatch-21.c | 14 ++++++
 gcc/testsuite/gcc.target/s390/hotpatch-22.c | 14 ++++++
 gcc/testsuite/gcc.target/s390/hotpatch-23.c | 14 ++++++
 gcc/testsuite/gcc.target/s390/hotpatch-24.c | 14 ++++++
 gcc/testsuite/gcc.target/s390/hotpatch-3.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-4.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-5.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-6.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-7.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-8.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-9.c  |  2 +-
 26 files changed, 144 insertions(+), 46 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/hotpatch-21.c
 create mode 100644 gcc/testsuite/gcc.target/s390/hotpatch-22.c
 create mode 100644 gcc/testsuite/gcc.target/s390/hotpatch-23.c
 create mode 100644 gcc/testsuite/gcc.target/s390/hotpatch-24.c

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 1924f2a..d0584e8 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -5295,6 +5295,7 @@ s390_asm_output_function_label (FILE *asm_out_file, const char *fname,
 
   if (hotpatch_p)
     {
+      unsigned int function_alignment;
       int i;
 
       /* Add a trampoline code area before the function label and initialize it
@@ -5308,34 +5309,14 @@ s390_asm_output_function_label (FILE *asm_out_file, const char *fname,
 	 stored directly before the label without crossing a cacheline
 	 boundary.  All this is necessary to make sure the trampoline code can
 	 be changed atomically.  */
+      function_alignment = MAX (8, DECL_ALIGN (decl) / BITS_PER_UNIT);
+      if (! DECL_USER_ALIGN (decl))
+	      function_alignment = MAX (function_alignment,
+					(unsigned int) align_functions);
+      ASM_OUTPUT_ALIGN (asm_out_file, floor_log2 (function_alignment));
     }
 
   ASM_OUTPUT_LABEL (asm_out_file, fname);
-
-  /* Output a series of NOPs after the function label.  */
-  if (hotpatch_p)
-    {
-      while (hw_after > 0)
-	{
-	  if (hw_after >= 3 && TARGET_CPU_ZARCH)
-	    {
-	      asm_fprintf (asm_out_file, "\tbrcl\t\t0,0\n");
-	      hw_after -= 3;
-	    }
-	  else if (hw_after >= 2)
-	    {
-	      gcc_assert (hw_after == 2 || !TARGET_CPU_ZARCH);
-	      asm_fprintf (asm_out_file, "\tnop\t0\n");
-	      hw_after -= 2;
-	    }
-	  else
-	    {
-	      gcc_assert (hw_after == 1);
-	      asm_fprintf (asm_out_file, "\tnopr\t%%r7\n");
-	      hw_after -= 1;
-	    }
-	}
-    }
 }
 
 /* Output machine-dependent UNSPECs occurring in address constant X
@@ -11368,6 +11349,7 @@ static void
 s390_reorg (void)
 {
   bool pool_overflow = false;
+  int hw_before, hw_after;
 
   /* Make sure all splits have been performed; splits after
      machine_dependent_reorg might confuse insn length counts.  */
@@ -11503,6 +11485,40 @@ s390_reorg (void)
       if (insn_added_p)
 	shorten_branches (get_insns ());
     }
+
+  s390_function_num_hotpatch_hw (current_function_decl, &hw_before, &hw_after);
+  if (hw_after > 0)
+    {
+      rtx_insn *insn;
+
+      /* Inject nops for hotpatching. */
+      for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
+	{
+	  if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_FUNCTION_BEG)
+	    break;
+	}
+      gcc_assert (insn);
+      /* Output a series of NOPs after the NOTE_INSN_FUNCTION_BEG.  */
+      while (hw_after > 0)
+	{
+	  if (hw_after >= 3 && TARGET_CPU_ZARCH)
+	    {
+	      insn = emit_insn_after (gen_nop_6_byte (), insn);
+	      hw_after -= 3;
+	    }
+	  else if (hw_after >= 2)
+	    {
+	      insn = emit_insn_after (gen_nop_4_byte (), insn);
+	      hw_after -= 2;
+	    }
+	  else
+	    {
+	      insn = emit_insn_after (gen_nop_2_byte (), insn);
+	      hw_after -= 1;
+	    }
+	}
+      gcc_assert (hw_after == 0);
+    }
 }
 
 /* Return true if INSN is a fp load insn writing register REGNO.  */
diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index bf698b1..c6001bb 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -153,6 +153,11 @@
    UNSPECV_CAS
    UNSPECV_ATOMIC_OP
 
+   ; Hotpatching (unremovable NOPs)
+   UNSPECV_NOP_2_BYTE
+   UNSPECV_NOP_4_BYTE
+   UNSPECV_NOP_6_BYTE
+
    ; Transactional Execution support
    UNSPECV_TBEGIN
    UNSPECV_TBEGIN_TDB
@@ -9705,6 +9710,26 @@
   "lr\t1,1"
   [(set_attr "op_type" "RR")])
 
+;;- Undeletable nops (used for hotpatching)
+
+(define_insn "nop_2_byte"
+  [(unspec_volatile [(const_int 0)] UNSPECV_NOP_2_BYTE)]
+  ""
+  "nopr\t%%r7"
+  [(set_attr "op_type" "RR")])
+
+(define_insn "nop_4_byte"
+  [(unspec_volatile [(const_int 0)] UNSPECV_NOP_4_BYTE)]
+  ""
+  "nop\t0"
+  [(set_attr "op_type" "RX")])
+
+(define_insn "nop_6_byte"
+  [(unspec_volatile [(const_int 0)] UNSPECV_NOP_6_BYTE)]
+  ""
+  "brcl\t0, 0"
+  [(set_attr "op_type" "RIL")])
+
 
 ;
 ; Special literal pool access instruction pattern(s).
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index a87376e..3133c01 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -20045,9 +20045,9 @@ The @var{stack-guard} option can only be used in conjunction with @var{stack-siz
 If the hotpatch option is enabled, a ``hot-patching'' function
 prologue is generated for all functions in the compilation unit.
 The funtion label is prepended with the given number of two-byte
-Nop instructions (@var{pre-halfwords}, maximum 1000000).  After
+NOP instructions (@var{pre-halfwords}, maximum 1000000).  After
 the label, 2 * @var{post-halfwords} bytes are appended, using the
-larges nop like instructions the architecture allows (maximum
+largest NOP like instructions the architecture allows (maximum
 1000000).
 
 If both arguments are zero, hotpatching is disabled.
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-1.c b/gcc/testsuite/gcc.target/s390/hotpatch-1.c
index 61d8f39..4e29817 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-1.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-1.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-10.c b/gcc/testsuite/gcc.target/s390/hotpatch-10.c
index 1097ab4..352c9d5 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-10.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-10.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-11.c b/gcc/testsuite/gcc.target/s390/hotpatch-11.c
index be08acc..beb8738 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-11.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-11.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-12.c b/gcc/testsuite/gcc.target/s390/hotpatch-12.c
index 6e909c0..d109478 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-12.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-12.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 999 } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-13.c b/gcc/testsuite/gcc.target/s390/hotpatch-13.c
index 959789c..9ac9f84 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-13.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-13.c
@@ -14,4 +14,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-14.c b/gcc/testsuite/gcc.target/s390/hotpatch-14.c
index 0f1f41f..209e4ac 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-14.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-14.c
@@ -14,4 +14,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-15.c b/gcc/testsuite/gcc.target/s390/hotpatch-15.c
index e306495..109034e 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-15.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-15.c
@@ -14,4 +14,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-16.c b/gcc/testsuite/gcc.target/s390/hotpatch-16.c
index 66f13fe..b01c1c3 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-16.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-16.c
@@ -14,4 +14,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-17.c b/gcc/testsuite/gcc.target/s390/hotpatch-17.c
index b301c5c..ce0b950 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-17.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-17.c
@@ -14,4 +14,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-18.c b/gcc/testsuite/gcc.target/s390/hotpatch-18.c
index 1c06783..9a24d5e 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-18.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-18.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-19.c b/gcc/testsuite/gcc.target/s390/hotpatch-19.c
index a1ec9c7..3586da2 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-19.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-19.c
@@ -20,4 +20,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-2.c b/gcc/testsuite/gcc.target/s390/hotpatch-2.c
index 0c82a5a..18be5aa 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-2.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-2.c
@@ -13,4 +13,5 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
+/* { dg-final { scan-assembler-times "\.align\t8" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-21.c b/gcc/testsuite/gcc.target/s390/hotpatch-21.c
new file mode 100644
index 0000000..d72dee3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-21.c
@@ -0,0 +1,14 @@
+/* Functional tests for the function hotpatching feature.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 -mzarch -mhotpatch=0,1" } */
+
+#include <stdio.h>
+
+void __attribute__ ((aligned(512))) hp1(void)
+{
+  printf("hello, world!\n");
+}
+
+/* Check number of occurences of certain instructions.  */
+/* { dg-final { scan-assembler-times "\.align\t512" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-22.c b/gcc/testsuite/gcc.target/s390/hotpatch-22.c
new file mode 100644
index 0000000..6507466
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-22.c
@@ -0,0 +1,14 @@
+/* Functional tests for the function hotpatching feature.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 -mzarch -mhotpatch=0,1 -falign-functions=1024" } */
+
+#include <stdio.h>
+
+void hp1(void)
+{
+  printf("hello, world!\n");
+}
+
+/* Check number of occurences of certain instructions.  */
+/* { dg-final { scan-assembler-times "\.align\t1024" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-23.c b/gcc/testsuite/gcc.target/s390/hotpatch-23.c
new file mode 100644
index 0000000..7dce5b8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-23.c
@@ -0,0 +1,14 @@
+/* Functional tests for the function hotpatching feature.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 -mzarch -mhotpatch=0,1 -falign-functions=4096" } */
+
+#include <stdio.h>
+
+void __attribute__ ((aligned(2048))) hp1(void)
+{
+  printf("hello, world!\n");
+}
+
+/* Check number of occurences of certain instructions.  */
+/* { dg-final { scan-assembler-times "\.align\t2048" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-24.c b/gcc/testsuite/gcc.target/s390/hotpatch-24.c
new file mode 100644
index 0000000..4d6815c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-24.c
@@ -0,0 +1,14 @@
+/* Functional tests for the function hotpatching feature.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 -mzarch -mhotpatch=0,1 -falign-functions=2048" } */
+
+#include <stdio.h>
+
+void __attribute__ ((aligned(4096))) hp1(void)
+{
+  printf("hello, world!\n");
+}
+
+/* Check number of occurences of certain instructions.  */
+/* { dg-final { scan-assembler-times "\.align\t4096" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-3.c b/gcc/testsuite/gcc.target/s390/hotpatch-3.c
index 041ef9c..8a2ab53 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-3.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-3.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-4.c b/gcc/testsuite/gcc.target/s390/hotpatch-4.c
index 050a065..9a69459 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-4.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-4.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-times "brcl\t\t0,0" 1 } } */
+/* { dg-final { scan-assembler-times "brcl\t0, 0" 1 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-5.c b/gcc/testsuite/gcc.target/s390/hotpatch-5.c
index 785dba7..b3d990f 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-5.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-5.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-times "brcl\t\t0,0" 1 } } */
+/* { dg-final { scan-assembler-times "brcl\t0, 0" 1 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-6.c b/gcc/testsuite/gcc.target/s390/hotpatch-6.c
index da96e43..10ba558 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-6.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-6.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-times "brcl\t\t0,0" 1 } } */
+/* { dg-final { scan-assembler-times "brcl\t0, 0" 1 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-7.c b/gcc/testsuite/gcc.target/s390/hotpatch-7.c
index b7c19d7..edf67d3 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-7.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-7.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-times "brcl\t\t0,0" 2 } } */
+/* { dg-final { scan-assembler-times "brcl\t0, 0" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-8.c b/gcc/testsuite/gcc.target/s390/hotpatch-8.c
index 25edd9b..a179c4a 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-8.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-8.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-9.c b/gcc/testsuite/gcc.target/s390/hotpatch-9.c
index 2143f9d..c4e21f2 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-9.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-9.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-times "nop\t0" 2 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
-- 
1.9.3


[-- Attachment #3: 5.0-ChangeLog --]
[-- Type: text/plain, Size: 1766 bytes --]

2015-03-02  Dominik Vogt  <vogt@linux.vnet.ibm.com>

	* config/s390/s390.c (s390_reorg): Move code to output nops after label
	to s390_reorg ().
	(s390_asm_output_function_label): Likewise.
	* config/s390/s390.c (s390_asm_output_function_label):
	Fix function label alignment with -mhtopatch.
	* config/s390/s390.md ("unspecv"): New values UNSPECV_NOP_2_BYTE,
	UNSPECV_NOP_4_BYTE and UNSPECV_NOP_6_BYTE
	("nop_2_byte"): New define_insn.
	("nop_4_byte"): Likewise.
	("nop_6_byte"): Likewise.
	* doc/extend.texi (hotpatch): hotpatch attribute doc fixes.
	* doc/invoke.texi (-mhotpatch): -mhotpatch doc fixes.

2015-03-02  Dominik Vogt  <vogt@linux.vnet.ibm.com>

	* gcc.target/s390/hotpatch-21.c: New test for hotpatch alignment.
	* gcc.target/s390/hotpatch-22.c: Likewise.
	* gcc.target/s390/hotpatch-23.c: Likewise.
	* gcc.target/s390/hotpatch-24.c: Likewise.
	* gcc.target/s390/hotpatch-2.c: Also check hotpatch alignment.
	* gcc.target/s390/hotpatch-1.c: Update expected output.
	* gcc.target/s390/hotpatch-2.c: Likewise.
	* gcc.target/s390/hotpatch-3.c: Likewise.
	* gcc.target/s390/hotpatch-4.c: Likewise.
	* gcc.target/s390/hotpatch-5.c: Likewise.
	* gcc.target/s390/hotpatch-6.c: Likewise.
	* gcc.target/s390/hotpatch-7.c: Likewise.
	* gcc.target/s390/hotpatch-8.c: Likewise.
	* gcc.target/s390/hotpatch-9.c: Likewise.
	* gcc.target/s390/hotpatch-10.c: Likewise.
	* gcc.target/s390/hotpatch-11.c: Likewise.
	* gcc.target/s390/hotpatch-12.c: Likewise.
	* gcc.target/s390/hotpatch-13.c: Likewise.
	* gcc.target/s390/hotpatch-14.c: Likewise.
	* gcc.target/s390/hotpatch-15.c: Likewise.
	* gcc.target/s390/hotpatch-16.c: Likewise.
	* gcc.target/s390/hotpatch-17.c: Likewise.
	* gcc.target/s390/hotpatch-18.c: Likewise.
	* gcc.target/s390/hotpatch-19.c: Likewise.

Re: [PATCH] S390: Hotpatching fixes.

[Jakub Jelinek]

On Mon, Mar 09, 2015 at 12:22:21PM +0100, Dominik Vogt wrote:
> @@ -5308,34 +5309,14 @@ s390_asm_output_function_label (FILE *asm_out_file, const char *fname,
>  	 stored directly before the label without crossing a cacheline
>  	 boundary.  All this is necessary to make sure the trampoline code can
>  	 be changed atomically.  */
> +      function_alignment = MAX (8, DECL_ALIGN (decl) / BITS_PER_UNIT);
> +      if (! DECL_USER_ALIGN (decl))
> +	      function_alignment = MAX (function_alignment,
> +					(unsigned int) align_functions);

Wrong formatting.  function_alignment should be only 2 columns to the right
from if.

> +(define_insn "nop_6_byte"
> +  [(unspec_volatile [(const_int 0)] UNSPECV_NOP_6_BYTE)]
> +  ""

Shouldn't this use "TARGET_ZARCH" condition?

BTW, have you tried say:
__attribute__((hotpatch (0, 6))) void foo (void) { __builtin_unreachable (); }
?
Have you verified there never are real instructions before
NOTE_INSN_FUNCTION_BEG?

	Jakub

Re: [PATCH] S390: Hotpatching fixes.

[Dominik Vogt]

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

New patch with review results:

 * 6-byte-NOP only for ZARCH
 * Formatting.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

[-- Attachment #2: 5.0-0001-S390-Hotpatching-fixes.patch --]
[-- Type: text/x-diff, Size: 19982 bytes --]

From 176268849643c46427ea873c35390700ea7a4489 Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Mon, 23 Feb 2015 13:48:26 +0100
Subject: [PATCH 1/2] S390: Hotpatching fixes.

 * Properly align function labels with -mhotpatch and add test cases.
 * Include the nops after the function label in the area covered by cfi and
   debug information.
 * Correct a typo in the documentation.
 * Fix formatting in the generated 6-byte-NOP and adapt the test cases.
---
 gcc/config/s390/s390.c                      | 66 ++++++++++++++++++-----------
 gcc/config/s390/s390.md                     | 25 +++++++++++
 gcc/doc/invoke.texi                         |  4 +-
 gcc/testsuite/gcc.target/s390/hotpatch-1.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-10.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-11.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-12.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-13.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-14.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-15.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-16.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-17.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-18.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-19.c |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-2.c  |  3 +-
 gcc/testsuite/gcc.target/s390/hotpatch-21.c | 14 ++++++
 gcc/testsuite/gcc.target/s390/hotpatch-22.c | 14 ++++++
 gcc/testsuite/gcc.target/s390/hotpatch-23.c | 14 ++++++
 gcc/testsuite/gcc.target/s390/hotpatch-24.c | 14 ++++++
 gcc/testsuite/gcc.target/s390/hotpatch-3.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-4.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-5.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-6.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-7.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-8.c  |  2 +-
 gcc/testsuite/gcc.target/s390/hotpatch-9.c  |  2 +-
 26 files changed, 144 insertions(+), 46 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/hotpatch-21.c
 create mode 100644 gcc/testsuite/gcc.target/s390/hotpatch-22.c
 create mode 100644 gcc/testsuite/gcc.target/s390/hotpatch-23.c
 create mode 100644 gcc/testsuite/gcc.target/s390/hotpatch-24.c

diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 1924f2a..c907577 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -5295,6 +5295,7 @@ s390_asm_output_function_label (FILE *asm_out_file, const char *fname,
 
   if (hotpatch_p)
     {
+      unsigned int function_alignment;
       int i;
 
       /* Add a trampoline code area before the function label and initialize it
@@ -5308,34 +5309,14 @@ s390_asm_output_function_label (FILE *asm_out_file, const char *fname,
 	 stored directly before the label without crossing a cacheline
 	 boundary.  All this is necessary to make sure the trampoline code can
 	 be changed atomically.  */
+      function_alignment = MAX (8, DECL_ALIGN (decl) / BITS_PER_UNIT);
+      if (! DECL_USER_ALIGN (decl))
+	function_alignment = MAX (function_alignment,
+				  (unsigned int) align_functions);
+      ASM_OUTPUT_ALIGN (asm_out_file, floor_log2 (function_alignment));
     }
 
   ASM_OUTPUT_LABEL (asm_out_file, fname);
-
-  /* Output a series of NOPs after the function label.  */
-  if (hotpatch_p)
-    {
-      while (hw_after > 0)
-	{
-	  if (hw_after >= 3 && TARGET_CPU_ZARCH)
-	    {
-	      asm_fprintf (asm_out_file, "\tbrcl\t\t0,0\n");
-	      hw_after -= 3;
-	    }
-	  else if (hw_after >= 2)
-	    {
-	      gcc_assert (hw_after == 2 || !TARGET_CPU_ZARCH);
-	      asm_fprintf (asm_out_file, "\tnop\t0\n");
-	      hw_after -= 2;
-	    }
-	  else
-	    {
-	      gcc_assert (hw_after == 1);
-	      asm_fprintf (asm_out_file, "\tnopr\t%%r7\n");
-	      hw_after -= 1;
-	    }
-	}
-    }
 }
 
 /* Output machine-dependent UNSPECs occurring in address constant X
@@ -11368,6 +11349,7 @@ static void
 s390_reorg (void)
 {
   bool pool_overflow = false;
+  int hw_before, hw_after;
 
   /* Make sure all splits have been performed; splits after
      machine_dependent_reorg might confuse insn length counts.  */
@@ -11503,6 +11485,40 @@ s390_reorg (void)
       if (insn_added_p)
 	shorten_branches (get_insns ());
     }
+
+  s390_function_num_hotpatch_hw (current_function_decl, &hw_before, &hw_after);
+  if (hw_after > 0)
+    {
+      rtx_insn *insn;
+
+      /* Inject nops for hotpatching. */
+      for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
+	{
+	  if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_FUNCTION_BEG)
+	    break;
+	}
+      gcc_assert (insn);
+      /* Output a series of NOPs after the NOTE_INSN_FUNCTION_BEG.  */
+      while (hw_after > 0)
+	{
+	  if (hw_after >= 3 && TARGET_CPU_ZARCH)
+	    {
+	      insn = emit_insn_after (gen_nop_6_byte (), insn);
+	      hw_after -= 3;
+	    }
+	  else if (hw_after >= 2)
+	    {
+	      insn = emit_insn_after (gen_nop_4_byte (), insn);
+	      hw_after -= 2;
+	    }
+	  else
+	    {
+	      insn = emit_insn_after (gen_nop_2_byte (), insn);
+	      hw_after -= 1;
+	    }
+	}
+      gcc_assert (hw_after == 0);
+    }
 }
 
 /* Return true if INSN is a fp load insn writing register REGNO.  */
diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index bf698b1..76dca0a 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -153,6 +153,11 @@
    UNSPECV_CAS
    UNSPECV_ATOMIC_OP
 
+   ; Hotpatching (unremovable NOPs)
+   UNSPECV_NOP_2_BYTE
+   UNSPECV_NOP_4_BYTE
+   UNSPECV_NOP_6_BYTE
+
    ; Transactional Execution support
    UNSPECV_TBEGIN
    UNSPECV_TBEGIN_TDB
@@ -9705,6 +9710,26 @@
   "lr\t1,1"
   [(set_attr "op_type" "RR")])
 
+;;- Undeletable nops (used for hotpatching)
+
+(define_insn "nop_2_byte"
+  [(unspec_volatile [(const_int 0)] UNSPECV_NOP_2_BYTE)]
+  ""
+  "nopr\t%%r7"
+  [(set_attr "op_type" "RR")])
+
+(define_insn "nop_4_byte"
+  [(unspec_volatile [(const_int 0)] UNSPECV_NOP_4_BYTE)]
+  ""
+  "nop\t0"
+  [(set_attr "op_type" "RX")])
+
+(define_insn "nop_6_byte"
+  [(unspec_volatile [(const_int 0)] UNSPECV_NOP_6_BYTE)]
+  "TARGET_CPU_ZARCH"
+  "brcl\t0, 0"
+  [(set_attr "op_type" "RIL")])
+
 
 ;
 ; Special literal pool access instruction pattern(s).
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index a87376e..3133c01 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -20045,9 +20045,9 @@ The @var{stack-guard} option can only be used in conjunction with @var{stack-siz
 If the hotpatch option is enabled, a ``hot-patching'' function
 prologue is generated for all functions in the compilation unit.
 The funtion label is prepended with the given number of two-byte
-Nop instructions (@var{pre-halfwords}, maximum 1000000).  After
+NOP instructions (@var{pre-halfwords}, maximum 1000000).  After
 the label, 2 * @var{post-halfwords} bytes are appended, using the
-larges nop like instructions the architecture allows (maximum
+largest NOP like instructions the architecture allows (maximum
 1000000).
 
 If both arguments are zero, hotpatching is disabled.
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-1.c b/gcc/testsuite/gcc.target/s390/hotpatch-1.c
index 61d8f39..4e29817 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-1.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-1.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-10.c b/gcc/testsuite/gcc.target/s390/hotpatch-10.c
index 1097ab4..352c9d5 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-10.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-10.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-11.c b/gcc/testsuite/gcc.target/s390/hotpatch-11.c
index be08acc..beb8738 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-11.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-11.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-12.c b/gcc/testsuite/gcc.target/s390/hotpatch-12.c
index 6e909c0..d109478 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-12.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-12.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 999 } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-13.c b/gcc/testsuite/gcc.target/s390/hotpatch-13.c
index 959789c..9ac9f84 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-13.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-13.c
@@ -14,4 +14,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-14.c b/gcc/testsuite/gcc.target/s390/hotpatch-14.c
index 0f1f41f..209e4ac 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-14.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-14.c
@@ -14,4 +14,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-15.c b/gcc/testsuite/gcc.target/s390/hotpatch-15.c
index e306495..109034e 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-15.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-15.c
@@ -14,4 +14,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-16.c b/gcc/testsuite/gcc.target/s390/hotpatch-16.c
index 66f13fe..b01c1c3 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-16.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-16.c
@@ -14,4 +14,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-17.c b/gcc/testsuite/gcc.target/s390/hotpatch-17.c
index b301c5c..ce0b950 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-17.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-17.c
@@ -14,4 +14,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-18.c b/gcc/testsuite/gcc.target/s390/hotpatch-18.c
index 1c06783..9a24d5e 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-18.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-18.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-19.c b/gcc/testsuite/gcc.target/s390/hotpatch-19.c
index a1ec9c7..3586da2 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-19.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-19.c
@@ -20,4 +20,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-2.c b/gcc/testsuite/gcc.target/s390/hotpatch-2.c
index 0c82a5a..18be5aa 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-2.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-2.c
@@ -13,4 +13,5 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
+/* { dg-final { scan-assembler-times "\.align\t8" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-21.c b/gcc/testsuite/gcc.target/s390/hotpatch-21.c
new file mode 100644
index 0000000..d72dee3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-21.c
@@ -0,0 +1,14 @@
+/* Functional tests for the function hotpatching feature.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 -mzarch -mhotpatch=0,1" } */
+
+#include <stdio.h>
+
+void __attribute__ ((aligned(512))) hp1(void)
+{
+  printf("hello, world!\n");
+}
+
+/* Check number of occurences of certain instructions.  */
+/* { dg-final { scan-assembler-times "\.align\t512" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-22.c b/gcc/testsuite/gcc.target/s390/hotpatch-22.c
new file mode 100644
index 0000000..6507466
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-22.c
@@ -0,0 +1,14 @@
+/* Functional tests for the function hotpatching feature.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 -mzarch -mhotpatch=0,1 -falign-functions=1024" } */
+
+#include <stdio.h>
+
+void hp1(void)
+{
+  printf("hello, world!\n");
+}
+
+/* Check number of occurences of certain instructions.  */
+/* { dg-final { scan-assembler-times "\.align\t1024" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-23.c b/gcc/testsuite/gcc.target/s390/hotpatch-23.c
new file mode 100644
index 0000000..7dce5b8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-23.c
@@ -0,0 +1,14 @@
+/* Functional tests for the function hotpatching feature.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 -mzarch -mhotpatch=0,1 -falign-functions=4096" } */
+
+#include <stdio.h>
+
+void __attribute__ ((aligned(2048))) hp1(void)
+{
+  printf("hello, world!\n");
+}
+
+/* Check number of occurences of certain instructions.  */
+/* { dg-final { scan-assembler-times "\.align\t2048" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-24.c b/gcc/testsuite/gcc.target/s390/hotpatch-24.c
new file mode 100644
index 0000000..4d6815c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-24.c
@@ -0,0 +1,14 @@
+/* Functional tests for the function hotpatching feature.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 -mzarch -mhotpatch=0,1 -falign-functions=2048" } */
+
+#include <stdio.h>
+
+void __attribute__ ((aligned(4096))) hp1(void)
+{
+  printf("hello, world!\n");
+}
+
+/* Check number of occurences of certain instructions.  */
+/* { dg-final { scan-assembler-times "\.align\t4096" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-3.c b/gcc/testsuite/gcc.target/s390/hotpatch-3.c
index 041ef9c..8a2ab53 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-3.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-3.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-4.c b/gcc/testsuite/gcc.target/s390/hotpatch-4.c
index 050a065..9a69459 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-4.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-4.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-times "brcl\t\t0,0" 1 } } */
+/* { dg-final { scan-assembler-times "brcl\t0, 0" 1 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-5.c b/gcc/testsuite/gcc.target/s390/hotpatch-5.c
index 785dba7..b3d990f 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-5.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-5.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-times "brcl\t\t0,0" 1 } } */
+/* { dg-final { scan-assembler-times "brcl\t0, 0" 1 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-6.c b/gcc/testsuite/gcc.target/s390/hotpatch-6.c
index da96e43..10ba558 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-6.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-6.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-times "brcl\t\t0,0" 1 } } */
+/* { dg-final { scan-assembler-times "brcl\t0, 0" 1 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-7.c b/gcc/testsuite/gcc.target/s390/hotpatch-7.c
index b7c19d7..edf67d3 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-7.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-7.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-not "nop\t0" } } */
-/* { dg-final { scan-assembler-times "brcl\t\t0,0" 2 } } */
+/* { dg-final { scan-assembler-times "brcl\t0, 0" 2 } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-8.c b/gcc/testsuite/gcc.target/s390/hotpatch-8.c
index 25edd9b..a179c4a 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-8.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-8.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-times "nopr\t%r7" 1 } } */
 /* { dg-final { scan-assembler-times "nop\t0" 1 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
diff --git a/gcc/testsuite/gcc.target/s390/hotpatch-9.c b/gcc/testsuite/gcc.target/s390/hotpatch-9.c
index 2143f9d..c4e21f2 100644
--- a/gcc/testsuite/gcc.target/s390/hotpatch-9.c
+++ b/gcc/testsuite/gcc.target/s390/hotpatch-9.c
@@ -13,4 +13,4 @@ void hp1(void)
 /* Check number of occurences of certain instructions.  */
 /* { dg-final { scan-assembler-not "nopr\t%r7" } } */
 /* { dg-final { scan-assembler-times "nop\t0" 2 } } */
-/* { dg-final { scan-assembler-not "brcl\t\t0,0" } } */
+/* { dg-final { scan-assembler-not "brcl\t0, 0" } } */
-- 
1.9.3


[-- Attachment #3: 5.0-ChangeLog --]
[-- Type: text/plain, Size: 1766 bytes --]

2015-03-02  Dominik Vogt  <vogt@linux.vnet.ibm.com>

	* config/s390/s390.c (s390_reorg): Move code to output nops after label
	to s390_reorg ().
	(s390_asm_output_function_label): Likewise.
	* config/s390/s390.c (s390_asm_output_function_label):
	Fix function label alignment with -mhtopatch.
	* config/s390/s390.md ("unspecv"): New values UNSPECV_NOP_2_BYTE,
	UNSPECV_NOP_4_BYTE and UNSPECV_NOP_6_BYTE
	("nop_2_byte"): New define_insn.
	("nop_4_byte"): Likewise.
	("nop_6_byte"): Likewise.
	* doc/extend.texi (hotpatch): hotpatch attribute doc fixes.
	* doc/invoke.texi (-mhotpatch): -mhotpatch doc fixes.

2015-03-02  Dominik Vogt  <vogt@linux.vnet.ibm.com>

	* gcc.target/s390/hotpatch-21.c: New test for hotpatch alignment.
	* gcc.target/s390/hotpatch-22.c: Likewise.
	* gcc.target/s390/hotpatch-23.c: Likewise.
	* gcc.target/s390/hotpatch-24.c: Likewise.
	* gcc.target/s390/hotpatch-2.c: Also check hotpatch alignment.
	* gcc.target/s390/hotpatch-1.c: Update expected output.
	* gcc.target/s390/hotpatch-2.c: Likewise.
	* gcc.target/s390/hotpatch-3.c: Likewise.
	* gcc.target/s390/hotpatch-4.c: Likewise.
	* gcc.target/s390/hotpatch-5.c: Likewise.
	* gcc.target/s390/hotpatch-6.c: Likewise.
	* gcc.target/s390/hotpatch-7.c: Likewise.
	* gcc.target/s390/hotpatch-8.c: Likewise.
	* gcc.target/s390/hotpatch-9.c: Likewise.
	* gcc.target/s390/hotpatch-10.c: Likewise.
	* gcc.target/s390/hotpatch-11.c: Likewise.
	* gcc.target/s390/hotpatch-12.c: Likewise.
	* gcc.target/s390/hotpatch-13.c: Likewise.
	* gcc.target/s390/hotpatch-14.c: Likewise.
	* gcc.target/s390/hotpatch-15.c: Likewise.
	* gcc.target/s390/hotpatch-16.c: Likewise.
	* gcc.target/s390/hotpatch-17.c: Likewise.
	* gcc.target/s390/hotpatch-18.c: Likewise.
	* gcc.target/s390/hotpatch-19.c: Likewise.

Re: [PATCH] S390: Hotpatching fixes.

[Jakub Jelinek]

Hi!

On Mon, Mar 09, 2015 at 01:19:38PM +0100, Dominik Vogt wrote:
> @@ -11368,6 +11349,7 @@ static void
>  s390_reorg (void)
>  {
>    bool pool_overflow = false;
> +  int hw_before, hw_after;
>  
>    /* Make sure all splits have been performed; splits after
>       machine_dependent_reorg might confuse insn length counts.  */
> @@ -11503,6 +11485,40 @@ s390_reorg (void)
>        if (insn_added_p)
>  	shorten_branches (get_insns ());
>      }
> +
> +  s390_function_num_hotpatch_hw (current_function_decl, &hw_before, &hw_after);
> +  if (hw_after > 0)

Two minor issues, both for nested functions:

1) the s390_function_num_hotpatch_hw assigns to ints whose addresses are
passed as arguments, even when it later decides to return false and in this
spot you ignore the return value.  Which means that hw_after could be
non-zero, even when you should be ignoring it.
So, either you should check above the return value too, or
change s390_function_num_hotpatch_hw so that it stores 0 for the nested
functions before returning false.

2) as s390_function_num_hotpatch_hw is now called twice for the same
function, for nested functions you'll get the warning reported twice too.
Perhaps add some additional argument whether you want the warning or not
and use it in one of the callers and not in the other one?
Plus supposedly add testsuite coverage for that.

	Jakub

Re: [PATCH] S390: Hotpatching fixes.

[Andreas Krebbel]

On 03/26/2015 09:56 PM, Jakub Jelinek wrote:
> Hi!
> 
> On Mon, Mar 09, 2015 at 01:19:38PM +0100, Dominik Vogt wrote:
>> @@ -11368,6 +11349,7 @@ static void
>>  s390_reorg (void)
>>  {
>>    bool pool_overflow = false;
>> +  int hw_before, hw_after;
>>  
>>    /* Make sure all splits have been performed; splits after
>>       machine_dependent_reorg might confuse insn length counts.  */
>> @@ -11503,6 +11485,40 @@ s390_reorg (void)
>>        if (insn_added_p)
>>  	shorten_branches (get_insns ());
>>      }
>> +
>> +  s390_function_num_hotpatch_hw (current_function_decl, &hw_before, &hw_after);
>> +  if (hw_after > 0)
> 
> Two minor issues, both for nested functions:
> 
> 1) the s390_function_num_hotpatch_hw assigns to ints whose addresses are
> passed as arguments, even when it later decides to return false and in this
> spot you ignore the return value.  Which means that hw_after could be
> non-zero, even when you should be ignoring it.
> So, either you should check above the return value too, or
> change s390_function_num_hotpatch_hw so that it stores 0 for the nested
> functions before returning false.
> 2) as s390_function_num_hotpatch_hw is now called twice for the same
> function, for nested functions you'll get the warning reported twice too.
> Perhaps add some additional argument whether you want the warning or not
> and use it in one of the callers and not in the other one?
> Plus supposedly add testsuite coverage for that.

At a second glance it is not really clear to me why we disable hotpatching for nested functions at
all. While it is probably a bit difficult to actually hotpatch them I don't see why we should
prevent it. We probably just copied that over from the x86 ms_hook_prologue attribute implementation:

static bool
ix86_function_ms_hook_prologue (const_tree fn)
{
  if (fn && lookup_attribute ("ms_hook_prologue", DECL_ATTRIBUTES (fn)))
    {
      if (decl_function_context (fn) != NULL_TREE)
	error_at (DECL_SOURCE_LOCATION (fn),
		  "ms_hook_prologue is not compatible with nested function");
      else
        return true;
    }
  return false;
}

Also the kernel guys (one of the main users of that feature) confirmed that they in principle prefer
hotpatching to behave more like -pg and -pg does insert an mcount call for nested functions.
(Although I would be surprised to hear of nested functions in the Linux kernel).

So I'm inclined to just remove that special handling of nested functions.

This would also fix 1)

Bye,

-Andreas-

Re: [PATCH] S390: Hotpatching fixes.

[Jakub Jelinek]

On Fri, Mar 27, 2015 at 10:30:38AM +0100, Andreas Krebbel wrote:
> At a second glance it is not really clear to me why we disable hotpatching for nested functions at
> all. While it is probably a bit difficult to actually hotpatch them I don't see why we should
> prevent it. We probably just copied that over from the x86 ms_hook_prologue attribute implementation:
> 
> static bool
> ix86_function_ms_hook_prologue (const_tree fn)
> {
>   if (fn && lookup_attribute ("ms_hook_prologue", DECL_ATTRIBUTES (fn)))
>     {
>       if (decl_function_context (fn) != NULL_TREE)
> 	error_at (DECL_SOURCE_LOCATION (fn),
> 		  "ms_hook_prologue is not compatible with nested function");
>       else
>         return true;
>     }
>   return false;
> }
> 
> Also the kernel guys (one of the main users of that feature) confirmed that they in principle prefer
> hotpatching to behave more like -pg and -pg does insert an mcount call for nested functions.
> (Although I would be surprised to hear of nested functions in the Linux kernel).
> 
> So I'm inclined to just remove that special handling of nested functions.

Agreed, I also wondered what would be so special about nested functions
here.
Sure, one could hotpatch them with code clobbering the static chain
register, but that wouldn't be a gcc issue.

	Jakub