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