Initial shrink-wrapping patch

On 10/06/11 01:04, Ian Lance Taylor wrote:
> On Wed, Oct 5, 2011 at 10:17 AM, Bernd Schmidt <bernds@codesourcery.com> wrote:
>>
>> I've committed the following after a x86_64-linux bootstrap.
> 
> This patch appears to have broken the Go bootstrap when compiling a C
> file in libgo.  Here is a reduced test case:
> 
> static struct
> {
>   int n;
>   unsigned int m;
>   _Bool f;
> } g;
> 
> _Bool
> f (int s)
> {
>   unsigned int b, m;
> 
>   if (!g.f)
>     return 0;
>   b = 1 << s;
>   while (1)
>     {
>       m = g.m;
>       if (m & b)
>         break;
>       if (__sync_bool_compare_and_swap (&g.m, m, m|b))
>         {
>           if (m == 0)
>             f2 (&g.n);
>           break;
>         }
>     }
>   return 1;
> }
> 
> Compiling this file with -O2 -fsplit-stack causes an ICE:
> 
> foo.c:29:1: internal compiler error: in maybe_record_trace_start, at
> dwarf2cfi.c:2243
> 
> Compiling the file with -O2 -fsplit-stack -fno-shrink-wrap works.

This appears to be because the split prologue contains a jump, which
means the find_many_sub_blocks call reorders the block numbers, and our
indices into bb_flags are off.

I'm testing the following patch, but it won't finish today - feel free
to test and check it in, or to just disable shrink-wrapping with split
prologues.


Bernd

Bernd Schmidt <bernds@codesourcery.com> writes:

> This appears to be because the split prologue contains a jump, which
> means the find_many_sub_blocks call reorders the block numbers, and our
> indices into bb_flags are off.
>
> I'm testing the following patch, but it won't finish today - feel free
> to test and check it in, or to just disable shrink-wrapping with split
> prologues.

Thinking about it I think this is the wrong approach.  The -fsplit-stack
code by definition has to wrap the entire function and it can not modify
any callee-saved registers.  We should do shrink wrapping before
-fsplit-stack, not the other way around.

Ian

On 10/06/11 05:17, Ian Lance Taylor wrote:
> Thinking about it I think this is the wrong approach.  The -fsplit-stack
> code by definition has to wrap the entire function and it can not modify
> any callee-saved registers.  We should do shrink wrapping before
> -fsplit-stack, not the other way around.

Sorry, I'm not following what you're saying here. Can you elaborate?


Bernd

On 10/06/11 01:47, Bernd Schmidt wrote:
> This appears to be because the split prologue contains a jump, which
> means the find_many_sub_blocks call reorders the block numbers, and our
> indices into bb_flags are off.

Testing of the patch completed - ok? Regardless of split-stack it seems
like a cleanup and eliminates a potential source of errors.


Bernd

On 10/06/2011 06:37 AM, Bernd Schmidt wrote:
> On 10/06/11 01:47, Bernd Schmidt wrote:
>> This appears to be because the split prologue contains a jump, which
>> means the find_many_sub_blocks call reorders the block numbers, and our
>> indices into bb_flags are off.
> 
> Testing of the patch completed - ok? Regardless of split-stack it seems
> like a cleanup and eliminates a potential source of errors.

Yes, patch is ok.


r~

Bernd Schmidt <bernds@codesourcery.com> writes:

> On 10/06/11 05:17, Ian Lance Taylor wrote:
>> Thinking about it I think this is the wrong approach.  The -fsplit-stack
>> code by definition has to wrap the entire function and it can not modify
>> any callee-saved registers.  We should do shrink wrapping before
>> -fsplit-stack, not the other way around.
>
> Sorry, I'm not following what you're saying here. Can you elaborate?

Basically -fsplit-stack wraps the entire function in code that (on
x86_64) looks like

	cmpq	%fs:112, %rsp
	jae	.L2
	movl	$24, %r10d
	movl	$0, %r11d
	call	__morestack
	ret
.L2:

There is absolutely no reason to try to shrink wrap that code.  It will
never help.  That code always has to be first.  It especially has to be
first because the gold linker recognizes the prologue specially when a
split-stack function calls a non-split-stack function, in order to
request a larger stack.

Therefore, it seems to me that we should apply shrink wrapping to the
function as it exists *before* the split-stack prologue is created.  The
flag_split_stack bit should be moved after the flag_shrink_wrap bit.

Ian