Re: [PATCH] s390: Add -fsplit-stack support

2016-02-15 Thread Marcin Kościelnicki

On 15/02/16 11:21, Andreas Krebbel wrote:

On 02/14/2016 05:01 PM, Marcin Kościelnicki wrote:

libgcc/ChangeLog:

* config.host: Use t-stack and t-stack-s390 for s390*-*-linux.
* config/s390/morestack.S: New file.
* config/s390/t-stack-s390: New file.
* generic-morestack.c (__splitstack_find): Add s390-specific code.

gcc/ChangeLog:

* common/config/s390/s390-common.c (s390_supports_split_stack):
New function.
(TARGET_SUPPORTS_SPLIT_STACK): New macro.
* config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
* config/s390/s390.c (struct machine_function): New field
split_stack_varargs_pointer.
(s390_register_info): Mark r12 as clobbered if it'll be used as temp
in s390_emit_prologue.
(s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
vararg pointer.
(morestack_ref): New global.
(SPLIT_STACK_AVAILABLE): New macro.
(s390_expand_split_stack_prologue): New function.
(s390_live_on_entry): New function.
(s390_va_start): Use split-stack vararg pointer if appropriate.
(s390_asm_file_end): Emit the split-stack note sections.
(TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
* config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
(UNSPECV_SPLIT_STACK_CALL): New unspec.
(UNSPECV_SPLIT_STACK_DATA): New unspec.
(split_stack_prologue): New expand.
(split_stack_space_check): New expand.
(split_stack_data): New insn.
(split_stack_call): New expand.
(split_stack_call_*): New insn.
(split_stack_cond_call): New expand.
(split_stack_cond_call_*): New insn.


Applied. Thanks!

-Andreas-



Thanks.  And how about that testcase I submitted, does that look OK?

Marcin Kościelnicki


Re: [PATCH] s390: Add -fsplit-stack support

2016-02-15 Thread Andreas Krebbel
On 02/14/2016 05:01 PM, Marcin Kościelnicki wrote:
> libgcc/ChangeLog:
> 
>   * config.host: Use t-stack and t-stack-s390 for s390*-*-linux.
>   * config/s390/morestack.S: New file.
>   * config/s390/t-stack-s390: New file.
>   * generic-morestack.c (__splitstack_find): Add s390-specific code.
> 
> gcc/ChangeLog:
> 
>   * common/config/s390/s390-common.c (s390_supports_split_stack):
>   New function.
>   (TARGET_SUPPORTS_SPLIT_STACK): New macro.
>   * config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
>   * config/s390/s390.c (struct machine_function): New field
>   split_stack_varargs_pointer.
>   (s390_register_info): Mark r12 as clobbered if it'll be used as temp
>   in s390_emit_prologue.
>   (s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
>   vararg pointer.
>   (morestack_ref): New global.
>   (SPLIT_STACK_AVAILABLE): New macro.
>   (s390_expand_split_stack_prologue): New function.
>   (s390_live_on_entry): New function.
>   (s390_va_start): Use split-stack vararg pointer if appropriate.
>   (s390_asm_file_end): Emit the split-stack note sections.
>   (TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
>   * config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
>   (UNSPECV_SPLIT_STACK_CALL): New unspec.
>   (UNSPECV_SPLIT_STACK_DATA): New unspec.
>   (split_stack_prologue): New expand.
>   (split_stack_space_check): New expand.
>   (split_stack_data): New insn.
>   (split_stack_call): New expand.
>   (split_stack_call_*): New insn.
>   (split_stack_cond_call): New expand.
>   (split_stack_cond_call_*): New insn.

Applied. Thanks!

-Andreas-



[PATCH] s390: Add -fsplit-stack support

2016-02-14 Thread Marcin Kościelnicki
libgcc/ChangeLog:

* config.host: Use t-stack and t-stack-s390 for s390*-*-linux.
* config/s390/morestack.S: New file.
* config/s390/t-stack-s390: New file.
* generic-morestack.c (__splitstack_find): Add s390-specific code.

gcc/ChangeLog:

* common/config/s390/s390-common.c (s390_supports_split_stack):
New function.
(TARGET_SUPPORTS_SPLIT_STACK): New macro.
* config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
* config/s390/s390.c (struct machine_function): New field
split_stack_varargs_pointer.
(s390_register_info): Mark r12 as clobbered if it'll be used as temp
in s390_emit_prologue.
(s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
vararg pointer.
(morestack_ref): New global.
(SPLIT_STACK_AVAILABLE): New macro.
(s390_expand_split_stack_prologue): New function.
(s390_live_on_entry): New function.
(s390_va_start): Use split-stack vararg pointer if appropriate.
(s390_asm_file_end): Emit the split-stack note sections.
(TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
* config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
(UNSPECV_SPLIT_STACK_CALL): New unspec.
(UNSPECV_SPLIT_STACK_DATA): New unspec.
(split_stack_prologue): New expand.
(split_stack_space_check): New expand.
(split_stack_data): New insn.
(split_stack_call): New expand.
(split_stack_call_*): New insn.
(split_stack_cond_call): New expand.
(split_stack_cond_call_*): New insn.
---
Whoops, I noticed a problem introduced when removing ESA bits: in the
__morestack exception-handling path in 31-bit version, I neglected
to stuff GOT address in %r12, which is necessary for the PLT stub to
work.  The only change in this version is the added larl %r12,
_GLOBAL_OFFSET_TABLE_ line.

 gcc/ChangeLog|  30 ++
 gcc/common/config/s390/s390-common.c |  14 +
 gcc/config/s390/s390-protos.h|   1 +
 gcc/config/s390/s390.c   | 214 +++-
 gcc/config/s390/s390.md  | 138 
 libgcc/ChangeLog |   7 +
 libgcc/config.host   |   4 +-
 libgcc/config/s390/morestack.S   | 611 +++
 libgcc/config/s390/t-stack-s390  |   2 +
 libgcc/generic-morestack.c   |   4 +
 10 files changed, 1018 insertions(+), 7 deletions(-)
 create mode 100644 libgcc/config/s390/morestack.S
 create mode 100644 libgcc/config/s390/t-stack-s390

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index e81d1fe..60a4608 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,33 @@
+2016-02-14  Marcin Kościelnicki  
+
+   * common/config/s390/s390-common.c (s390_supports_split_stack):
+   New function.
+   (TARGET_SUPPORTS_SPLIT_STACK): New macro.
+   * config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
+   * config/s390/s390.c (struct machine_function): New field
+   split_stack_varargs_pointer.
+   (s390_register_info): Mark r12 as clobbered if it'll be used as temp
+   in s390_emit_prologue.
+   (s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
+   vararg pointer.
+   (morestack_ref): New global.
+   (SPLIT_STACK_AVAILABLE): New macro.
+   (s390_expand_split_stack_prologue): New function.
+   (s390_live_on_entry): New function.
+   (s390_va_start): Use split-stack vararg pointer if appropriate.
+   (s390_asm_file_end): Emit the split-stack note sections.
+   (TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
+   * config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
+   (UNSPECV_SPLIT_STACK_CALL): New unspec.
+   (UNSPECV_SPLIT_STACK_DATA): New unspec.
+   (split_stack_prologue): New expand.
+   (split_stack_space_check): New expand.
+   (split_stack_data): New insn.
+   (split_stack_call): New expand.
+   (split_stack_call_*): New insn.
+   (split_stack_cond_call): New expand.
+   (split_stack_cond_call_*): New insn.
+
 2016-02-14  Venkataramanan Kumar  
 
*  config/i386/znver1.md
diff --git a/gcc/common/config/s390/s390-common.c 
b/gcc/common/config/s390/s390-common.c
index 4519c21..1e497e6 100644
--- a/gcc/common/config/s390/s390-common.c
+++ b/gcc/common/config/s390/s390-common.c
@@ -105,6 +105,17 @@ s390_handle_option (struct gcc_options *opts 
ATTRIBUTE_UNUSED,
 }
 }
 
+/* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
+   We don't verify it, since earlier versions just have padding at
+   its place, which works just as well.  */
+
+static bool
+s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
+  struct gcc_options *opts ATTRIBUTE_UNUSED)
+{
+  return true;
+}
+
 #undef TARGET_DEFAULT_TARGET_FLAGS
 #define TARGET_DEFAULT_TARGET_FLAGS (TARGET_DEFAULT)
 
@@ -117,4 +128,7 @@ s390_handle_option (struct gcc_

Re: [PATCH] s390: Add -fsplit-stack support

2016-02-10 Thread Marcin Kościelnicki

On 04/02/16 13:44, Marcin Kościelnicki wrote:

On 03/02/16 18:27, Ulrich Weigand wrote:

Marcin Kościelnicki wrote:


libgcc/ChangeLog:

* config.host: Use t-stack and t-stack-s390 for s390*-*-linux.
* config/s390/morestack.S: New file.
* config/s390/t-stack-s390: New file.
* generic-morestack.c (__splitstack_find): Add s390-specific code.

gcc/ChangeLog:

* common/config/s390/s390-common.c (s390_supports_split_stack):
New function.
(TARGET_SUPPORTS_SPLIT_STACK): New macro.
* config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
* config/s390/s390.c (struct machine_function): New field
split_stack_varargs_pointer.
(s390_register_info): Mark r12 as clobbered if it'll be used as temp
in s390_emit_prologue.
(s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
vararg pointer.
(morestack_ref): New global.
(SPLIT_STACK_AVAILABLE): New macro.
(s390_expand_split_stack_prologue): New function.
(s390_live_on_entry): New function.
(s390_va_start): Use split-stack vararg pointer if appropriate.
(s390_asm_file_end): Emit the split-stack note sections.
(TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
* config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
(UNSPECV_SPLIT_STACK_CALL): New unspec.
(UNSPECV_SPLIT_STACK_DATA): New unspec.
(split_stack_prologue): New expand.
(split_stack_space_check): New expand.
(split_stack_data): New insn.
(split_stack_call): New expand.
(split_stack_call_*): New insn.
(split_stack_cond_call): New expand.
(split_stack_cond_call_*): New insn.
---
Changes applied.  Testsuite still running, still works on my simple
tests.

As for common code prerequisites: #3 is no longer needed, and very
likely
so is #4 (it fixes problems that I've only seen with ESA mode, and
testsuite
runs just fine without it now).


OK, I see.  The patch is OK for mainline then, assuming testing passes.


Well, testing passes (as in, is no worse than x86 - the testsuite
doesn't really agree with -fsplit-stack in a few places involving
backtraces).  However, there's still the libgo issue to be taken care
of.  For my tests, I patched it up with:
[...]


I see the libgo patch has landed today.  Can we get this pushed?

Marcin Kościelnicki




Re: [PATCH] s390: Add -fsplit-stack support

2016-02-05 Thread Ulrich Weigand
Marcin Kościelnicki wrote:

> I'll stay with checking for larl - while I can imagine someone adding a 
> new conditional branch instruction, I don't see a need for another 
> larl-like instruction.  Besides, this way the failure mode for an 
> unknown instruction would be producing an error, instead of silently 
> emitting code with unfixed prologue.

OK, fine with me.  B.t.w. Andreas has checked in the sibcall fix,
so you no longer should be seeing larl used for sibcalls.

> I've updated and resubmitted the gold patch.

Thanks!

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  ulrich.weig...@de.ibm.com



Re: [PATCH] s390: Add -fsplit-stack support

2016-02-05 Thread Marcin Kościelnicki

On 04/02/16 17:27, Ulrich Weigand wrote:

Marcin Kościelnicki wrote:


Fair enough.  Here's what I'm going to implement in gold:

- any PLT relocation: call
- PC32DBL on a larl: non-call
- PC32DBL otherwise: call
- any other relocation: non-call

Does that sound right?


Hmm, I'm wondering about the PC32DBL choices.  There are now
a large number of other non-call instructions that use PC32DBL,
including lrl, strl, crl, cgrl, cgfrl, ...

However, these all access *data* at the pointed-to location,
so it is quite unlikely they would ever be used with a
function symbol.  So, assuming that you also check that the
target of the relocation is a function symbol, treating only
larl as non-call might be OK.


Yeah, I make sure the symbol is a STT_FUNC.


Maybe a more conservative approach might be to  make the decision
the other way round: for PC32DBL check for *branch* instructions,
and treat only those are calls.  There's just a few branch
instruction using PC32DBL:

brasl  (call)
brcl   (conditional or unconditional sibcall)
brcth  (???)

where the last one is extremely unlikely (but theorically
possible) to be used as conditional sibcall combined with
a register decrement; I don't think this can ever happen
with current compilers however.


I'll stay with checking for larl - while I can imagine someone adding a 
new conditional branch instruction, I don't see a need for another 
larl-like instruction.  Besides, this way the failure mode for an 
unknown instruction would be producing an error, instead of silently 
emitting code with unfixed prologue.


For full completeness, there are also PC16DBL relocations that
*could* target called functions, but only when compiling with
the -msmall-exec flag to assume total executable size is less
than 64 KB.  These are used by the following instructions:

bras
brc
brct
brctg
brxh
brxhg
brxle
brxlg
crj
cgrj
clrj
clgrj
cij
cgij
clij
clgij

Note that those are *all* branch instructions, so it might
make sense to add any PC16DBL targetting a function symbol
to the list of calls, just in case.  (But since basically
nobody ever uses -msmall-exec, it doesn't really matter
much either.)


Ah right, I've added PC16DBL to the "always call" list.


Bye,
Ulrich



I've updated and resubmitted the gold patch.

Marcin Kościelnicki


Re: [PATCH] s390: Add -fsplit-stack support

2016-02-04 Thread Ulrich Weigand
Marcin Kościelnicki wrote:

> Fair enough.  Here's what I'm going to implement in gold:
> 
> - any PLT relocation: call
> - PC32DBL on a larl: non-call
> - PC32DBL otherwise: call
> - any other relocation: non-call
> 
> Does that sound right?

Hmm, I'm wondering about the PC32DBL choices.  There are now
a large number of other non-call instructions that use PC32DBL,
including lrl, strl, crl, cgrl, cgfrl, ...

However, these all access *data* at the pointed-to location,
so it is quite unlikely they would ever be used with a
function symbol.  So, assuming that you also check that the
target of the relocation is a function symbol, treating only
larl as non-call might be OK.

Maybe a more conservative approach might be to  make the decision
the other way round: for PC32DBL check for *branch* instructions,
and treat only those are calls.  There's just a few branch
instruction using PC32DBL:

brasl  (call)
brcl   (conditional or unconditional sibcall)
brcth  (???)

where the last one is extremely unlikely (but theorically
possible) to be used as conditional sibcall combined with
a register decrement; I don't think this can ever happen
with current compilers however.

For full completeness, there are also PC16DBL relocations that
*could* target called functions, but only when compiling with
the -msmall-exec flag to assume total executable size is less
than 64 KB.  These are used by the following instructions:

bras
brc
brct
brctg
brxh
brxhg
brxle
brxlg
crj
cgrj
clrj
clgrj
cij
cgij
clij
clgij

Note that those are *all* branch instructions, so it might
make sense to add any PC16DBL targetting a function symbol
to the list of calls, just in case.  (But since basically
nobody ever uses -msmall-exec, it doesn't really matter
much either.)

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  ulrich.weig...@de.ibm.com



Re: [PATCH] s390: Add -fsplit-stack support

2016-02-04 Thread Marcin Kościelnicki

On 04/02/16 16:06, Ulrich Weigand wrote:

Marcin Kościelnicki wrote:


Ugh. I take that back.  For -fPIC, the load-address sequence is:

  larl%r1,f@GOTENT
  lg  %r2,0(%r1)
  br  %r14


This is correct.


And (sibling) call sequence is:

  larl%r1,f@GOTENT
  lg  %r1,0(%r1)
  br  %r1


Oops.  That is actually a GCC bug.  The sibcall sequence really must be:

jg f@PLT

This is a real bug since it forces non-lazy symbol resolution for f
just because the compiler chose those use a sibcall optimization;
that's not supposed to happen.

It seems this bug was accidentally introduced here:

2010-04-20  Andreas Krebbel  

 PR target/43635
 * config/s390/s390.c (s390_emit_call): Turn direct into indirect
 calls for -fpic -m31 if they have been sibcall optimized.

since the patch doesn't check for TARGET_64BIT ...

Andreas, can you have a look?


It seems there's no proper way to recognize a call vs a load address -
so we can either go with emitting the marker, or have the same problem
as on ppc.

So - how much should we care?


I think we should fix that bug.  That won't help for existing objects,
but those don't use split stack either, so that shouldn't matter.

If we fix that bug before (or at the same time as) adding split-stack
support, the linker will still be able to distigunish function pointer
loads from calls (including sibcalls) on all objects using split stack.

Bye,
Ulrich




Fair enough.  Here's what I'm going to implement in gold:

- any PLT relocation: call
- PC32DBL on a larl: non-call
- PC32DBL otherwise: call
- any other relocation: non-call

Does that sound right?

Marcin Kościelnicki


Re: [PATCH] s390: Add -fsplit-stack support

2016-02-04 Thread Ulrich Weigand
Marcin Kościelnicki wrote:

> Ugh. I take that back.  For -fPIC, the load-address sequence is:
> 
>  larl%r1,f@GOTENT
>  lg  %r2,0(%r1)
>  br  %r14

This is correct.

> And (sibling) call sequence is:
> 
>  larl%r1,f@GOTENT
>  lg  %r1,0(%r1)
>  br  %r1

Oops.  That is actually a GCC bug.  The sibcall sequence really must be:

jg f@PLT

This is a real bug since it forces non-lazy symbol resolution for f
just because the compiler chose those use a sibcall optimization;
that's not supposed to happen.

It seems this bug was accidentally introduced here:

2010-04-20  Andreas Krebbel  

PR target/43635
* config/s390/s390.c (s390_emit_call): Turn direct into indirect
calls for -fpic -m31 if they have been sibcall optimized.

since the patch doesn't check for TARGET_64BIT ...

Andreas, can you have a look?

> It seems there's no proper way to recognize a call vs a load address - 
> so we can either go with emitting the marker, or have the same problem 
> as on ppc.
> 
> So - how much should we care?

I think we should fix that bug.  That won't help for existing objects,
but those don't use split stack either, so that shouldn't matter.

If we fix that bug before (or at the same time as) adding split-stack
support, the linker will still be able to distigunish function pointer
loads from calls (including sibcalls) on all objects using split stack.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  ulrich.weig...@de.ibm.com



Re: [PATCH] s390: Add -fsplit-stack support

2016-02-04 Thread Marcin Kościelnicki

On 03/02/16 18:27, Ulrich Weigand wrote:

Marcin Kościelnicki wrote:


libgcc/ChangeLog:

* config.host: Use t-stack and t-stack-s390 for s390*-*-linux.
* config/s390/morestack.S: New file.
* config/s390/t-stack-s390: New file.
* generic-morestack.c (__splitstack_find): Add s390-specific code.

gcc/ChangeLog:

* common/config/s390/s390-common.c (s390_supports_split_stack):
New function.
(TARGET_SUPPORTS_SPLIT_STACK): New macro.
* config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
* config/s390/s390.c (struct machine_function): New field
split_stack_varargs_pointer.
(s390_register_info): Mark r12 as clobbered if it'll be used as temp
in s390_emit_prologue.
(s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
vararg pointer.
(morestack_ref): New global.
(SPLIT_STACK_AVAILABLE): New macro.
(s390_expand_split_stack_prologue): New function.
(s390_live_on_entry): New function.
(s390_va_start): Use split-stack vararg pointer if appropriate.
(s390_asm_file_end): Emit the split-stack note sections.
(TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
* config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
(UNSPECV_SPLIT_STACK_CALL): New unspec.
(UNSPECV_SPLIT_STACK_DATA): New unspec.
(split_stack_prologue): New expand.
(split_stack_space_check): New expand.
(split_stack_data): New insn.
(split_stack_call): New expand.
(split_stack_call_*): New insn.
(split_stack_cond_call): New expand.
(split_stack_cond_call_*): New insn.
---
Changes applied.  Testsuite still running, still works on my simple tests.

As for common code prerequisites: #3 is no longer needed, and very likely
so is #4 (it fixes problems that I've only seen with ESA mode, and testsuite
runs just fine without it now).


OK, I see.  The patch is OK for mainline then, assuming testing passes.


Well, testing passes (as in, is no worse than x86 - the testsuite 
doesn't really agree with -fsplit-stack in a few places involving 
backtraces).  However, there's still the libgo issue to be taken care 
of.  For my tests, I patched it up with:


diff --git a/libgo/runtime/proc.c b/libgo/runtime/proc.c
index c25a217..efa6806 100644
--- a/libgo/runtime/proc.c
+++ b/libgo/runtime/proc.c
@@ -2016,17 +2016,19 @@ doentersyscall()
m->locks++;

// Leave SP around for GC and traceback.
+   {
 #ifdef USING_SPLIT_STACK
-   g->gcstack = __splitstack_find(nil, nil, &g->gcstack_size,
-  &g->gcnext_segment, &g->gcnext_sp,
-  &g->gcinitial_sp);
+   size_t size_tmp;
+   g->gcstack = __splitstack_find(nil, nil, &size_tmp,
+  &g->gcnext_segment, 
&g->gcnext_sp,
+  &g->gcinitial_sp);
+   g->gcstack_size = size_tmp;
 #else
-   {
void *v;

g->gcnext_sp = (byte *) &v;
-   }
 #endif
+   }

g->status = Gsyscall;

@@ -2064,9 +2066,13 @@ runtime_entersyscallblock(void)

// Leave SP around for GC and traceback.
 #ifdef USING_SPLIT_STACK
-   g->gcstack = __splitstack_find(nil, nil, &g->gcstack_size,
-  &g->gcnext_segment, &g->gcnext_sp,
-  &g->gcinitial_sp);
+   {
+   size_t size_tmp;
+   g->gcstack = __splitstack_find(nil, nil, &size_tmp,
+  &g->gcnext_segment, 
&g->gcnext_sp,
+  &g->gcinitial_sp);
+   g->gcstack_size = size_tmp;
+   }
 #else
g->gcnext_sp = (byte *) &p;
 #endif

Andreas, did you have any luck with fixing this?  If not, I'll try 
submitting the above patch to gofrontend.




Thanks again,
Ulrich





Re: [PATCH] s390: Add -fsplit-stack support

2016-02-03 Thread Marcin Kościelnicki




The second issue I'm still not sure about is the magic nop marker
for frameless functions.  In an earlier mail you wrote:


Both currently supported
architectures always emit split-stack code on every function.


At least for rs6000 this doesn't appear to be true; in
rs6000_expand_split_stack_prologue we have:

   if (!info->push_p)
 return;

so it does nothing for frameless routines.

Now on i386 we do indeed generate code for frameless routines;
in fact, the *same* full stack check is generated as for any
other routine.  Now I'm wondering: is there are reason why
this check would be necessary (and there's simply a bug in
the rs6000 implementation)?  Then we obviously should do the
same on s390.


Try that on powerpc64(le):

$ cat a.c
#include 

void f(void) {
}

typedef void (*fptr)(void);

fptr g(void);

int main() {
 printf("%p\n", g());
}

$ cat b.c
void f(void);

typedef void (*fptr)(void);

fptr g(void) {
 return f;
}

$ gcc -O3 -fsplit-stack -c b.c
$ gcc -O3 -c a.c
$ gcc a.o b.o -fuse-ld=gold

I don't have a recent enough gcc for powerpc, but from what I've seen in
the code, this should explode with a linker error.

Of course, mixing split-stack and non-split-stack code when function
pointers are involved is sketchy anyway, so what's one more bug...

That said, for s390, we can avoid the above problem by checking the
relocation in gold now that ESA paths are gone - for direct function
calls (the only ones we care about), we should be seeing a relocation in
brasl.  So I'll remove the nopmark thing and add proper recognition in
gold.


Ugh. I take that back.  For -fPIC, the load-address sequence is:

larl%r1,f@GOTENT
lg  %r2,0(%r1)
br  %r14

And (sibling) call sequence is:

larl%r1,f@GOTENT
lg  %r1,0(%r1)
br  %r1

It seems there's no proper way to recognize a call vs a load address - 
so we can either go with emitting the marker, or have the same problem 
as on ppc.


So - how much should we care?





On the other hand, if rs6000 works fine *without* any code
in frameless routines, why wouldn't that work for s390 too?

Emitting a nop (that is always executed) still looks weird to me.


Bye,
Ulrich







Re: [PATCH] s390: Add -fsplit-stack support

2016-02-03 Thread Ulrich Weigand
Marcin Kościelnicki wrote:

> libgcc/ChangeLog:
> 
>   * config.host: Use t-stack and t-stack-s390 for s390*-*-linux.
>   * config/s390/morestack.S: New file.
>   * config/s390/t-stack-s390: New file.
>   * generic-morestack.c (__splitstack_find): Add s390-specific code.
> 
> gcc/ChangeLog:
> 
>   * common/config/s390/s390-common.c (s390_supports_split_stack):
>   New function.
>   (TARGET_SUPPORTS_SPLIT_STACK): New macro.
>   * config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
>   * config/s390/s390.c (struct machine_function): New field
>   split_stack_varargs_pointer.
>   (s390_register_info): Mark r12 as clobbered if it'll be used as temp
>   in s390_emit_prologue.
>   (s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
>   vararg pointer.
>   (morestack_ref): New global.
>   (SPLIT_STACK_AVAILABLE): New macro.
>   (s390_expand_split_stack_prologue): New function.
>   (s390_live_on_entry): New function.
>   (s390_va_start): Use split-stack vararg pointer if appropriate.
>   (s390_asm_file_end): Emit the split-stack note sections.
>   (TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
>   * config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
>   (UNSPECV_SPLIT_STACK_CALL): New unspec.
>   (UNSPECV_SPLIT_STACK_DATA): New unspec.
>   (split_stack_prologue): New expand.
>   (split_stack_space_check): New expand.
>   (split_stack_data): New insn.
>   (split_stack_call): New expand.
>   (split_stack_call_*): New insn.
>   (split_stack_cond_call): New expand.
>   (split_stack_cond_call_*): New insn.
> ---
> Changes applied.  Testsuite still running, still works on my simple tests.
> 
> As for common code prerequisites: #3 is no longer needed, and very likely
> so is #4 (it fixes problems that I've only seen with ESA mode, and testsuite
> runs just fine without it now).

OK, I see.  The patch is OK for mainline then, assuming testing passes.

Thanks again,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  ulrich.weig...@de.ibm.com



[PATCH] s390: Add -fsplit-stack support

2016-02-03 Thread Marcin Kościelnicki
libgcc/ChangeLog:

* config.host: Use t-stack and t-stack-s390 for s390*-*-linux.
* config/s390/morestack.S: New file.
* config/s390/t-stack-s390: New file.
* generic-morestack.c (__splitstack_find): Add s390-specific code.

gcc/ChangeLog:

* common/config/s390/s390-common.c (s390_supports_split_stack):
New function.
(TARGET_SUPPORTS_SPLIT_STACK): New macro.
* config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
* config/s390/s390.c (struct machine_function): New field
split_stack_varargs_pointer.
(s390_register_info): Mark r12 as clobbered if it'll be used as temp
in s390_emit_prologue.
(s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
vararg pointer.
(morestack_ref): New global.
(SPLIT_STACK_AVAILABLE): New macro.
(s390_expand_split_stack_prologue): New function.
(s390_live_on_entry): New function.
(s390_va_start): Use split-stack vararg pointer if appropriate.
(s390_asm_file_end): Emit the split-stack note sections.
(TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
* config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
(UNSPECV_SPLIT_STACK_CALL): New unspec.
(UNSPECV_SPLIT_STACK_DATA): New unspec.
(split_stack_prologue): New expand.
(split_stack_space_check): New expand.
(split_stack_data): New insn.
(split_stack_call): New expand.
(split_stack_call_*): New insn.
(split_stack_cond_call): New expand.
(split_stack_cond_call_*): New insn.
---
Changes applied.  Testsuite still running, still works on my simple tests.

As for common code prerequisites: #3 is no longer needed, and very likely
so is #4 (it fixes problems that I've only seen with ESA mode, and testsuite
runs just fine without it now).

 gcc/ChangeLog|  30 ++
 gcc/common/config/s390/s390-common.c |  14 +
 gcc/config/s390/s390-protos.h|   1 +
 gcc/config/s390/s390.c   | 214 +++-
 gcc/config/s390/s390.md  | 138 
 libgcc/ChangeLog |   7 +
 libgcc/config.host   |   4 +-
 libgcc/config/s390/morestack.S   | 609 +++
 libgcc/config/s390/t-stack-s390  |   2 +
 libgcc/generic-morestack.c   |   4 +
 10 files changed, 1016 insertions(+), 7 deletions(-)
 create mode 100644 libgcc/config/s390/morestack.S
 create mode 100644 libgcc/config/s390/t-stack-s390

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 92db764..8e3f9f7 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,33 @@
+2016-02-03  Marcin Kościelnicki  
+
+   * common/config/s390/s390-common.c (s390_supports_split_stack):
+   New function.
+   (TARGET_SUPPORTS_SPLIT_STACK): New macro.
+   * config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
+   * config/s390/s390.c (struct machine_function): New field
+   split_stack_varargs_pointer.
+   (s390_register_info): Mark r12 as clobbered if it'll be used as temp
+   in s390_emit_prologue.
+   (s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
+   vararg pointer.
+   (morestack_ref): New global.
+   (SPLIT_STACK_AVAILABLE): New macro.
+   (s390_expand_split_stack_prologue): New function.
+   (s390_live_on_entry): New function.
+   (s390_va_start): Use split-stack vararg pointer if appropriate.
+   (s390_asm_file_end): Emit the split-stack note sections.
+   (TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
+   * config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
+   (UNSPECV_SPLIT_STACK_CALL): New unspec.
+   (UNSPECV_SPLIT_STACK_DATA): New unspec.
+   (split_stack_prologue): New expand.
+   (split_stack_space_check): New expand.
+   (split_stack_data): New insn.
+   (split_stack_call): New expand.
+   (split_stack_call_*): New insn.
+   (split_stack_cond_call): New expand.
+   (split_stack_cond_call_*): New insn.
+
 2016-02-03  Kirill Yukhin  
 
PR target/69118
diff --git a/gcc/common/config/s390/s390-common.c 
b/gcc/common/config/s390/s390-common.c
index 4519c21..1e497e6 100644
--- a/gcc/common/config/s390/s390-common.c
+++ b/gcc/common/config/s390/s390-common.c
@@ -105,6 +105,17 @@ s390_handle_option (struct gcc_options *opts 
ATTRIBUTE_UNUSED,
 }
 }
 
+/* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
+   We don't verify it, since earlier versions just have padding at
+   its place, which works just as well.  */
+
+static bool
+s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
+  struct gcc_options *opts ATTRIBUTE_UNUSED)
+{
+  return true;
+}
+
 #undef TARGET_DEFAULT_TARGET_FLAGS
 #define TARGET_DEFAULT_TARGET_FLAGS (TARGET_DEFAULT)
 
@@ -117,4 +128,7 @@ s390_handle_option (struct gcc_options *opts 
ATTRIBUTE_UNUSED,
 #undef TARGET_OPTION

Re: [PATCH] s390: Add -fsplit-stack support

2016-02-03 Thread Ulrich Weigand
Marcin Kościelnicki wrote:

> Comment fixed, split_stack_marker gone, reorg gone.  Generated code seems 
> sane,
> but testsuite still running.
> 
> I will need to modify the gold patch to handle the "leaf function taking 
> non-split
> stack function address" issue - this will likely require messing with the 
> target
> independent plumbing, the hook for that doesn't seem to get enough params.

Thanks for making those changes; the patch is looking a lot nicer (and shorter 
:-))
now!  Just to clarify, your original patch series had two common-code 
prerequisite
patches (3/5 and 4/5) -- it looks like those may still be needed?  If so, we'll
have to get approval from the appropriate middle-end maintainers before this
patch can go it as well.

As to the back-end patch, I've now only got some cosmetical issues:

> +  insn = emit_insn (gen_main_base_64 (r1, parm_base));

Now that we aren't using the literal pool infrastructure for the block any more,
I guess we shouldn't be using it to load the address either.  Just something
like:
  insn = emit_move_insn (r1, gen_rtx_LABEL_REF (VOIDmode, parm_base));
should do it.

> +(define_insn "split_stack_data"
> +  [(unspec_volatile [(match_operand 0 "" "X")
> +  (match_operand 1 "" "X")
> +  (match_operand 2 "consttable_operand" "X")
> +  (match_operand 3 "consttable_operand" "X")]

And similarly here, just use const_int_operand.

Otherwise, this all looks very good to me.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  ulrich.weig...@de.ibm.com



[PATCH] s390: Add -fsplit-stack support

2016-02-02 Thread Marcin Kościelnicki
libgcc/ChangeLog:

* config.host: Use t-stack and t-stack-s390 for s390*-*-linux.
* config/s390/morestack.S: New file.
* config/s390/t-stack-s390: New file.
* generic-morestack.c (__splitstack_find): Add s390-specific code.

gcc/ChangeLog:

* common/config/s390/s390-common.c (s390_supports_split_stack):
New function.
(TARGET_SUPPORTS_SPLIT_STACK): New macro.
* config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
* config/s390/s390.c (struct machine_function): New field
split_stack_varargs_pointer.
(s390_register_info): Mark r12 as clobbered if it'll be used as temp
in s390_emit_prologue.
(s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
vararg pointer.
(morestack_ref): New global.
(SPLIT_STACK_AVAILABLE): New macro.
(s390_expand_split_stack_prologue): New function.
(s390_live_on_entry): New function.
(s390_va_start): Use split-stack vararg pointer if appropriate.
(s390_asm_file_end): Emit the split-stack note sections.
(TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
* config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
(UNSPECV_SPLIT_STACK_CALL): New unspec.
(UNSPECV_SPLIT_STACK_DATA): New unspec.
(split_stack_prologue): New expand.
(split_stack_space_check): New expand.
(split_stack_data): New insn.
(split_stack_call): New expand.
(split_stack_call_*): New insn.
(split_stack_cond_call): New expand.
(split_stack_cond_call_*): New insn.
---
Comment fixed, split_stack_marker gone, reorg gone.  Generated code seems sane,
but testsuite still running.

I will need to modify the gold patch to handle the "leaf function taking 
non-split
stack function address" issue - this will likely require messing with the target
independent plumbing, the hook for that doesn't seem to get enough params.

 gcc/ChangeLog|  30 ++
 gcc/common/config/s390/s390-common.c |  14 +
 gcc/config/s390/s390-protos.h|   1 +
 gcc/config/s390/s390.c   | 214 +++-
 gcc/config/s390/s390.md  | 138 
 libgcc/ChangeLog |   7 +
 libgcc/config.host   |   4 +-
 libgcc/config/s390/morestack.S   | 609 +++
 libgcc/config/s390/t-stack-s390  |   2 +
 libgcc/generic-morestack.c   |   4 +
 10 files changed, 1016 insertions(+), 7 deletions(-)
 create mode 100644 libgcc/config/s390/morestack.S
 create mode 100644 libgcc/config/s390/t-stack-s390

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 9a2cec8..568dff4 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,33 @@
+2016-02-02  Marcin Kościelnicki  
+
+   * common/config/s390/s390-common.c (s390_supports_split_stack):
+   New function.
+   (TARGET_SUPPORTS_SPLIT_STACK): New macro.
+   * config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
+   * config/s390/s390.c (struct machine_function): New field
+   split_stack_varargs_pointer.
+   (s390_register_info): Mark r12 as clobbered if it'll be used as temp
+   in s390_emit_prologue.
+   (s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
+   vararg pointer.
+   (morestack_ref): New global.
+   (SPLIT_STACK_AVAILABLE): New macro.
+   (s390_expand_split_stack_prologue): New function.
+   (s390_live_on_entry): New function.
+   (s390_va_start): Use split-stack vararg pointer if appropriate.
+   (s390_asm_file_end): Emit the split-stack note sections.
+   (TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
+   * config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
+   (UNSPECV_SPLIT_STACK_CALL): New unspec.
+   (UNSPECV_SPLIT_STACK_DATA): New unspec.
+   (split_stack_prologue): New expand.
+   (split_stack_space_check): New expand.
+   (split_stack_data): New insn.
+   (split_stack_call): New expand.
+   (split_stack_call_*): New insn.
+   (split_stack_cond_call): New expand.
+   (split_stack_cond_call_*): New insn.
+
 2016-02-02  Thomas Schwinge  
 
* omp-builtins.def (BUILT_IN_GOACC_HOST_DATA): Remove.
diff --git a/gcc/common/config/s390/s390-common.c 
b/gcc/common/config/s390/s390-common.c
index 4519c21..1e497e6 100644
--- a/gcc/common/config/s390/s390-common.c
+++ b/gcc/common/config/s390/s390-common.c
@@ -105,6 +105,17 @@ s390_handle_option (struct gcc_options *opts 
ATTRIBUTE_UNUSED,
 }
 }
 
+/* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
+   We don't verify it, since earlier versions just have padding at
+   its place, which works just as well.  */
+
+static bool
+s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
+  struct gcc_options *opts ATTRIBUTE_UNUSED)
+{
+  return true;
+}
+
 #undef TARGET_DEFAULT_TARGET_FLAGS
 #define TARGET_DEFAULT_TARGE

Re: [PATCH] s390: Add -fsplit-stack support

2016-02-02 Thread Marcin Kościelnicki

On 02/02/16 19:33, Ulrich Weigand wrote:

Marcin Kościelnicki wrote:


Here we go.  I've also removed the "see below", since I don't really
see anything below...


The "see below" refers to this code (which I agree isn't really obvious):

   if (TARGET_TPF_PROFILING)
 {
   /* Generate a BAS instruction to serve as a function
  entry intercept to facilitate the use of tracing
  algorithms located at the branch target.  */
   emit_insn (gen_prologue_tpf ());

What is not explicitly called out here is that this tracing function
actually refers to some hard registers, in particular r14, and assumes
they still have the original contents as at function entry.

That is why the prolog code avoid using r14 as temporary if the TPF
tracing mechanism is in use.  Now I think this doesn't apply to r12,
so this part of your patch should still be fine.  (In addition, TPF
is not going to support split stacks --or indeed the Go language--
anyway, so it doesn't really matter all that much.)


Very well, I'll improve the comment.



I do have two other issues; sorry for bringing those up again although
they've been discussed up in the past, but I still think we can find
some improvements here ...

The first is the question Andreas brought up, why we need the extra
set of insns introduced by s390_reorg.  I think this may really have
been necessary for the ESA case where data elements had to be intermixed
into code at a specific location.  But since we no longer support ESA,
we now just have a data block that can be placed anywhere.  For example,
we could just have an insn (at any point in the prolog stream) that
simply emits the full data block during final output, along the lines of
(note: needs to be updated for SImode vs. DImode.):

(define_insn "split_stack_data"
   [(unspec_volatile [(match_operand 0 "bras_sym_operand" "X")
  (match_operand 1 "bras_sym_operand" "X")
  (match_operand 2 "consttable_operand" "X")
  (match_operand 3 "consttable_operand" "X")]
 UNSPECV_SPLIT_STACK_DATA)]
   ""
{
   switch_to_section (targetm.asm_out.function_rodata_section
   (current_function_decl));

   output_asm_insn (\".align 3", operands);
   (*targetm.asm_out.internal_label) (asm_out_file, \"L\",
  CODE_LABEL_NUMBER (operands[0]));
   output_asm_insn (\".quad %2\", operands);
   output_asm_insn (\".quad %3\", operands);
   output_asm_insn (\".quad %1-%0\", operands);

   switch_to_section (current_function_section ());
   return "";
}
   [(set_attr "length" "0")])

Or possibly even cleaner, we can simply define the data block at the
tree level as if it were an initialized global variable of a certain
struct type, and just leave it to common code to emit it as usual.

Then we just have the code bits, but I don't really see much
difference between the split_stack_call and split_stack_sibcall
patterns (apart from the data block), so if code flow is OK with
the former insns, it should be OK with the latter too ..

[ Or else, if there *are* code flow issues, the other alternative
would be to emit the full call sequence, code and data, from a
single insn pattern during final output.  This might have the extra
benefit that the assembler sequence is fully fixed, and thus easier
to detect in the linker.  ]

Getting rid of the extra transformation in s390_reorg would not
just remove a bunch of code from the back-end (always good!),
it would also speed up compile time a bit.


When I wasn't using reorg, I had problems with gcc deleting the label in 
.rodata, since it wasn't used by any jump instruction.  I guess having a 
whole-block instruction that emits the label on its own should solve the 
issue, though - let's try that.



The second issue I'm still not sure about is the magic nop marker
for frameless functions.  In an earlier mail you wrote:


Both currently supported
architectures always emit split-stack code on every function.


At least for rs6000 this doesn't appear to be true; in
rs6000_expand_split_stack_prologue we have:

   if (!info->push_p)
 return;

so it does nothing for frameless routines.

Now on i386 we do indeed generate code for frameless routines;
in fact, the *same* full stack check is generated as for any
other routine.  Now I'm wondering: is there are reason why
this check would be necessary (and there's simply a bug in
the rs6000 implementation)?  Then we obviously should do the
same on s390.


Try that on powerpc64(le):

$ cat a.c
#include 

void f(void) {
}

typedef void (*fptr)(void);

fptr g(void);

int main() {
printf("%p\n", g());
}

$ cat b.c
void f(void);

typedef void (*fptr)(void);

fptr g(void) {
return f;
}

$ gcc -O3 -fsplit-stack -c b.c
$ gcc -O3 -c a.c
$ gcc a.o b.o -fuse-ld=gold

I don't have a recent enough gcc for powerpc, but from what I've seen in 
the code, this should explode with a linker error.


Of course, mixin

Re: [PATCH] s390: Add -fsplit-stack support

2016-02-02 Thread Ulrich Weigand
Marcin Kościelnicki wrote:

> Here we go.  I've also removed the "see below", since I don't really
> see anything below...

The "see below" refers to this code (which I agree isn't really obvious):

  if (TARGET_TPF_PROFILING)
{
  /* Generate a BAS instruction to serve as a function
 entry intercept to facilitate the use of tracing
 algorithms located at the branch target.  */
  emit_insn (gen_prologue_tpf ());

What is not explicitly called out here is that this tracing function
actually refers to some hard registers, in particular r14, and assumes
they still have the original contents as at function entry.

That is why the prolog code avoid using r14 as temporary if the TPF
tracing mechanism is in use.  Now I think this doesn't apply to r12,
so this part of your patch should still be fine.  (In addition, TPF
is not going to support split stacks --or indeed the Go language--
anyway, so it doesn't really matter all that much.)


I do have two other issues; sorry for bringing those up again although
they've been discussed up in the past, but I still think we can find
some improvements here ...

The first is the question Andreas brought up, why we need the extra
set of insns introduced by s390_reorg.  I think this may really have
been necessary for the ESA case where data elements had to be intermixed
into code at a specific location.  But since we no longer support ESA,
we now just have a data block that can be placed anywhere.  For example,
we could just have an insn (at any point in the prolog stream) that
simply emits the full data block during final output, along the lines of
(note: needs to be updated for SImode vs. DImode.):

(define_insn "split_stack_data"
  [(unspec_volatile [(match_operand 0 "bras_sym_operand" "X")
 (match_operand 1 "bras_sym_operand" "X")
 (match_operand 2 "consttable_operand" "X")
 (match_operand 3 "consttable_operand" "X")]
UNSPECV_SPLIT_STACK_DATA)]
  ""
{
  switch_to_section (targetm.asm_out.function_rodata_section
  (current_function_decl));

  output_asm_insn (\".align 3", operands);
  (*targetm.asm_out.internal_label) (asm_out_file, \"L\",
 CODE_LABEL_NUMBER (operands[0]));
  output_asm_insn (\".quad %2\", operands);
  output_asm_insn (\".quad %3\", operands);
  output_asm_insn (\".quad %1-%0\", operands);

  switch_to_section (current_function_section ());
  return "";
}
  [(set_attr "length" "0")])

Or possibly even cleaner, we can simply define the data block at the
tree level as if it were an initialized global variable of a certain
struct type, and just leave it to common code to emit it as usual.

Then we just have the code bits, but I don't really see much
difference between the split_stack_call and split_stack_sibcall
patterns (apart from the data block), so if code flow is OK with
the former insns, it should be OK with the latter too ..

[ Or else, if there *are* code flow issues, the other alternative
would be to emit the full call sequence, code and data, from a
single insn pattern during final output.  This might have the extra
benefit that the assembler sequence is fully fixed, and thus easier
to detect in the linker.  ]

Getting rid of the extra transformation in s390_reorg would not
just remove a bunch of code from the back-end (always good!),
it would also speed up compile time a bit.


The second issue I'm still not sure about is the magic nop marker
for frameless functions.  In an earlier mail you wrote:

> Both currently supported 
> architectures always emit split-stack code on every function.

At least for rs6000 this doesn't appear to be true; in
rs6000_expand_split_stack_prologue we have:

  if (!info->push_p)
return;

so it does nothing for frameless routines.

Now on i386 we do indeed generate code for frameless routines;
in fact, the *same* full stack check is generated as for any
other routine.  Now I'm wondering: is there are reason why
this check would be necessary (and there's simply a bug in
the rs6000 implementation)?  Then we obviously should do the
same on s390.

On the other hand, if rs6000 works fine *without* any code
in frameless routines, why wouldn't that work for s390 too?

Emitting a nop (that is always executed) still looks weird to me.


Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  ulrich.weig...@de.ibm.com



[PATCH] s390: Add -fsplit-stack support

2016-02-02 Thread Marcin Kościelnicki
libgcc/ChangeLog:

* config.host: Use t-stack and t-stack-s390 for s390*-*-linux.
* config/s390/morestack.S: New file.
* config/s390/t-stack-s390: New file.
* generic-morestack.c (__splitstack_find): Add s390-specific code.

gcc/ChangeLog:

* common/config/s390/s390-common.c (s390_supports_split_stack):
New function.
(TARGET_SUPPORTS_SPLIT_STACK): New macro.
* config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
* config/s390/s390.c (struct machine_function): New field
split_stack_varargs_pointer.
(s390_register_info): Mark r12 as clobbered if it'll be used as temp
in s390_emit_prologue.
(s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
vararg pointer.
(morestack_ref): New global.
(SPLIT_STACK_AVAILABLE): New macro.
(s390_expand_split_stack_prologue): New function.
(s390_expand_split_stack_call): New function.
(s390_live_on_entry): New function.
(s390_va_start): Use split-stack vararg pointer if appropriate.
(s390_reorg): Lower the split-stack pseudo-insns.
(s390_asm_file_end): Emit the split-stack note sections.
(TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
* config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
(UNSPECV_SPLIT_STACK_CALL): New unspec.
(UNSPECV_SPLIT_STACK_SIBCALL): New unspec.
(UNSPECV_SPLIT_STACK_MARKER): New unspec.
(split_stack_prologue): New expand.
(split_stack_call): New expand.
(split_stack_call_*): New insn.
(split_stack_cond_call): New expand.
(split_stack_cond_call_*): New insn.
(split_stack_space_check): New expand.
(split_stack_sibcall): New expand.
(split_stack_sibcall_*): New insn.
(split_stack_cond_sibcall): New expand.
(split_stack_cond_sibcall_*): New insn.
(split_stack_marker): New insn.
---
Here we go.  I've also removed the "see below", since I don't really
see anything below...

 gcc/ChangeLog|  37 +++
 gcc/common/config/s390/s390-common.c |  14 +
 gcc/config/s390/s390-protos.h|   1 +
 gcc/config/s390/s390.c   | 323 ++-
 gcc/config/s390/s390.md  | 177 ++
 libgcc/ChangeLog |   7 +
 libgcc/config.host   |   4 +-
 libgcc/config/s390/morestack.S   | 609 +++
 libgcc/config/s390/t-stack-s390  |   2 +
 libgcc/generic-morestack.c   |   4 +
 10 files changed, 1171 insertions(+), 7 deletions(-)
 create mode 100644 libgcc/config/s390/morestack.S
 create mode 100644 libgcc/config/s390/t-stack-s390

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 9a2cec8..af86079 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,40 @@
+2016-02-02  Marcin Kościelnicki  
+
+   * common/config/s390/s390-common.c (s390_supports_split_stack):
+   New function.
+   (TARGET_SUPPORTS_SPLIT_STACK): New macro.
+   * config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
+   * config/s390/s390.c (struct machine_function): New field
+   split_stack_varargs_pointer.
+   (s390_register_info): Mark r12 as clobbered if it'll be used as temp
+   in s390_emit_prologue.
+   (s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
+   vararg pointer.
+   (morestack_ref): New global.
+   (SPLIT_STACK_AVAILABLE): New macro.
+   (s390_expand_split_stack_prologue): New function.
+   (s390_expand_split_stack_call): New function.
+   (s390_live_on_entry): New function.
+   (s390_va_start): Use split-stack vararg pointer if appropriate.
+   (s390_reorg): Lower the split-stack pseudo-insns.
+   (s390_asm_file_end): Emit the split-stack note sections.
+   (TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
+   * config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
+   (UNSPECV_SPLIT_STACK_CALL): New unspec.
+   (UNSPECV_SPLIT_STACK_SIBCALL): New unspec.
+   (UNSPECV_SPLIT_STACK_MARKER): New unspec.
+   (split_stack_prologue): New expand.
+   (split_stack_call): New expand.
+   (split_stack_call_*): New insn.
+   (split_stack_cond_call): New expand.
+   (split_stack_cond_call_*): New insn.
+   (split_stack_space_check): New expand.
+   (split_stack_sibcall): New expand.
+   (split_stack_sibcall_*): New insn.
+   (split_stack_cond_sibcall): New expand.
+   (split_stack_cond_sibcall_*): New insn.
+   (split_stack_marker): New insn.
+
 2016-02-02  Thomas Schwinge  
 
* omp-builtins.def (BUILT_IN_GOACC_HOST_DATA): Remove.
diff --git a/gcc/common/config/s390/s390-common.c 
b/gcc/common/config/s390/s390-common.c
index 4519c21..1e497e6 100644
--- a/gcc/common/config/s390/s390-common.c
+++ b/gcc/common/config/s390/s390-common.c
@@ -105,6 +105,17 @@ s390_handle_option (struct gcc_options *op

Re: [PATCH] s390: Add -fsplit-stack support

2016-02-02 Thread Andreas Krebbel
On 02/02/2016 03:52 PM, Marcin Kościelnicki wrote:
> libgcc/ChangeLog:
> 
>   * config.host: Use t-stack and t-stack-s390 for s390*-*-linux.
>   * config/s390/morestack.S: New file.
>   * config/s390/t-stack-s390: New file.
>   * generic-morestack.c (__splitstack_find): Add s390-specific code.
> 
> gcc/ChangeLog:
> 
>   * common/config/s390/s390-common.c (s390_supports_split_stack):
>   New function.
>   (TARGET_SUPPORTS_SPLIT_STACK): New macro.
>   * config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
>   * config/s390/s390.c (struct machine_function): New field
>   split_stack_varargs_pointer.
>   (s390_register_info): Mark r12 as clobbered if it'll be used as temp
>   in s390_emit_prologue.
>   (s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
>   vararg pointer.
>   (morestack_ref): New global.
>   (SPLIT_STACK_AVAILABLE): New macro.
>   (s390_expand_split_stack_prologue): New function.
>   (s390_expand_split_stack_call): New function.
>   (s390_live_on_entry): New function.
>   (s390_va_start): Use split-stack vararg pointer if appropriate.
>   (s390_reorg): Lower the split-stack pseudo-insns.
>   (s390_asm_file_end): Emit the split-stack note sections.
>   (TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
>   * config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
>   (UNSPECV_SPLIT_STACK_CALL): New unspec.
>   (UNSPECV_SPLIT_STACK_SIBCALL): New unspec.
>   (UNSPECV_SPLIT_STACK_MARKER): New unspec.
>   (split_stack_prologue): New expand.
>   (split_stack_call): New expand.
>   (split_stack_call_*): New insn.
>   (split_stack_cond_call): New expand.
>   (split_stack_cond_call_*): New insn.
>   (split_stack_space_check): New expand.
>   (split_stack_sibcall): New expand.
>   (split_stack_sibcall_*): New insn.
>   (split_stack_cond_sibcall): New expand.
>   (split_stack_cond_sibcall_*): New insn.
>   (split_stack_marker): New insn.
> ---
> I've implemented most of your requested changes, with two exceptions:
> 
> - I don't use virtual_incoming_args_rtx in s390_expand_split_stack_prologue,
>   since this causes constraint error - I suppose it just cannot be used after
>   reload.
Right. As an elimination reg it cannot be used in the code path called from 
s390_reorg.

> - It seems to me there's no problem with TPF and r1 - the conditional you
>   mention is meant to avoid modifying r14 (which we do - by aiming at r1 and
>   r12 for arg pointer and temp, respectively), not to ensure use of r1 as the
>   temporary.  Unless there's a good reason to avoid modifying r12, the code
>   seems fine to me.
Ok. The comment above this check then does not seem to be correct anymore. 
Could you please adjust
it as well. It should read "avoid register 14" then.

  /* Choose best register to use for temp use within prologue.
 See below for why TPF must use the register 1.  */

  if (!has_hard_reg_initial_val (Pmode, RETURN_REGNUM)
  && !crtl->is_leaf
  && !TARGET_TPF_PROFILING)
temp_reg = gen_rtx_REG (Pmode, RETURN_REGNUM);
...

-Andreas-



> 
> As for the testcase we discussed, I'll submit it as a separate patch.
> 
> 
>  gcc/ChangeLog|  37 +++
>  gcc/common/config/s390/s390-common.c |  14 +
>  gcc/config/s390/s390-protos.h|   1 +
>  gcc/config/s390/s390.c   | 321 +-
>  gcc/config/s390/s390.md  | 177 ++
>  libgcc/ChangeLog |   7 +
>  libgcc/config.host   |   4 +-
>  libgcc/config/s390/morestack.S   | 609 
> +++
>  libgcc/config/s390/t-stack-s390  |   2 +
>  libgcc/generic-morestack.c   |   4 +
>  10 files changed, 1170 insertions(+), 6 deletions(-)
>  create mode 100644 libgcc/config/s390/morestack.S
>  create mode 100644 libgcc/config/s390/t-stack-s390
> 
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 9a2cec8..af86079 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,40 @@
> +2016-02-02  Marcin Kościelnicki  
> +
> + * common/config/s390/s390-common.c (s390_supports_split_stack):
> + New function.
> + (TARGET_SUPPORTS_SPLIT_STACK): New macro.
> + * config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
> + * config/s390/s390.c (struct machine_function): New field
> + split_stack_varargs_pointer.
> + (s390_register_info): Mark r12 as clobbered if it'll be used as temp
> + in s390_emit_prologue.
> + (s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
> + vararg pointer.
> + (morestack_ref): New global.
> + (SPLIT_STACK_AVAILABLE): New macro.
> + (s390_expand_split_stack_prologue): New function.
> + (s390_expand_split_stack_call): New function.
> + (s390_live_on_entry): New function.
> + (s390_va_start): Use split-stack vararg pointer if appropriate.
> + (s390_

[PATCH] s390: Add -fsplit-stack support

2016-02-02 Thread Marcin Kościelnicki
libgcc/ChangeLog:

* config.host: Use t-stack and t-stack-s390 for s390*-*-linux.
* config/s390/morestack.S: New file.
* config/s390/t-stack-s390: New file.
* generic-morestack.c (__splitstack_find): Add s390-specific code.

gcc/ChangeLog:

* common/config/s390/s390-common.c (s390_supports_split_stack):
New function.
(TARGET_SUPPORTS_SPLIT_STACK): New macro.
* config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
* config/s390/s390.c (struct machine_function): New field
split_stack_varargs_pointer.
(s390_register_info): Mark r12 as clobbered if it'll be used as temp
in s390_emit_prologue.
(s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
vararg pointer.
(morestack_ref): New global.
(SPLIT_STACK_AVAILABLE): New macro.
(s390_expand_split_stack_prologue): New function.
(s390_expand_split_stack_call): New function.
(s390_live_on_entry): New function.
(s390_va_start): Use split-stack vararg pointer if appropriate.
(s390_reorg): Lower the split-stack pseudo-insns.
(s390_asm_file_end): Emit the split-stack note sections.
(TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
* config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
(UNSPECV_SPLIT_STACK_CALL): New unspec.
(UNSPECV_SPLIT_STACK_SIBCALL): New unspec.
(UNSPECV_SPLIT_STACK_MARKER): New unspec.
(split_stack_prologue): New expand.
(split_stack_call): New expand.
(split_stack_call_*): New insn.
(split_stack_cond_call): New expand.
(split_stack_cond_call_*): New insn.
(split_stack_space_check): New expand.
(split_stack_sibcall): New expand.
(split_stack_sibcall_*): New insn.
(split_stack_cond_sibcall): New expand.
(split_stack_cond_sibcall_*): New insn.
(split_stack_marker): New insn.
---
I've implemented most of your requested changes, with two exceptions:

- I don't use virtual_incoming_args_rtx in s390_expand_split_stack_prologue,
  since this causes constraint error - I suppose it just cannot be used after
  reload.
- It seems to me there's no problem with TPF and r1 - the conditional you
  mention is meant to avoid modifying r14 (which we do - by aiming at r1 and
  r12 for arg pointer and temp, respectively), not to ensure use of r1 as the
  temporary.  Unless there's a good reason to avoid modifying r12, the code
  seems fine to me.

As for the testcase we discussed, I'll submit it as a separate patch.


 gcc/ChangeLog|  37 +++
 gcc/common/config/s390/s390-common.c |  14 +
 gcc/config/s390/s390-protos.h|   1 +
 gcc/config/s390/s390.c   | 321 +-
 gcc/config/s390/s390.md  | 177 ++
 libgcc/ChangeLog |   7 +
 libgcc/config.host   |   4 +-
 libgcc/config/s390/morestack.S   | 609 +++
 libgcc/config/s390/t-stack-s390  |   2 +
 libgcc/generic-morestack.c   |   4 +
 10 files changed, 1170 insertions(+), 6 deletions(-)
 create mode 100644 libgcc/config/s390/morestack.S
 create mode 100644 libgcc/config/s390/t-stack-s390

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 9a2cec8..af86079 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,40 @@
+2016-02-02  Marcin Kościelnicki  
+
+   * common/config/s390/s390-common.c (s390_supports_split_stack):
+   New function.
+   (TARGET_SUPPORTS_SPLIT_STACK): New macro.
+   * config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
+   * config/s390/s390.c (struct machine_function): New field
+   split_stack_varargs_pointer.
+   (s390_register_info): Mark r12 as clobbered if it'll be used as temp
+   in s390_emit_prologue.
+   (s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
+   vararg pointer.
+   (morestack_ref): New global.
+   (SPLIT_STACK_AVAILABLE): New macro.
+   (s390_expand_split_stack_prologue): New function.
+   (s390_expand_split_stack_call): New function.
+   (s390_live_on_entry): New function.
+   (s390_va_start): Use split-stack vararg pointer if appropriate.
+   (s390_reorg): Lower the split-stack pseudo-insns.
+   (s390_asm_file_end): Emit the split-stack note sections.
+   (TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
+   * config/s390/s390.md (UNSPEC_STACK_CHECK): New unspec.
+   (UNSPECV_SPLIT_STACK_CALL): New unspec.
+   (UNSPECV_SPLIT_STACK_SIBCALL): New unspec.
+   (UNSPECV_SPLIT_STACK_MARKER): New unspec.
+   (split_stack_prologue): New expand.
+   (split_stack_call): New expand.
+   (split_stack_call_*): New insn.
+   (split_stack_cond_call): New expand.
+   (split_stack_cond_call_*): New insn.
+   (split_stack_space_check): New expand.
+   (split_stack_sibcall): New expand.
+

Re: [PATCH] s390: Add -fsplit-stack support

2016-01-29 Thread Andreas Krebbel
On 01/29/2016 04:43 PM, Marcin Kościelnicki wrote:
> The testsuite with -fsplit-stack already hits all of them, and checking 
> them manually is rather tricky (I don't know if it could be done in 
> target-independent way at all), but I think it'd be reasonable to make 
> assembly testcases calling __morestack for the last two cases, to check 
> if the registers are being preserved, etc.
Sounds good. Thanks!

...
>>> +  if (frame_size <= 0x7fff || (TARGET_EXTIMM && frame_size <= 0xu))
>> The agfi immediate value is a signed 32 bit integer.  So you can only
>> add up to 2G-1.  I think it would be more readable to write this as:
> 
> We're emitting ALGFI here, which accepts unsigned 32-bit integer.
Ah right. Then it would be:

if (CONST_OK_FOR_K (frame_size) || CONST_OK_FOR_Op (frame_size))

instead.

>>
>> if (CONST_OK_FOR_K (frame_size) || CONST_OK_FOR_Os (frame_size))
>>
>> as in s390_emit_prologue. The Os check will check for TARGET_EXTIMM as well.
> 
> Alright.

...

>> I'm wondering if it is really necessary to expand the call in that
>> two-step approach?! We do the general literal pool handling in
>> s390_reorg because we need all the insn lengths to be finalized before
>> performing the branch/pool splitting loop.  But this shouldn't be necessary
>> in this case.  Would it be possible to expand the call already in
>> emit_prologue phase and get rid of the s390_reorg part?
> 
> There's an internal literal pool involved, which needs to be emitted as 
> one chunk.  Optimizations are also very likely to destroy the sequence: 
> consider the target address that __morestack will call - the control 
> flow change happens in __morestack jump instruction, but the address 
> itself is encoded in one of the pool literals.  Just not worth the risk.
Ok.

...
>>> +   # OK, no stack allocation needed.  We still follow the protocol and
>>> +   # call our caller - it doesn't cost much and makes sure vararg works.
>>> +   # No need to set any registers here - %r0 and %r2-%r6 weren't modified.
>>> +   basr%r14, %r10  # Call our caller.
>> The comment confuses me.  It somewhat sounds to me like the call
>> wouldn't be really needed but in fact it cannot even remotely work
>> without jumping back to the function body right?!
> 
> Certainly.  __morestack's task is to call the given function entry point 
> once the necessary stack space is established.  In fact, in the no 
> allocation case, a sibling-call would actually be possible, if it 
> weren't for one annoying detail: there are no free GPRs we could use to 
> keep the address of the entry point - %r0 may be used to keep static 
> chain, %r1 may have to be the argument pointer, %r2-%r5 may be used to 
> keep parameters, and %r6-%r15 are callee-saved.
Ok. The comment isn't about no-call vs. call it is about sibcall vs. call - got 
it.

Bye,

-Andreas-



Re: [PATCH] s390: Add -fsplit-stack support

2016-01-29 Thread Marcin Kościelnicki

On 29/01/16 14:33, Andreas Krebbel wrote:

Hi Marcin,

sorry for the late feedback.

A few comments regarding the split stack implementation:

The GNU coding style requires to replace every 8 leading blanks on a
line with a tab.  There are many lines in your patch violating this.
In case you are an emacs user `whitespace-cleanup' will fix this for
you.


OK, will do.


Could you please add a testcase checking the different
variants. I.e. with early exit, no-alloc in __morestack, and with an
actual allocation?


The testsuite with -fsplit-stack already hits all of them, and checking 
them manually is rather tricky (I don't know if it could be done in 
target-independent way at all), but I think it'd be reasonable to make 
assembly testcases calling __morestack for the last two cases, to check 
if the registers are being preserved, etc.




There are a few more comments inline.

Bye,

-Andreas-


diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index c881d52..71f6f38 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,38 @@
  2016-01-16  Marcin Kościelnicki  

+   * common/config/s390/s390-common.c (s390_supports_split_stack):
+   New function.
+   (TARGET_SUPPORTS_SPLIT_STACK): New macro.
+   * config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
+   * config/s390/s390.c (struct machine_function): New field
+   split_stack_varargs_pointer.
+   (s390_register_info): Mark r12 as clobbered if it'll be used as temp
+   in s390_emit_prologue.
+   (s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
+   vararg pointer.
+   (morestack_ref): New global.
+   (SPLIT_STACK_AVAILABLE): New macro.
+   (s390_expand_split_stack_prologue): New function.
+   (s390_expand_split_stack_call): New function.
+   (s390_live_on_entry): New function.
+   (s390_va_start): Use split-stack vararg pointer if appropriate.
+   (s390_reorg): Lower the split-stack pseudo-insns.
+   (s390_asm_file_end): Emit the split-stack note sections.
+   (TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
+   * config/s390/s390.md: (UNSPEC_STACK_CHECK): New unspec.
+   (UNSPECV_SPLIT_STACK_CALL): New unspec.
+   (UNSPECV_SPLIT_STACK_SIBCALL): New unspec.
+   (UNSPECV_SPLIT_STACK_MARKER): New unspec.
+   (split_stack_prologue): New expand.
+   (split_stack_call_*): New insn.
+   (split_stack_cond_call_*): New insn.
+   (split_stack_space_check): New expand.
+   (split_stack_sibcall_*): New insn.
+   (split_stack_cond_sibcall_*): New insn.
+   (split_stack_marker): New insn.
+
+2016-01-02  Marcin Kościelnicki  
+
* cfgrtl.c (rtl_tidy_fallthru_edge): Bail for unconditional jumps
with side effects.

diff --git a/gcc/common/config/s390/s390-common.c 
b/gcc/common/config/s390/s390-common.c
index 4519c21..1e497e6 100644
--- a/gcc/common/config/s390/s390-common.c
+++ b/gcc/common/config/s390/s390-common.c
@@ -105,6 +105,17 @@ s390_handle_option (struct gcc_options *opts 
ATTRIBUTE_UNUSED,
  }
  }

+/* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
+   We don't verify it, since earlier versions just have padding at
+   its place, which works just as well.  */
+
+static bool
+s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
+  struct gcc_options *opts ATTRIBUTE_UNUSED)
+{
+  return true;
+}
+
  #undef TARGET_DEFAULT_TARGET_FLAGS
  #define TARGET_DEFAULT_TARGET_FLAGS (TARGET_DEFAULT)

@@ -117,4 +128,7 @@ s390_handle_option (struct gcc_options *opts 
ATTRIBUTE_UNUSED,
  #undef TARGET_OPTION_INIT_STRUCT
  #define TARGET_OPTION_INIT_STRUCT s390_option_init_struct

+#undef TARGET_SUPPORTS_SPLIT_STACK
+#define TARGET_SUPPORTS_SPLIT_STACK s390_supports_split_stack
+
  struct gcc_targetm_common targetm_common = TARGETM_COMMON_INITIALIZER;
diff --git a/gcc/config/s390/s390-protos.h b/gcc/config/s390/s390-protos.h
index 633bc1e..09032c9 100644
--- a/gcc/config/s390/s390-protos.h
+++ b/gcc/config/s390/s390-protos.h
@@ -42,6 +42,7 @@ extern bool s390_handle_option (struct gcc_options *opts 
ATTRIBUTE_UNUSED,
  extern HOST_WIDE_INT s390_initial_elimination_offset (int, int);
  extern void s390_emit_prologue (void);
  extern void s390_emit_epilogue (bool);
+extern void s390_expand_split_stack_prologue (void);
  extern bool s390_can_use_simple_return_insn (void);
  extern bool s390_can_use_return_insn (void);
  extern void s390_function_profiler (FILE *, int);
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 3be64de..6afce7c 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -426,6 +426,13 @@ struct GTY(()) machine_function
/* True if the current function may contain a tbegin clobbering
   FPRs.  */
bool tbegin_p;
+
+  /* For -fsplit-stack support: A stack local which holds a pointer to
+ the stack arguments for a function with a variable number of
+ arguments.  This is set at the start of the function and is used
+   

Re: [PATCH] s390: Add -fsplit-stack support

2016-01-29 Thread Andreas Krebbel
Hi Marcin,

sorry for the late feedback.

A few comments regarding the split stack implementation:

The GNU coding style requires to replace every 8 leading blanks on a
line with a tab.  There are many lines in your patch violating this.
In case you are an emacs user `whitespace-cleanup' will fix this for
you.

Could you please add a testcase checking the different
variants. I.e. with early exit, no-alloc in __morestack, and with an
actual allocation?

There are a few more comments inline.

Bye,

-Andreas-

> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index c881d52..71f6f38 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,5 +1,38 @@
>  2016-01-16  Marcin Kościelnicki  
> 
> + * common/config/s390/s390-common.c (s390_supports_split_stack):
> + New function.
> + (TARGET_SUPPORTS_SPLIT_STACK): New macro.
> + * config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
> + * config/s390/s390.c (struct machine_function): New field
> + split_stack_varargs_pointer.
> + (s390_register_info): Mark r12 as clobbered if it'll be used as temp
> + in s390_emit_prologue.
> + (s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
> + vararg pointer.
> + (morestack_ref): New global.
> + (SPLIT_STACK_AVAILABLE): New macro.
> + (s390_expand_split_stack_prologue): New function.
> + (s390_expand_split_stack_call): New function.
> + (s390_live_on_entry): New function.
> + (s390_va_start): Use split-stack vararg pointer if appropriate.
> + (s390_reorg): Lower the split-stack pseudo-insns.
> + (s390_asm_file_end): Emit the split-stack note sections.
> + (TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
> + * config/s390/s390.md: (UNSPEC_STACK_CHECK): New unspec.
> + (UNSPECV_SPLIT_STACK_CALL): New unspec.
> + (UNSPECV_SPLIT_STACK_SIBCALL): New unspec.
> + (UNSPECV_SPLIT_STACK_MARKER): New unspec.
> + (split_stack_prologue): New expand.
> + (split_stack_call_*): New insn.
> + (split_stack_cond_call_*): New insn.
> + (split_stack_space_check): New expand.
> + (split_stack_sibcall_*): New insn.
> + (split_stack_cond_sibcall_*): New insn.
> + (split_stack_marker): New insn.
> +
> +2016-01-02  Marcin Kościelnicki  
> +
>   * cfgrtl.c (rtl_tidy_fallthru_edge): Bail for unconditional jumps
>   with side effects.
> 
> diff --git a/gcc/common/config/s390/s390-common.c 
> b/gcc/common/config/s390/s390-common.c
> index 4519c21..1e497e6 100644
> --- a/gcc/common/config/s390/s390-common.c
> +++ b/gcc/common/config/s390/s390-common.c
> @@ -105,6 +105,17 @@ s390_handle_option (struct gcc_options *opts 
> ATTRIBUTE_UNUSED,
>  }
>  }
> 
> +/* -fsplit-stack uses a field in the TCB, available with glibc-2.23.
> +   We don't verify it, since earlier versions just have padding at
> +   its place, which works just as well.  */
> +
> +static bool
> +s390_supports_split_stack (bool report ATTRIBUTE_UNUSED,
> +struct gcc_options *opts ATTRIBUTE_UNUSED)
> +{
> +  return true;
> +}
> +
>  #undef TARGET_DEFAULT_TARGET_FLAGS
>  #define TARGET_DEFAULT_TARGET_FLAGS (TARGET_DEFAULT)
> 
> @@ -117,4 +128,7 @@ s390_handle_option (struct gcc_options *opts 
> ATTRIBUTE_UNUSED,
>  #undef TARGET_OPTION_INIT_STRUCT
>  #define TARGET_OPTION_INIT_STRUCT s390_option_init_struct
> 
> +#undef TARGET_SUPPORTS_SPLIT_STACK
> +#define TARGET_SUPPORTS_SPLIT_STACK s390_supports_split_stack
> +
>  struct gcc_targetm_common targetm_common = TARGETM_COMMON_INITIALIZER;
> diff --git a/gcc/config/s390/s390-protos.h b/gcc/config/s390/s390-protos.h
> index 633bc1e..09032c9 100644
> --- a/gcc/config/s390/s390-protos.h
> +++ b/gcc/config/s390/s390-protos.h
> @@ -42,6 +42,7 @@ extern bool s390_handle_option (struct gcc_options *opts 
> ATTRIBUTE_UNUSED,
>  extern HOST_WIDE_INT s390_initial_elimination_offset (int, int);
>  extern void s390_emit_prologue (void);
>  extern void s390_emit_epilogue (bool);
> +extern void s390_expand_split_stack_prologue (void);
>  extern bool s390_can_use_simple_return_insn (void);
>  extern bool s390_can_use_return_insn (void);
>  extern void s390_function_profiler (FILE *, int);
> diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
> index 3be64de..6afce7c 100644
> --- a/gcc/config/s390/s390.c
> +++ b/gcc/config/s390/s390.c
> @@ -426,6 +426,13 @@ struct GTY(()) machine_function
>/* True if the current function may contain a tbegin clobbering
>   FPRs.  */
>bool tbegin_p;
> +
> +  /* For -fsplit-stack support: A stack local which holds a pointer to
> + the stack arguments for a function with a variable number of
> + arguments.  This is set at the start of the function and is used
> + to initialize the overflow_arg_area field of the va_list
> + structure.  */
> +  rtx split_stack_varargs_pointer;
>  };
> 
>  /* Few accessor macros for struct cfun->machine->s390_frame_layout.  */
> @@ -9316,9 +9323,13 @@ s390_register_info ()
> cfun_frame_lay

[PATCH] s390: Add -fsplit-stack support

2016-01-16 Thread Marcin Kościelnicki
libgcc/ChangeLog:

* config.host: Use t-stack and t-stack-s390 for s390*-*-linux.
* config/s390/morestack.S: New file.
* config/s390/t-stack-s390: New file.
* generic-morestack.c (__splitstack_find): Add s390-specific code.

gcc/ChangeLog:

* common/config/s390/s390-common.c (s390_supports_split_stack):
New function.
(TARGET_SUPPORTS_SPLIT_STACK): New macro.
* config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
* config/s390/s390.c (struct machine_function): New field
split_stack_varargs_pointer.
(s390_register_info): Mark r12 as clobbered if it'll be used as temp
in s390_emit_prologue.
(s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
vararg pointer.
(morestack_ref): New global.
(SPLIT_STACK_AVAILABLE): New macro.
(s390_expand_split_stack_prologue): New function.
(s390_expand_split_stack_call): New function.
(s390_live_on_entry): New function.
(s390_va_start): Use split-stack vararg pointer if appropriate.
(s390_reorg): Lower the split-stack pseudo-insns.
(s390_asm_file_end): Emit the split-stack note sections.
(TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
* config/s390/s390.md: (UNSPEC_STACK_CHECK): New unspec.
(UNSPECV_SPLIT_STACK_CALL): New unspec.
(UNSPECV_SPLIT_STACK_SIBCALL): New unspec.
(UNSPECV_SPLIT_STACK_MARKER): New unspec.
(split_stack_prologue): New expand.
(split_stack_call_*): New insn.
(split_stack_cond_call_*): New insn.
(split_stack_space_check): New expand.
(split_stack_sibcall_*): New insn.
(split_stack_cond_sibcall_*): New insn.
(split_stack_marker): New insn.
---
Support for !TARGET_CPU_ZARCH removed and sorried.  I've also cleaned up
the 31-bit versions of morestack.S routines to more closely mirror their
64-bit counterparts, since I can now use the newer opcodes.

I'm also submitting a new version of the gold patch, which has support for
old CPUs likewise removed.

 gcc/ChangeLog|  33 ++
 gcc/common/config/s390/s390-common.c |  14 +
 gcc/config/s390/s390-protos.h|   1 +
 gcc/config/s390/s390.c   | 371 -
 gcc/config/s390/s390.md  | 109 +++
 libgcc/ChangeLog |   7 +
 libgcc/config.host   |   4 +-
 libgcc/config/s390/morestack.S   | 609 +++
 libgcc/config/s390/t-stack-s390  |   2 +
 libgcc/generic-morestack.c   |   4 +
 10 files changed, 1148 insertions(+), 6 deletions(-)
 create mode 100644 libgcc/config/s390/morestack.S
 create mode 100644 libgcc/config/s390/t-stack-s390

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index c881d52..71f6f38 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,38 @@
 2016-01-16  Marcin Kościelnicki  
 
+   * common/config/s390/s390-common.c (s390_supports_split_stack):
+   New function.
+   (TARGET_SUPPORTS_SPLIT_STACK): New macro.
+   * config/s390/s390-protos.h: Add s390_expand_split_stack_prologue.
+   * config/s390/s390.c (struct machine_function): New field
+   split_stack_varargs_pointer.
+   (s390_register_info): Mark r12 as clobbered if it'll be used as temp
+   in s390_emit_prologue.
+   (s390_emit_prologue): Use r12 as temp if r1 is taken by split-stack
+   vararg pointer.
+   (morestack_ref): New global.
+   (SPLIT_STACK_AVAILABLE): New macro.
+   (s390_expand_split_stack_prologue): New function.
+   (s390_expand_split_stack_call): New function.
+   (s390_live_on_entry): New function.
+   (s390_va_start): Use split-stack vararg pointer if appropriate.
+   (s390_reorg): Lower the split-stack pseudo-insns.
+   (s390_asm_file_end): Emit the split-stack note sections.
+   (TARGET_EXTRA_LIVE_ON_ENTRY): New macro.
+   * config/s390/s390.md: (UNSPEC_STACK_CHECK): New unspec.
+   (UNSPECV_SPLIT_STACK_CALL): New unspec.
+   (UNSPECV_SPLIT_STACK_SIBCALL): New unspec.
+   (UNSPECV_SPLIT_STACK_MARKER): New unspec.
+   (split_stack_prologue): New expand.
+   (split_stack_call_*): New insn.
+   (split_stack_cond_call_*): New insn.
+   (split_stack_space_check): New expand.
+   (split_stack_sibcall_*): New insn.
+   (split_stack_cond_sibcall_*): New insn.
+   (split_stack_marker): New insn.
+
+2016-01-02  Marcin Kościelnicki  
+
* cfgrtl.c (rtl_tidy_fallthru_edge): Bail for unconditional jumps
with side effects.
 
diff --git a/gcc/common/config/s390/s390-common.c 
b/gcc/common/config/s390/s390-common.c
index 4519c21..1e497e6 100644
--- a/gcc/common/config/s390/s390-common.c
+++ b/gcc/common/config/s390/s390-common.c
@@ -105,6 +105,17 @@ s390_handle_option (struct gcc_options *opts 
ATTRIBUTE_UNUSED,
 }
 }
 
+/* -fsplit-stack uses a field in the TCB, available wit