Patchwork PATCH: Properly check the end of basic block

login
register
mail settings
Submitter Uros Bizjak
Date Nov. 18, 2010, 10:50 p.m.
Message ID <AANLkTikEZqCrgk-Jrp36hximSqOhQ2MhZpGcKEBNpjHX@mail.gmail.com>
Download mbox | patch
Permalink /patch/72162/
State New
Headers show

Comments

Uros Bizjak - Nov. 18, 2010, 10:50 p.m.
On Thu, Nov 18, 2010 at 11:32 PM, Richard Henderson <rth@redhat.com> wrote:
> On 11/18/2010 02:15 PM, Uros Bizjak wrote:
>> Tested on x86_64-pc-linux-gnu {,-m32}. OK for mainline and release branches?
>
> Definitely not, as mentioned elsewhere.
>
> I just thought that I'd add that, for next stage1, it would be nice
> to have a targetm boolean that says whether pass_machine_reorg needs
> the cfg.  Because it does seem silly to free the cfg only to have
> to immeidately re-compute it.
>
> But that's not something to fix now.

The approach, proposed by H.J also works. IMO, the patch is OK,
perhaps with a comment like:


Uros.
Richard Henderson - Nov. 18, 2010, 11:09 p.m.
On 11/18/2010 02:50 PM, Uros Bizjak wrote:
> -	  emit_jump_insn_before (gen_return_internal_long (), ret);
> +	  /* We have to update BB_END (bb) here - delete_insn will
> +	     not do it automatically since CFG is not available in
> +	     machine_reorg pass.  */
> +	  BB_END (bb)
> +	    = emit_jump_insn_before (gen_return_internal_long (), ret);

While that by itself is fine, calling compute_bb_for_insn at the 
beginning of md_reorg will make sure that things stay up-to-date
for any other changes that are being made within that function.

I.e. compute_bb_for_insn seems safer overall.


r~

Patch

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 166920)
+++ config/i386/i386.c	(working copy)
@@ -29640,7 +29640,11 @@  ix86_pad_returns (void)
 	}
       if (replace)
 	{
-	  emit_jump_insn_before (gen_return_internal_long (), ret);
+	  /* We have to update BB_END (bb) here - delete_insn will
+	     not do it automatically since CFG is not available in
+	     machine_reorg pass.  */
+	  BB_END (bb)
+	    = emit_jump_insn_before (gen_return_internal_long (), ret);
 	  delete_insn (ret);
 	}
     }