Patch RFC: disable block partitioning with split stack

[Ian Lance Taylor <iant@golang.org>]

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

This recent patch
https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00518.html turned on
block partitioning even when profiling is not enabled.  This can break
code compiled with -fsplit-stack, if the cold partition calls a
function that is not compiled with -fsplit-stack (such as a C library
function).  The problem is that when the linker sees a split-stack
function call a non-split-stack function, it adjusts the function
header to request more stack space.  This doesn't work if the call is
in the cold partition, as the linker doesn't know how to find the
header to adjust.  You can see this by trying to build the Go library
using the gold linker with this patch.

This patch fixes the problem by disabling an implicit
-freorder-blocks-and-partition when -fsplit-stack is turned on.  If
the user explicitly requested -freorder-blocks-and-partition we leave
it alone, assuming that they know what they are doing.  But if it was
only turned it on implicitly because of the optimization level we turn
it off.

This is done in both the main option handling code and in the Go
frontend option handling, because the Go frontend implicitly turns on
-fsplit-stack itself.

The test case in the patch fails to build when using the gold linker
without the patch.

Bootstrapped and ran split-stack test cases on x86_64-pc-linux-gnu.

I plan to commit this tomorrow unless I hear objections.

Ian


2017-06-08  Ian Lance Taylor  <iant@golang.org>

* opts.c (finish_options): If -fsplit-stack, disable implicit
-forder-blocks-and-partition.

2017-06-08  Ian Lance Taylor  <iant@golang.org>

* go-lang.c (go_langhook_post_options): If -fsplit-stack is turned
on, disable implicit -forder-blocks-and-partition.

2017-06-08  Ian Lance Taylor  <iant@golang.org>

* gcc.dg/tree-prof/split-1.c: New test.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 2833 bytes --]

Index: opts.c
===================================================================
--- opts.c	(revision 249028)
+++ opts.c	(working copy)
@@ -863,6 +863,16 @@ finish_options (struct gcc_options *opts
       opts->x_flag_reorder_blocks = 1;
     }
 
+  /* If stack splitting is turned on, and the user did not explicitly
+     request function partitioning, turn off partitioning, as it
+     confuses the linker when trying to handle partitioned split-stack
+     code that calls a non-split-stack functions.  But if partitioning
+     was turned on explicitly just hope for the best.  */
+  if (opts->x_flag_split_stack
+      && opts->x_flag_reorder_blocks_and_partition
+      && !opts_set->x_flag_reorder_blocks_and_partition)
+    opts->x_flag_reorder_blocks_and_partition = 0;
+
   if (opts->x_flag_reorder_blocks_and_partition
       && !opts_set->x_flag_reorder_functions)
     opts->x_flag_reorder_functions = 1;
Index: go/go-lang.c
===================================================================
--- go/go-lang.c	(revision 249028)
+++ go/go-lang.c	(working copy)
@@ -304,6 +304,15 @@ go_langhook_post_options (const char **p
       && targetm_common.supports_split_stack (false, &global_options))
     global_options.x_flag_split_stack = 1;
 
+  /* If stack splitting is turned on, and the user did not explicitly
+     request function partitioning, turn off partitioning, as it
+     confuses the linker when trying to handle partitioned split-stack
+     code that calls a non-split-stack function.  */
+  if (global_options.x_flag_split_stack
+      && global_options.x_flag_reorder_blocks_and_partition
+      && !global_options_set.x_flag_reorder_blocks_and_partition)
+    global_options.x_flag_reorder_blocks_and_partition = 0;
+
   /* Returning false means that the backend should be used.  */
   return false;
 }
Index: testsuite/gcc.dg/tree-prof/split-1.c
===================================================================
--- testsuite/gcc.dg/tree-prof/split-1.c	(revision 0)
+++ testsuite/gcc.dg/tree-prof/split-1.c	(working copy)
@@ -0,0 +1,41 @@
+/* Test case that we don't get a link-time error when using
+   -fsplit-stack with -freorder-blocks-and-partition.  */
+/* { dg-require-effective-target freorder } */
+/* { dg-options "-O2 -fsplit-stack" } */
+
+extern unsigned int sleep (unsigned int);
+
+#define SIZE 10000
+
+const char *sarr[SIZE];
+const char *buf_hot;
+const char *buf_cold;
+
+__attribute__((noinline))
+void 
+foo (int path)
+{
+  int i;
+  if (path)
+    {
+      for (i = 0; i < SIZE; i++)
+	sarr[i] = buf_hot;
+    }
+  else
+    {
+      for (i = 0; i < SIZE; i++)
+	sarr[i] = buf_cold;
+      sleep (0);
+    }
+}
+
+int
+main (int argc, char *argv[])
+{
+  int i;
+  buf_hot =  "hello";
+  buf_cold = "world";
+  for (i = 0; i < 1000000; i++)
+    foo (argc);
+  return 0;
+}