diff mbox

RFA: Fix rtl-optimization/58021

Message ID 20130729162406.tunrdei084808gw8-nzlynne@webmail.spamcop.net
State New
Headers show

Commit Message

Joern Rennecke July 29, 2013, 8:24 p.m. UTC 
The interesting case where we encounter a basic block head is when the
check of return_copy for BB_HEAD check succeeds with return_copy being  
a label;
then last_insn is a NOTE_INSN_BASIC_BLOCK, and we must not try to split
off a part of the basic block before that note.
That can be properly tested for by changing the !INSN_P (last_insn)  
check into a
NOTE_INSN_BASIC_BLOCK_P (last_insn) check.

last_insn == BB_HEAD (src_bb) can't actually be true, because we don't copy
return_copy to last_insn when we've bit BB_HEAD, so I removed that test.

bootstrapped / regtested on i686-pc-linux-gnu .



AFAICT, the second block split is useless, and could be replaced with
pre_exit = src_bb;
, and then the slack space allocated for post_entry / pre_exit blocks reduced
to two.  However, I didn't want to tie a bugfix with a potentialy  
destabilizing
cleanup - maybe there's some target port code (in the context of  
likely spilled
classes?) that relies on the MODE_EXIT switch note only before, but in  
a block separate from the return value copy?
2013-07-29  Joern Rennecke  <joern.rennecke@embecosm.com>

	PR rtl-optimization/58021
	* mode-switching.c (create_pre_exit): Always split of preceding
	insns if we are not at the basic block head.

Comments

Jeff Law July 29, 2013, 9:35 p.m. UTC | #1 
On 07/29/2013 02:24 PM, Joern Rennecke wrote:
> The interesting case where we encounter a basic block head is when
> the check of return_copy for BB_HEAD check succeeds with return_copy
> being a label; then last_insn is a NOTE_INSN_BASIC_BLOCK, and we must
> not try to split off a part of the basic block before that note. That
> can be properly tested for by changing the !INSN_P (last_insn) check
> into a NOTE_INSN_BASIC_BLOCK_P (last_insn) check.
>
> last_insn == BB_HEAD (src_bb) can't actually be true, because we
> don't copy return_copy to last_insn when we've bit BB_HEAD, so I
> removed that test.
>
> bootstrapped / regtested on i686-pc-linux-gnu .
OK for the trunk.

>
>
>
> AFAICT, the second block split is useless, and could be replaced
> with pre_exit = src_bb; , and then the slack space allocated for
> post_entry / pre_exit blocks reduced to two.  However, I didn't want
> to tie a bugfix with a potentialy destabilizing cleanup - maybe
> there's some target port code (in the context of likely spilled
> classes?) that relies on the MODE_EXIT switch note only before, but
> in a block separate from the return value copy?
Well, the mode switching code is only used on sh (which you obviously 
know quite well) x86 and the epiphany ports.  I trust your judgement on 
this since you've probably dealt more with the optimize_switching code 
than anyone.

jeff
diff mbox

Patch

Index: mode-switching.c
===================================================================
--- mode-switching.c	(revision 201313)
+++ mode-switching.c	(working copy)
@@ -420,7 +420,7 @@  create_pre_exit (int n_entities, int *en
 			|| (GET_MODE_CLASS (GET_MODE (ret_reg)) != MODE_INT
 			    && nregs != 1));
 
-	    if (INSN_P (last_insn))
+	    if (!NOTE_INSN_BASIC_BLOCK_P (last_insn))
 	      {
 		before_return_copy
 		  = emit_note_before (NOTE_INSN_DELETED, last_insn);
@@ -428,9 +428,8 @@  create_pre_exit (int n_entities, int *en
 		   require a different mode than MODE_EXIT, so if we might
 		   have such instructions, keep them in a separate block
 		   from pre_exit.  */
-		if (last_insn != BB_HEAD (src_bb))
-		  src_bb = split_block (src_bb,
-					PREV_INSN (before_return_copy))->dest;
+		src_bb = split_block (src_bb,
+				      PREV_INSN (before_return_copy))->dest;
 	      }
 	    else
 	      before_return_copy = last_insn;