Patchwork Fix PR rtl-optimization/51187

login
register
mail settings
Submitter Eric Botcazou
Date Nov. 19, 2011, 8:41 p.m.
Message ID <201111192141.05177.ebotcazou@adacore.com>
Download mbox | patch
Permalink /patch/126614/
State New
Headers show

Comments

Eric Botcazou - Nov. 19, 2011, 8:41 p.m.
This is the miscompilation of the cross-compiler targetting AVR by the native 
compiler on the SPARC at -O2, a latent problem in reorg.c that is exposed in 
the 4.5.x (and later) series by the introduction of __builtin_unreachable.

relax_delay_slots has code that tries to detect if a simple (conditional) jump 
is useless, i.e. if there is no active insn between the jump and the label.
If so, it first re-emits the insns in the delay slot(s) of the jump and then 
invokes delete_related_insns on the jump.

delete_related_insns first deletes the jump and then, if the number of uses of 
the label has reached zero, calls itself recursively on the label.  Now, when 
invoked on a label, delete_related_insns not only deletes the label but also 
the entire extended basic block starting at the label if there is a barrier 
just before the label.

With __builtin_unreachable you can have barriers at somewhat unexpected places 
and the jump might be guarding the fallthrough to the barrier; deleting the 
jump in this case doesn't mean that the block at the label can be deleted.

The solution of deleting the barrier with the jump would work, but happens to 
be tricky to implement in delete_related_insns.  The attached patch disables 
the optimization instead in this case, on the grounds that it isn't the job of 
reorg.c to optimize the CFG when no other RTL pass was able to do it before.

Bootstrapped/regtested on SPARC/Linux, applied on mainline, 4.6/4.5 branches.


2011-11-19  Eric Botcazou  <ebotcazou@adacore.com>

	PR rtl-optimization/51187
	* reorg.c (relax_delay_slots): Do not consider a jump useless if there
	is a barrier between the jump and its target label.


2011-11-19  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc.dg/delay-slot-2.c: New test.
Andrew Pinski - Nov. 22, 2011, 3:16 a.m.
On Sat, Nov 19, 2011 at 12:41 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> This is the miscompilation of the cross-compiler targetting AVR by the native
> compiler on the SPARC at -O2, a latent problem in reorg.c that is exposed in
> the 4.5.x (and later) series by the introduction of __builtin_unreachable.

I saw this also when I looked into __builtin_unreachable in one bug
report at one point on MIPS.  See
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49054#c1 .

Thanks,
Andrew Pinski
Eric Botcazou - Nov. 22, 2011, 11:33 a.m.
> I saw this also when I looked into __builtin_unreachable in one bug
> report at one point on MIPS.  See
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49054#c1 .

Yes, the Debian folks also saw the problem on other architectures.

Patch

Index: reorg.c
===================================================================
--- reorg.c	(revision 181505)
+++ reorg.c	(working copy)
@@ -3600,9 +3600,11 @@  relax_delay_slots (rtx first)
 	    }
 	}
 
+      /* See if we have a simple (conditional) jump that is useless.  */
       if (! INSN_ANNULLED_BRANCH_P (delay_insn)
-	  && prev_active_insn (target_label) == insn
 	  && ! condjump_in_parallel_p (delay_insn)
+	  && prev_active_insn (target_label) == insn
+	  && ! BARRIER_P (prev_nonnote_insn (target_label))
 #ifdef HAVE_cc0
 	  /* If the last insn in the delay slot sets CC0 for some insn,
 	     various code assumes that it is in a delay slot.  We could