Re: CFG review needed for fix of PowerPC shrink-wrap support 3 of 3
On 11/14/2011 11:56 AM, Alan Modra wrote: * function.c (thread_prologue_and_epilogue_insns): Guard emitting return with single_succ_p test. Ok. r~
CFG review needed for fix of PowerPC shrink-wrap support 3 of 3
From: Bernd Schmidt ber...@codesourcery.com Date: Mon, 14 Nov 2011 10:51:56 +0100 On 11/11/11 20:13, Hans-Peter Nilsson wrote: AFAICT, your patch has got sufficiently testing now (on three targets to boot) to be considered safe to check in. Or is something amiss? (If it's the unchecked code quality you mentioned, that can be just as well dealt with having the tree in a working state; having the tree broken for some targets accomplishes nothing.) I briefly looked at that, and while it does tend to move stuff around compared to an unpatched compiler, the effects don't seem to be all that bad. If we find a testcase where it's a real problem, we could do more aggressive reordering after epilogue generation, when we no longer have exit fallthroguh edges. So I'd like to ask for an OK here. Looks like all we need is a positive review of http://gcc.gnu.org/ml/gcc-patches/2011-11/msg01409.html and a ChangeLog entry to unbreak three or more targets. Someone with approval rights: pretty please? brgds, H-P
Re: CFG review needed for fix of PowerPC shrink-wrap support 3 of 3
Someone with approval rights: pretty please? Can I add my +1 pretty please as well here :) ? According to #c3 this fixes arm-linux-gnueabi cross-builds for C++ as well and potentially allows this to bootstrap again. I have kicked off a bootstrap and test run on arm-linux-gnueabi . cheers Ramana
Re: CFG review needed for fix of PowerPC shrink-wrap support 3 of 3
Ramana Radhakrishnan ramana.radhakrish...@linaro.org writes: Someone with approval rights: pretty please? Can I add my +1 pretty please as well here :) ? According to #c3 this fixes arm-linux-gnueabi cross-builds for C++ as well and potentially allows this to bootstrap again. I have kicked off a bootstrap and test run on arm-linux-gnueabi . It's better than before, but Ada bootstrap (on Solaris/SPARC, cf. PR bootstrap/51086) still fails with the same ICE. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: CFG review needed for fix of PowerPC shrink-wrap support 3 of 3
On 11/14/2011 04:10 AM, Hans-Peter Nilsson wrote: Looks like all we need is a positive review of http://gcc.gnu.org/ml/gcc-patches/2011-11/msg01409.html and a ChangeLog entry to unbreak three or more targets. Someone with approval rights: pretty please? That patch is ok. r~
Re: CFG review needed for fix of PowerPC shrink-wrap support 3 of 3
On Mon, Nov 14, 2011 at 07:48:03AM -1000, Richard Henderson wrote:
On 11/14/2011 04:10 AM, Hans-Peter Nilsson wrote:
Looks like all we need is a positive review of
http://gcc.gnu.org/ml/gcc-patches/2011-11/msg01409.html and a
ChangeLog entry to unbreak three or more targets.
Someone with approval rights: pretty please?
That patch is ok.
Sorry for the bootstrap problems. I believe the cause is my
extraction of convert_jumps_to_returns and emit_return_for_exit. The
new HAVE_return code misses a single_succ_p test (covered by the
bitmap_bit_p (bb_flags, ...) test in the HAVE_simple_return case).
I haven't really looked into what Bernd's fix does. I know this one
fixes what I broke..
* function.c (thread_prologue_and_epilogue_insns): Guard
emitting return with single_succ_p test.
Index: gcc/function.c
===
--- gcc/function.c (revision 181188)
+++ gcc/function.c (working copy)
@@ -6230,7 +6230,8 @@ thread_prologue_and_epilogue_insns (void
!active_insn_between (BB_HEAD (last_bb), BB_END (last_bb)))
convert_jumps_to_returns (last_bb, false, NULL);
- if (EDGE_COUNT (exit_fallthru_edge-src-preds) != 0)
+ if (EDGE_COUNT (last_bb-preds) != 0
+ single_succ_p (last_bb))
{
last_bb = emit_return_for_exit (exit_fallthru_edge, false);
epilogue_end = returnjump = BB_END (last_bb);
--
Alan Modra
Australia Development Lab, IBM
Re: CFG review needed for fix of PowerPC shrink-wrap support 3 of 3
From: Richard Henderson r...@redhat.com Date: Mon, 14 Nov 2011 18:48:03 +0100 On 11/14/2011 04:10 AM, Hans-Peter Nilsson wrote: Looks like all we need is a positive review of http://gcc.gnu.org/ml/gcc-patches/2011-11/msg01409.html and a ChangeLog entry to unbreak three or more targets. Someone with approval rights: pretty please? That patch is ok. Committed with this ChangeLog entry, obvious from the function header and the added comment. 2011-11-15 Bernd Schmidt ber...@codesourcery.com PR rtl-optimization/51051 * cfgrtl.c (cfg_layout_can_merge_blocks_p): Return FALSE if the move would cause fallthrough into the exit block. brgds, H-P
Re: CFG review needed for fix of PowerPC shrink-wrap support 3 of 3
On 11/14/2011 11:56 AM, Alan Modra wrote: On Mon, Nov 14, 2011 at 07:48:03AM -1000, Richard Henderson wrote: On 11/14/2011 04:10 AM, Hans-Peter Nilsson wrote: Looks like all we need is a positive review of http://gcc.gnu.org/ml/gcc-patches/2011-11/msg01409.html and a ChangeLog entry to unbreak three or more targets. Someone with approval rights: pretty please? That patch is ok. Sorry for the bootstrap problems. I believe the cause is my extraction of convert_jumps_to_returns and emit_return_for_exit. The new HAVE_return code misses a single_succ_p test (covered by the bitmap_bit_p (bb_flags, ...) test in the HAVE_simple_return case). I haven't really looked into what Bernd's fix does. I know this one fixes what I broke.. * function.c (thread_prologue_and_epilogue_insns): Guard emitting return with single_succ_p test. Hmm. This looks plausible too. Bernd's patch made sure that cfglayout didn't do something impossible. Of course, it's possible that his patch should merely be an assert, and the correct fix goes here. Thoughts? r~
Re: CFG review needed for fix of PowerPC shrink-wrap support 3 of 3
On 11/15/11 01:43, Richard Henderson wrote: On 11/14/2011 11:56 AM, Alan Modra wrote: * function.c (thread_prologue_and_epilogue_insns): Guard emitting return with single_succ_p test. Hmm. This looks plausible too. Bernd's patch made sure that cfglayout didn't do something impossible. Of course, it's possible that his patch should merely be an assert, and the correct fix goes here. I haven't really looked at Alan's patch; I'm thinking there were probably two bugs - the one I found being latent before. Note that the problematic CFG exists way before thread_prologue_and_epilogue even runs, so an assert would still trigger. Bernd
Re: CFG review needed for fix of PowerPC shrink-wrap support 3 of 3
From: Bernd Schmidt ber...@codesourcery.com Date: Tue, 15 Nov 2011 01:54:34 +0100 On 11/15/11 01:43, Richard Henderson wrote: On 11/14/2011 11:56 AM, Alan Modra wrote: * function.c (thread_prologue_and_epilogue_insns): Guard emitting return with single_succ_p test. Hmm. This looks plausible too. Bernd's patch made sure that cfglayout didn't do something impossible. Of course, it's possible that his patch should merely be an assert, and the correct fix goes here. I haven't really looked at Alan's patch; I'm thinking there were probably two bugs - the one I found being latent before. Note that the problematic CFG exists way before thread_prologue_and_epilogue even runs, so an assert would still trigger. I suspect that Alan's fix here will take care of the sparc ada bootstrap regression reported, and regardless if it's correct it should be installed.
