On Fri, Jun 9, 2017 at 11:22 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>> On Fri, Jun 9, 2017 at 3:16 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>> >
>> >> 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.
>> >
>> > If code does not work, I wonder why we let user to overwrite the default
>> > by hand? In other cases we drop the flag with inform message.
>>
>> My thinking here is that there is no fundamental reason that the code
>> does not work, and the actual problem does not lie in GCC but rather
>> in the linker (specifically, gold). It's possible in principle to fix
>> gold to make this work, and someone who is using a fixed gold could
>> then direct GCC to take advantage of this optimization (and later
>> after that version of gold is wide-spread enough we can change GCC to
>> drop this patch).
>
> Thanks for explanation. Perhaps we could have this documented, because
> otherwise people will think the option is simply broken. I guess even better
> we could have configure autodetection for the broken linker.
Committed to mainline with docs as follows.
Ian
Index: opts.c
===================================================================
--- opts.c (revision 249070)
+++ 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: doc/invoke.texi
===================================================================
--- doc/invoke.texi (revision 249070)
+++ doc/invoke.texi (working copy)
@@ -8656,7 +8656,9 @@ paging and cache locality performance.
This optimization is automatically turned off in the presence of
exception handling, for linkonce sections, for functions with a user-defined
section attribute and on any architecture that does not support named
-sections.
+sections. When @option{-fsplit-stack} is used this option is not
+enabled by default (to avoid linker errors), but may be enabled
+explicitly (if using a working linker).
Enabled for x86 at levels @option{-O2}, @option{-O3}.
Index: go/go-lang.c
===================================================================
--- go/go-lang.c (revision 249070)
+++ 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;
+}
