On Tue, Oct 30, 2012 at 6:20 AM, Teresa Johnson wrote:
> Index: bb-reorder.c
> ===================================================================
> --- bb-reorder.c        (revision 192692)
> +++ bb-reorder.c        (working copy)
> @@ -2188,6 +2188,8 @@ insert_section_boundary_note (void)
>         first_partition = BB_PARTITION (bb);
>        if (BB_PARTITION (bb) != first_partition)
>         {
> +          /* There should be a barrier between text sections. */
> +          emit_barrier_after (BB_END (bb->prev_bb));

So why isn't there one? There can't be a fall-through edge from one
section to the other, so cfgrtl.c:fixup_reorder_chain should have
added a barrier here already in the code under the comment:

  /* Now add jumps and labels as needed to match the blocks new
     outgoing edges.  */

Why isn't it doing that for you?

BTW, something else I noted in cfgrtl.c:
NOTE_INSN_SWITCH_TEXT_SECTIONS shouldn't be copied in
duplicate_insn_chain, so the following is necessary for robustness:

Index: cfgrtl.c
===================================================================
--- cfgrtl.c    (revision 191819)
+++ cfgrtl.c    (working copy)
@@ -3615,7 +3615,6 @@
              break;

            case NOTE_INSN_EPILOGUE_BEG:
-           case NOTE_INSN_SWITCH_TEXT_SECTIONS:
              emit_note_copy (insn);
              break;


There can be only one! One note to rule them all! etc.


> Index: cfgrtl.c
> ===================================================================
> --- cfgrtl.c    (revision 192692)
> +++ cfgrtl.c    (working copy)
> @@ -912,7 +912,8 @@ rtl_can_merge_blocks (basic_block a, basic_block b
>       partition boundaries).  See  the comments at the top of
>       bb-reorder.c:partition_hot_cold_basic_blocks for complete details.  */
>
> -  if (BB_PARTITION (a) != BB_PARTITION (b))
> +  if (find_reg_note (BB_END (a), REG_CROSSING_JUMP, NULL_RTX)
> +      || BB_PARTITION (a) != BB_PARTITION (b))
>      return false;

My dislike for this whole scheme just continues to grow... How can
there be a REG_CROSSING_JUMP note if BB_PARTITION(a)==BB_PARTITION(b)?
That is a bug. We should not need the notes here.

As long as we have the CFG, BB_PARTITION(a)==BB_PARTITION(b) should be
the canonical way to check whether two blocks are in the same
partition, and the EDGE_CROSSING flag should be set iff an edge
crosses from one section to another. The REG_CROSSING_JUMP note should
only be used to see if a JUMP_INSN may jump to another section,
without having to check all successor edges.

Any place where we have to check the BB_PARTITION or
edge->flags&EDGE_CROSSING *and* REG_CROSSING_JUMP indicates a bug in
the partitioning updating.

Another BTW: sched-vis.c doesn't handle REG_CROSSING_JUMP notes so
that slim RTL dumping breaks. I need this patchlet to make things
work:
Index: sched-vis.c
===================================================================
--- sched-vis.c (revision 191819)
+++ sched-vis.c (working copy)
@@ -553,6 +553,11 @@
 {
   char t1[BUF_LEN], t2[BUF_LEN], t3[BUF_LEN];

+  if (! x)
+    {
+      sprintf (buf, "(nil)");
+      return;
+    }
   switch (GET_CODE (x))
     {
     case SET:

Ciao!
Steven

Reply via email to