Initial shrink-wrapping patch

On 10/05/11 17:13, Richard Guenther wrote:
> On Wed, Oct 5, 2011 at 12:29 AM, Richard Henderson <rth@redhat.com> wrote:
>> On 10/04/2011 03:10 PM, Bernd Schmidt wrote:
>>>       * doc/invoke.texi (-fshrink-wrap): Document.
>>>       * opts.c (default_options_table): Add it.
>>>       * common.opt (fshrink-wrap): Add.
>>>       * function.c (emit_return_into_block): Remove useless declaration.
>>>       (record_hard_reg_uses_1, record_hard_reg_uses, frame_required_for_rtx,
>>>       requires_stack_frame_p, gen_return_pattern): New static functions.
>>>       (emit_return_into_block): New arg simple_p.  All callers changed.
>>>       Use gen_return_pattern.
>>>       (thread_prologue_and_epilogue_insns): Implement shrink-wrapping.
>>>       * config/i386/i386.md (return): Expand into a simple_return.
>>>       (simple_return): New expander):
>>>       (simple_return_internal, simple_return_internal_long,
>>>       simple_return_pop_internal_long, simple_return_indirect_internal):
>>>       Renamed from return_internal, return_internal_long,
>>>       return_pop_internal_long and return_indirect_internal; changed to use
>>>       simple_return.
>>>       * config/i386/i386.c (ix86_expand_epilogue): Adjust to expand
>>>       simple returns.
>>>       (ix86_pad_returns): Likewise.
>>>       * function.h (struct rtl_data): Add member shrink_wrapped.
>>>       * cfgcleanup.c (outgoing_edges_match): If shrink-wrapped, edges that
>>>       are not jumps or sibcalls can't be compared.
>>>
>>>       * gcc.target/i386/sw-1.c: New test.
>>
>> Ok.
>>
>> As a followup, I think this option needs to be disabled for profiling
>> and profile_after_prologue.  Should be a mere matter of frobbing the
>> options at startup.
> 
> This breaks bootstrap on x86_64-linux.
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50621

Caused by the x86_64 expand_epilogue not generating REG_CFA_RESTORE
notes, and in another case by queuing but not emitting them.

Bootstrapping the following now. Ok? (Alternatively, could keep the
redzone logic, but make it depend on !flag_shrink_wrap).


Bernd
* config/i386/i386.c (ix86_add_cfa_restore_note): Lose CFA_OFFSET
	argument.  All callers changed.  Always emit a note.
	(ix86_expand_epilogue): Ensure queued cfa_adjust notes are attached
	to an insn.

On 10/05/2011 08:59 AM, Bernd Schmidt wrote:
> Bootstrapping the following now. Ok? (Alternatively, could keep the
> redzone logic, but make it depend on !flag_shrink_wrap).

It's a good space-saving optimization, that redzone logic.  We
should be able to keep it based on crtl->shrink_wrapped; that
does appear to get set before we actually emit the epilogue.


r~

HJ found some more maybe_record_trace_start failures. In one case I
debugged, we have

(insn/f 31 0 32 (parallel [
            (set (reg/f:DI 7 sp)
                (plus:DI (reg/f:DI 7 sp)
                    (const_int 8 [0x8])))
            (clobber (reg:CC 17 flags))
            (clobber (mem:BLK (scratch) [0 A8]))
        ]) -1
     (expr_list:REG_CFA_ADJUST_CFA (set (reg/f:DI 7 sp)
            (plus:DI (reg/f:DI 7 sp)
                (const_int 8 [0x8])))
        (nil)))

The insn pattern is later changed by csa to adjust by 24, and the note
is left untouched; that seems to be triggering the problem.

Richard, is there a reason to use REG_CFA_ADJUST_CFA rather than
REG_CFA_DEF_CFA? If no, I'll just try to fix i386.c not to emit the former.


Bernd

On 10/06/2011 11:03 AM, Bernd Schmidt wrote:
> HJ found some more maybe_record_trace_start failures. In one case I
> debugged, we have
> 
> (insn/f 31 0 32 (parallel [
>             (set (reg/f:DI 7 sp)
>                 (plus:DI (reg/f:DI 7 sp)
>                     (const_int 8 [0x8])))
>             (clobber (reg:CC 17 flags))
>             (clobber (mem:BLK (scratch) [0 A8]))
>         ]) -1
>      (expr_list:REG_CFA_ADJUST_CFA (set (reg/f:DI 7 sp)
>             (plus:DI (reg/f:DI 7 sp)
>                 (const_int 8 [0x8])))
>         (nil)))
> 
> The insn pattern is later changed by csa to adjust by 24, and the note
> is left untouched; that seems to be triggering the problem.

Hmm.  That seems a bit odd, considering this function probably does not
use alloca (no frame pointer), and uses accumulated outgoing arguments
(x86_64 never uses no-accumulate-outgoing-args, afaik).

> Richard, is there a reason to use REG_CFA_ADJUST_CFA rather than
> REG_CFA_DEF_CFA? If no, I'll just try to fix i386.c not to emit the former.

Not that I can think of.  But if that change makes any difference at all,
that's almost certainly another bug.

What PR are you looking at here?


r~

On 10/06/11 20:13, Richard Henderson wrote:
> What PR are you looking at here?

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50632

Testcase is gcc.dg/pr50132.c.


Bernd