Patch: More basic-block reordering infrastructure.

Richard Henderson rth@cygnus.com
Sun Jan 30 22:03:00 GMT 2000


On Sat, Jan 29, 2000 at 01:02:38PM -0800, Jason Eckhardt wrote:
> + #define REORDER_BLOCK_FLAGS(b) \
> +   ((reorder_block_def) BASIC_BLOCK (b)->aux)->flags

Please move the `BASIC_BLOCK (foo)' out of these macros and into the
caller.  Examining the rest of the patch, I see that it would rarely
be needed, since you're usually doing `foo->index' as the argument.

> +   if (rob_dump_file)

Don't pass around rob_dump_file, just look at rtl_dump_file directly.

> +       char *cond_type_str [] = {"not cond jump", "predict then",
> + 				"predict else",
> + 				"predict then w/o else",
> + 				"predict not then w/o else"};

 const char * const cond_type_str[]

etc.

> + static void
> + make_reorder_chain (b)
> +      int b;
> + {
> +   edge e;
> +   int visited_edge = 0;
> +   basic_block bb = BASIC_BLOCK (b);

Pass in bb, not b.  That's what's getting used anyway.

> +   if (e && e->dest->index != EXIT_BLOCK
> +       && e->dest->index != e->src->index

  e->dest != EXIT_BLOCK_PTR
  && e->dest != e->src

And so on.  Generally, try to avoid looking at a block's index.

> +   for (insn = BASIC_BLOCK (i)->head;
> +        NEXT_INSN (insn) != 0;
> +        insn = NEXT_INSN (insn))
> +     { /* Empty.  */ }
> +   set_last_insn (insn);

Start from bb->end instead?  And please use `continue' instead
of `{ /* Empty.  */ }'.

> + #if 0
> +   /* Add NOTE_INSN_BLOCK_END */

What is this block for, especially since it's ifdefed out?
It appears to be trying to play with block notes, and in
a way that could never work.

> + 	      b = (basic_block) obstack_alloc (function_obstack, sizeof (*b));
> + 	      VARRAY_GROW (basic_block_info, ++n_basic_blocks);
> + 	      BASIC_BLOCK (n_basic_blocks - 1) = b;
> + 	      b->index = n_basic_blocks - 1;
> + 	      b->head = emit_note_before (NOTE_INSN_BASIC_BLOCK, jump_insn);
> + 	      NOTE_BASIC_BLOCK (b->head) = b;
> + 	      b->end = barrier_insn;

Use create_basic_block.

> + #if 0
> +   if (debug_info_level > DINFO_LEVEL_TERSE)
> +     fatal ("debug level greater than 1 not supported with -freorder-blocks");
> + #endif

Kill this.

> +   if (dump_file)
> +     {
> +       rtx insn, last_insn;
> +       last_insn = get_insns ();
> +       for (insn = NEXT_INSN (get_insns ());
> + 	   insn && PREV_INSN (insn) == last_insn
> + 	     && NEXT_INSN (PREV_INSN (insn)) == insn;
> + 	   last_insn = insn,
> + 	     insn = NEXT_INSN (insn))
> + 	{ /* Empty.  */};
> +       if (insn)
> + 	{
> + 	  warning ("insn chaining error at %d while reordering basic blocks\n",
> + 		   INSN_UID (last_insn));
> + 	  fprintf (rob_dump_file, "insn chaining error at %d\n",
> + 		   INSN_UID (last_insn));
> + 	}
> +     }

Conditionalize on ENABLE_CHECKING rather than dump_file.
Abort, don't print warnings.



r~


More information about the Gcc-patches mailing list