diff mbox

Initial shrink-wrapping patch

Message ID 4E8DD0E3.5010106@codesourcery.com
State New
Headers show

Commit Message

Bernd Schmidt Oct. 6, 2011, 4:01 p.m. UTC
On 10/06/11 17:57, Ian Lance Taylor wrote:
> 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.

Urgh, ok.

> 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.

Sounds like we just need to always emit the split prologue on the
original entry edge then. Can you test the following with Go?


Bernd
* function.c (thread_prologue_and_epilogue_insns): Emit split
	prologue on the orig_entry_edge. Don't account for it in
	prologue_clobbered.

Comments

Richard Henderson Oct. 6, 2011, 5 p.m. UTC | #1
On 10/06/2011 09:01 AM, Bernd Schmidt wrote:
> On 10/06/11 17:57, Ian Lance Taylor wrote:
>> 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.
> 
> Urgh, ok.
> 
>> 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.
> 
> Sounds like we just need to always emit the split prologue on the
> original entry edge then. Can you test the following with Go?

Looks reasonable.

I wonder if we can have this as a generic feature?  I'm thinking about
things like the MIPS and Alpha load-gp stuff.  That operation also needs
to happen exactly at the start of the function, due to the pc-relative
nature of the operation.

I do see that MIPS works around this by emitting the load-gp as text
in the legacy prologue.  But Alpha makes some effort to emit this as
rtl, so that the scheduler knows about the two pipeline reservations
and the latency of any use of the gp register.

Would a "pre_prologue" named pattern seem wrong to anyone?


r~
Ian Lance Taylor Oct. 6, 2011, 7:33 p.m. UTC | #2
Bernd Schmidt <bernds@codesourcery.com> writes:

> On 10/06/11 17:57, Ian Lance Taylor wrote:
>> 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.
>
> Urgh, ok.
>
>> 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.
>
> Sounds like we just need to always emit the split prologue on the
> original entry edge then. Can you test the following with Go?

Unfortunately, that patch fails compiling libgo/runtime/go-map-len.c.
go-map-len.c is a C file, and actually quite a simple one.

../../../trunk/libgo/runtime/go-map-len.c: In function ‘__go_map_len’:
../../../trunk/libgo/runtime/go-map-len.c:23:1: error: in basic block 2:
../../../trunk/libgo/runtime/go-map-len.c:23:1: error: flow control insn inside 
a basic block
(jump_insn 38 37 39 2 (set (pc)
        (if_then_else (geu (reg:CC 17 flags)
                (const_int 0 [0]))
            (label_ref 43)
            (pc))) ../../../trunk/libgo/runtime/go-map-len.c:18 -1
     (expr_list:REG_DEAD (reg:CC 17 flags)
        (expr_list:REG_BR_PROB (const_int 9900 [0x26ac])
            (nil)))
 -> 43)
../../../trunk/libgo/runtime/go-map-len.c:23:1: internal compiler error: in rtl_
verify_flow_info_1, at cfgrtl.c:2001
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.


Note that you can test Go yourself by adding --enable-languages=go to
your configure line.  Nothing special is required to build Go.  It works
better if you use the gold linker but that is not required.

Ian
diff mbox

Patch

Index: gcc/function.c
===================================================================
--- gcc/function.c	(revision 179619)
+++ gcc/function.c	(working copy)
@@ -5602,10 +5602,6 @@  thread_prologue_and_epilogue_insns (void
 	  note_stores (PATTERN (p_insn), record_hard_reg_sets,
 		       &prologue_clobbered);
 	}
-      for (p_insn = split_prologue_seq; p_insn; p_insn = NEXT_INSN (p_insn))
-	if (NONDEBUG_INSN_P (p_insn))
-	  note_stores (PATTERN (p_insn), record_hard_reg_sets,
-		       &prologue_clobbered);
 
       bitmap_initialize (&bb_antic_flags, &bitmap_default_obstack);
       bitmap_initialize (&bb_on_list, &bitmap_default_obstack);
@@ -5758,7 +5754,7 @@  thread_prologue_and_epilogue_insns (void
 
   if (split_prologue_seq != NULL_RTX)
     {
-      insert_insn_on_edge (split_prologue_seq, entry_edge);
+      insert_insn_on_edge (split_prologue_seq, orig_entry_edge);
       inserted = true;
     }
   if (prologue_seq != NULL_RTX)