This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Patch: More basic-block reordering infrastructure.
- To: Jason Eckhardt <jle at cygnus dot com>
- Subject: Re: Patch: More basic-block reordering infrastructure.
- From: Richard Henderson <rth at cygnus dot com>
- Date: Sun, 30 Jan 2000 22:03:45 -0800
- Cc: gcc-patches at gcc dot gnu dot org
- References: <200001292102.NAA14270@cse.cygnus.com>
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~