Fix PR 53743 and other -freorder-blocks-and-partition failures (issue6823047)
Steven Bosscher
stevenb.gcc@gmail.com
Thu May 9 22:41:00 GMT 2013
On Thu, May 9, 2013 at 11:42 PM, Diego Novillo wrote:
> On 2013-05-08 01:13 , Teresa Johnson wrote:
>> -static void
>> +void
>> emit_barrier_after_bb (basic_block bb)
>> {
>> rtx barrier = emit_barrier_after (BB_END (bb));
>> - BB_FOOTER (bb) = unlink_insn_chain (barrier, barrier);
>> + if (current_ir_type () == IR_RTL_CFGLAYOUT)
>> + BB_FOOTER (bb) = unlink_insn_chain (barrier, barrier);
>
>
> What if the current IR is not RTL? Should we fail here? It doesn't seem
> like it makes sense to call this from gimple, for instance.
It also makes no sense calling it in IR_RTL_CFGLAYOUT mode. Barriers
are meaningless in cfglayout mode.
- emit_barrier_after (BB_END (jump_block));
+ /* We might be in cfg layout mode, and if so, the following routine will
+ insert the barrier correctly. */
+ emit_barrier_after_bb (jump_block);
We're practically always in cfglayout mode, but oh well...
+ if (current_ir_type () == IR_RTL_CFGLAYOUT)
emit_barrier_after (BB_END (jump_block));
>> Index: cfgrtl.c
>> ===================================================================
>> --- cfgrtl.c (revision 198686)
>> +++ cfgrtl.c (working copy)
>> @@ -44,6 +44,7 @@ along with GCC; see the file COPYING3. If not see
>> #include "tree.h"
>> #include "hard-reg-set.h"
>> #include "basic-block.h"
>> +#include "bb-reorder.h"
>
>
> You may need to modify Makefile.in to declare this new dependency.
Eh, no. cfgrtl should not depend on bb-reorder.
And cfglayout should not depend on basic block partitioning, either,
so this change:
> /* Finalize the changes: reorder insn list according to the sequence specified
> - by aux pointers, enter compensation code, rebuild scope forest. */
> + by aux pointers, enter compensation code, rebuild scope forest. If
> + this is called when we will FINALIZE_REORDER_BLOCKS, indicate that
> + to fixup_reorder_chain so that it can insert the proper switch text
> + section notes. */
>
> void
> -cfg_layout_finalize (void)
> +cfg_layout_finalize (bool finalize_reorder_blocks)
is Just Wrong (tm).
This patch mixes up things badly from the point of
what-depends-on-what, the whole approach looks wrong to me.
Ciao!
Steven
More information about the Gcc-patches
mailing list