[4/6] Shrink-wrapping

On 07/21/11 11:52, Richard Sandiford wrote:
> The name "active_insn_after" seems a bit too similar to "next_active_insn"
> for the difference to be obvious.  How about something like
> "first_active_target_insn" instead?

Changed.
>> -  for (; insn; insn = next)
>> +  for (; insn && !ANY_RETURN_P (insn); insn = next)
>>      {
>>        if (NONJUMP_INSN_P (insn) && GET_CODE (PATTERN (insn)) == SEQUENCE)
>>  	insn = XVECEXP (PATTERN (insn), 0, 0);
> 
> Since ANY_RETURN looks for patterns, while this loop iterates over insns,
> I think it'd be more obvious to have:
> 
>   if (insn && ANY_RETURN_P (insn))
>     return 1;
> 
> above the loop instead

That alone wouldn't work since we assign JUMP_LABELs to next. Left alone
for now.

>> --- gcc/jump.c	(revision 176230)
>> +++ gcc/jump.c	(working copy)
>> @@ -1217,7 +1217,7 @@ delete_related_insns (rtx insn)
> 
> Given what you said above, and given that this is a public function,
> I think we should keep the null check.

Changed.
> 
> This pattern came up in reorg.c too, so maybe it would be worth having
> a jump_to_label_p inline function somewhere, such as:

Done. Only has two uses for now though; reorg.c uses different patterns
mostly.

> It looks like the old code tried to allow returns to be redirected
> to a label -- (return) to (set (pc) (label_ref)) -- whereas the new
> code doesn't. [...]
> 
> How about:
> 
>       x = redirect_target (nlabel);
>       if (GET_CODE (x) == LABEL_REF && loc == &PATTERN (insn))
> 	x = gen_rtx_SET (VOIDmode, pc_rtx, x);
>       validate_change (insn, loc, x, 1);

Changed, although this probably isn't a useful thing to allow (it will
just add one more unnecessary jump to the code?).

[ifcvt changes]
> I found the placement of this code a bit confusing as things stand.
> new_dest_label is only meaningful if other_bb != new_dest, so it seemed
> like something that should directly replace the existing new_label
> assignment.  It's OK if it makes the shrink-wrap stuff easier though.

Changed.

>> @@ -1195,6 +1195,9 @@ duplicate_insn_chain (rtx from, rtx to)
>>  	      break;
>>  	    }
>>  	  copy = emit_copy_of_insn_after (insn, get_last_insn ());
>> +	  if (JUMP_P (insn) && JUMP_LABEL (insn) != NULL_RTX
>> +	      && ANY_RETURN_P (JUMP_LABEL (insn)))
>> +	    JUMP_LABEL (copy) = JUMP_LABEL (insn);
> 
> I think this should go in emit_copy_of_insn_after instead.

Here I'd like to avoid modifying the existing code in
emit_copy_of_insn_after if possible. Not sure why it's not copying
JUMP_LABELS, but that's something I'd prefer to investigate at some
other time rather than risk breaking things.

>> @@ -2294,6 +2294,8 @@ create_cfi_notes (void)
>>  	  dwarf2out_frame_debug (insn, false);
>>  	  continue;
>>  	}
>> +      if (GET_CODE (pat) == ADDR_VEC || GET_CODE (pat) == ADDR_DIFF_VEC)
>> +	continue;
>>  
>>        if (GET_CODE (pat) == SEQUENCE)
>>  	{
> 
> rth better approve this bit...

It went away.

New patch below. Retested on i686-linux and mips64-elf. Ok?

Bernd
* rtlanal.c (tablejump_p): False for returns.
	* reorg.c (first_active_target_insn): New static function.
	(find_end_label): Set JUMP_LABEL for a new returnjump.
	(optimize_skip, get_jump_flags, rare_destination,
	mostly_true_jump, get_branch_condition,
	steal_delay_list_from_target, own_thread_p,
	fill_simple_delay_slots, follow_jumps, fill_slots_from_thread,
	fill_eager_delay_slots, relax_delay_slots, make_return_insns,
	dbr_schedule): Adjust to handle ret_rtx in JUMP_LABELs.
	* jump.c (delete_related_insns): Likewise.
	(jump_to_label_p): New function.
	(redirect_target): New static function.
	(redirect_exp_1): Use it.  Adjust to handle ret_rtx in JUMP_LABELS.
	(redirect_jump_1): Assert that the new label is nonnull.
	(redirect_jump): Likewise.
	(redirect_jump_2): Check for ANY_RETURN_P rather than NULL labels.
	* ifcvt.c (find_if_case_1): Take care when redirecting jumps to the
	exit block.
	(dead_or_predicable): Change NEW_DEST arg to DEST_EDGE.  All callers
	changed.  Ensure that the right label is passed to redirect_jump.
	* function.c (emit_return_into_block,
	thread_prologue_and_epilogue_insns): Ensure new returnjumps have
	ret_rtx in their JUMP_LABEL.
	* print-rtl.c (print_rtx): Handle ret_rtx in a JUMP_LABEL.
	* emit-rtl.c (skip_consecutive_labels): Allow the caller to
	pass ret_rtx as label.
	* cfglayout.c (fixup_reorder_chain): Use
	force_nonfallthru_and_redirect rather than force_nonfallthru.
	(duplicate_insn_chain): Copy JUMP_LABELs for returns.
	* rtl.h (ANY_RETURN_P): New macro.
	(jump_to_label_p): Declare.
	* resource.c (find_dead_or_set_registers): Handle ret_rtx in
	JUMP_LABELs.
	(mark_target_live_regs): Likewise.
	* basic-block.h (force_nonfallthru_and_redirect): Declare.
	* cfgrtl.c (force_nonfallthru_and_redirect): No longer static.
	* config/alpha/alpha.c (alpha_tablejump_addr_vec,
	alpha_tablejump_best_label): Remove functions.
	* config/alpha/alpha-protos.c (alpha_tablejump_addr_vec,
	alpha_tablejump_best_label): Remove declarations.
	* config/sh/sh.c (barrier_align, split_branches): Adjust for
	ret_rtx in JUMP_LABELs.
	* config/arm/arm.c (is_jump_table): Likewise.

[4/6] Shrink-wrapping

Commit Message

Comments

Patch