Discussion:
[patch][rfc] Clean up CFG dumping
Steven Bosscher
2012-07-16 15:57:35 UTC
Permalink
Hello,

There are comments in basic-block.h that advise to update certain
parts of the compiler if a new edge flag or basic block flag is added:

-/* Always update the table in cfg.c dump_edge_info. */

and

- Always update the table in cfg.c dump_bb_info. */

Apparently this is not enough, because there are more places where the
BB flags are dumped. For instance, cfg.c:dump_cfg_bb_info does not
handle BB_MODIFIED, BB_VISITED, and BB_IN_TRANSACTION, and
dump_bb_info doesn't even exist in cfg.c (it's in cfgrtl.c). The flags
also aren't documented very well in the code.

Furthermore, there are multiple places where "common" (i.e. IR
agnostic) basic block information is dumped, with some functions
taking TDF_* flags and others not, some functions taking a FILE as the
first argument and others as the second argument, etc. In short:
Unnecessarily messy.

The attached patch cleans up the worst of it:

* A new file cfg-flags.def is used to define the BB_* and EDGE_*
flags. The names are the full string of the name of the flag in
uppercase, that's a change from before. I can add a separate name
field to DEF_EDGE_FLAG and DEF_BB_FLAG if necessary.

* Now that there is dumpfile.h for the TDF_* masks, it's easier to use
them everywhere. I have added one new flag to dump with the ";;"
prefix (I think it should be used in more places, but that's something
for later, perhaps).

* All basic block header/footer and edge dumping is consolidated in
dump_edge_info and dump_bb_info. This affects GIMPLE dump the most,
because it uses a different form of dumping for basic block
predecessors and successors. I expect some fall-out in the test suite
(patterns that no longer match) that I'll have to address before the
patch is ready for mainline.

* Slim RTL printing is better integrated: print_rtl_with_bb takes
flags and uses dump_rtl_slim if TDF_SLIM is passed. The same happens
in rtl_dump_bb, which always dumps non-slim RTL without this patch.

Bootstrapped on powerpc64-unknown-linux-gnu. Testing will probably
reveal some test suite pattern mismatches, and I also still want to
bootstrap&test this on x86_64.
I'd like to hear what people think of the cfg-flags.def change.

Ciao!
Steven

* dumpfile.h (TDF_COMMENT): New define.
* basic-block.h (EDGE_FALLTHRU, EDGE_ABNORMAL, EDGE_ABNORMAL_CALL,
EDGE_EH, EDGE_FAKE, EDGE_DFS_BACK, EDGE_CAN_FALLTHRU,
EDGE_IRREDUCIBLE_LOOP, EDGE_SIBCALL, EDGE_LOOP_EXIT, EDGE_TRUE_VALUE,
EDGE_FALSE_VALUE, EDGE_EXECUTABLE, EDGE_CROSSING, EDGE_PRESERVE):
Move to new file cfg-flags.h.
(enum cfg_edge_flags): New enum, using cfg-flags.h.
(EDGE_ALL_FLAGS): Compute value automatically.
(BB_NEW, BB_REACHABLE, BB_IRREDUCIBLE_LOOP, BB_SUPERBLOCK,
BB_DISABLE_SCHEDULE, BB_HOT_PARTITION, BB_COLD_PARTITION,
BB_DUPLICATED, BB_NON_LOCAL_GOTO_TARGET, BB_RTL,
BB_FORWARDER_BLOCK, BB_NONTHREADABLE_BLOCK, BB_MODIFIED, BB_VISITED,
BB_IN_TRANSACTION): Move to new file cfg-flags.h.
(enum bb_flags): Rename to cfg_bb_flags. Use cfg-flags.h.
(BB_ALL_FLAGS): New, compute value automatically.
(dump_bb_info): Update prototype.
(dump_edge_info): Update prototype.
* cfg-flags.h: New file.
* cfg.c (dump_edge_info): Take flags argument. Be verbose only if
TDF_DETAILS and not TDF_SLIM. Include cfg-flags.h for bitnames.
Check that the edge flags are within the range of EDGE_ALL_FLAGS.
(debug_bb): Update dump_bb call.
(dump_cfg_bb_info): Remove.
(dump_bb_info): New function. Use cfg-flags.h for bitnames.
Adjust verbosity using TDF_* flags. Check that the basic block flags
are within the range of BB_ALL_FLAGS.
(brief_dump_cfg): Use dump_bb_info instead of dump_cfg_bb_info.
* cfghooks.h (struct cfghooks): Update dump_bb hook, take a FILE
first for consistency with other dump functions.
(dump_bb): Update prototype accordingly.
* cfghooks.c: Include dumpfile.h.
(verify_flow_info): Update dump_edge_info calls.
(dump_bb): Take a flags argument and pass it around.
Use dump_bb_info to dump common information about a basic block.
(dump_flow_info): Moved here from cfgrtl.c. Make IL agnostic.
(debug_flow_info): Moved here from cfgrtl.c.
* profile.c (is_edge_inconsistent): Update dump_bb calls.
* loop-invariant.c (find_defs): Update print_rtl_with_bb call.
* rtl.h (debug_bb_n_slim, debug_bb_slim, print_rtl_slim,
print_rtl_slim_with_bb): Remove prototypes.
(dump_insn_slim): Adjust prototype to take a const_rtx.
(print_rtl_with_bb): Adjust prototype.
* sched-rgn.c (debug_region): Use dump_bb instead of debug_bb_n_slim.
* sched-vis.c (dump_insn_slim): Take a const_rtx.
(debug_insn_slim): Prototype here near DEBUG_FUNCTION marker.
(print_rtl_slim_with_bb): Remove.
(print_rtl_slim): Rename to debug_rtl_slim. Print only insn info,
not basic block info (print_rtl_with_bb with TDF_SLIM should be used
for that. Prototype here near DEBUG_FUNCTION marker.
(debug_bb_slim): Prototype here near DEBUG_FUNCTION marker.
Use dump_bb.
(debug_bb_n_slim): Prototype here near DEBUG_FUNCTION marker.
* tree-cfg.c (gimple_can_merge_blocks_p): Use EDGE_COMPLEX.
(remove_bb): Update dump_bb call.
(gimple_debug_bb): Use dump_bb.
(dump_function_to_file): Update gimple_dump_bb call.
(print_loops_bb): Likewise.
* tree-flow.h (gimple_dump_bb): Update prototype.
* gimple-pretty-print.c (dump_bb_header): Rename to
dump_gimple_bb_header. Write to a stream instead of a pretty
printer. Use dump_bb_info to dump basic block info.
(dump_bb_end): Rename to dump_gimple_bb_footer. Write to a
stream instead of a pretty printer. Use dump_bb_info.
(gimple_dump_bb_buff): Do not call dump_bb_header and dump_bb_end.
(gimple_dump_bb): Do it here with dump_gimple_bb_header and
dump_gimple_bb_footer.
* cfgrtl.c (rtl_dump_bb): Update prototype. Only dump DF if the
dump flags have TDF_DETAILS. Use dump_insn_slim if TDF_SLIM.
(print_rtl_with_bb): Take a flags argument and pass it around.
Use dump_insn_slim if TDF_SLIM.
(dump_bb_info): Removed and re-incarnated in cfg.c.
(dump_flow_info): Moved to cfghooks.c.
(debug_flow_info): Moved to cfghooks.c.
* passes.c (execute_function_dump): Unconditionally use
print_rtl_with_bb for RTL dumps, now that it understands TDF_SLIM.
* final.c (dump_basic_block_info): Update dump_edge_info calls.
* tree-vrp.c (dump_asserts_for): Likewise.
* ifcvt.c (if_convert): Unconditionally use print_rtl_with_bb.
* tree-if-conv.c (if_convertible_bb_p): Don't look at
EDGE_ABNORMAL_CALL, it has no meaning in the GIMPLE world.
* trans-mem.c (make_tm_edge): Don't set EDGE_ABNORMAL_CALL,
for the same reason.
* config/rl78/rl78.c (rl78_reorg): Update print_rtl_with_bb calls.
Steven Bosscher
2012-07-16 19:55:30 UTC
Permalink
Post by Steven Bosscher
Hello,
There are comments in basic-block.h that advise to update certain
-/* Always update the table in cfg.c dump_edge_info. */
and
- Always update the table in cfg.c dump_bb_info. */
Apparently this is not enough, because there are more places where the
BB flags are dumped. For instance, cfg.c:dump_cfg_bb_info does not
handle BB_MODIFIED, BB_VISITED, and BB_IN_TRANSACTION, and
dump_bb_info doesn't even exist in cfg.c (it's in cfgrtl.c). The flags
also aren't documented very well in the code.
Furthermore, there are multiple places where "common" (i.e. IR
agnostic) basic block information is dumped, with some functions
taking TDF_* flags and others not, some functions taking a FILE as the
Unnecessarily messy.
* A new file cfg-flags.def is used to define the BB_* and EDGE_*
flags. The names are the full string of the name of the flag in
uppercase, that's a change from before. I can add a separate name
field to DEF_EDGE_FLAG and DEF_BB_FLAG if necessary.
* Now that there is dumpfile.h for the TDF_* masks, it's easier to use
them everywhere. I have added one new flag to dump with the ";;"
prefix (I think it should be used in more places, but that's something
for later, perhaps).
* All basic block header/footer and edge dumping is consolidated in
dump_edge_info and dump_bb_info. This affects GIMPLE dump the most,
because it uses a different form of dumping for basic block
predecessors and successors. I expect some fall-out in the test suite
(patterns that no longer match) that I'll have to address before the
patch is ready for mainline.
* Slim RTL printing is better integrated: print_rtl_with_bb takes
flags and uses dump_rtl_slim if TDF_SLIM is passed. The same happens
in rtl_dump_bb, which always dumps non-slim RTL without this patch.
Bootstrapped on powerpc64-unknown-linux-gnu. Testing will probably
reveal some test suite pattern mismatches, and I also still want to
bootstrap&test this on x86_64.
I'd like to hear what people think of the cfg-flags.def change.
As it turns out, the test suite passes without new regressions on
powerpc64-unknown-linux-gnu and on x86_64-unknown-linux-gnu.
Apparently there aren't any patterns checking edge or bb info. Good
for me :-)

OK for trunk?

Ciao!
Steven
Post by Steven Bosscher
* dumpfile.h (TDF_COMMENT): New define.
* basic-block.h (EDGE_FALLTHRU, EDGE_ABNORMAL, EDGE_ABNORMAL_CALL,
EDGE_EH, EDGE_FAKE, EDGE_DFS_BACK, EDGE_CAN_FALLTHRU,
EDGE_IRREDUCIBLE_LOOP, EDGE_SIBCALL, EDGE_LOOP_EXIT, EDGE_TRUE_VALUE,
Move to new file cfg-flags.h.
(enum cfg_edge_flags): New enum, using cfg-flags.h.
(EDGE_ALL_FLAGS): Compute value automatically.
(BB_NEW, BB_REACHABLE, BB_IRREDUCIBLE_LOOP, BB_SUPERBLOCK,
BB_DISABLE_SCHEDULE, BB_HOT_PARTITION, BB_COLD_PARTITION,
BB_DUPLICATED, BB_NON_LOCAL_GOTO_TARGET, BB_RTL,
BB_FORWARDER_BLOCK, BB_NONTHREADABLE_BLOCK, BB_MODIFIED, BB_VISITED,
BB_IN_TRANSACTION): Move to new file cfg-flags.h.
(enum bb_flags): Rename to cfg_bb_flags. Use cfg-flags.h.
(BB_ALL_FLAGS): New, compute value automatically.
(dump_bb_info): Update prototype.
(dump_edge_info): Update prototype.
* cfg-flags.h: New file.
* cfg.c (dump_edge_info): Take flags argument. Be verbose only if
TDF_DETAILS and not TDF_SLIM. Include cfg-flags.h for bitnames.
Check that the edge flags are within the range of EDGE_ALL_FLAGS.
(debug_bb): Update dump_bb call.
(dump_cfg_bb_info): Remove.
(dump_bb_info): New function. Use cfg-flags.h for bitnames.
Adjust verbosity using TDF_* flags. Check that the basic block flags
are within the range of BB_ALL_FLAGS.
(brief_dump_cfg): Use dump_bb_info instead of dump_cfg_bb_info.
* cfghooks.h (struct cfghooks): Update dump_bb hook, take a FILE
first for consistency with other dump functions.
(dump_bb): Update prototype accordingly.
* cfghooks.c: Include dumpfile.h.
(verify_flow_info): Update dump_edge_info calls.
(dump_bb): Take a flags argument and pass it around.
Use dump_bb_info to dump common information about a basic block.
(dump_flow_info): Moved here from cfgrtl.c. Make IL agnostic.
(debug_flow_info): Moved here from cfgrtl.c.
* profile.c (is_edge_inconsistent): Update dump_bb calls.
* loop-invariant.c (find_defs): Update print_rtl_with_bb call.
* rtl.h (debug_bb_n_slim, debug_bb_slim, print_rtl_slim,
print_rtl_slim_with_bb): Remove prototypes.
(dump_insn_slim): Adjust prototype to take a const_rtx.
(print_rtl_with_bb): Adjust prototype.
* sched-rgn.c (debug_region): Use dump_bb instead of debug_bb_n_slim.
* sched-vis.c (dump_insn_slim): Take a const_rtx.
(debug_insn_slim): Prototype here near DEBUG_FUNCTION marker.
(print_rtl_slim_with_bb): Remove.
(print_rtl_slim): Rename to debug_rtl_slim. Print only insn info,
not basic block info (print_rtl_with_bb with TDF_SLIM should be used
for that. Prototype here near DEBUG_FUNCTION marker.
(debug_bb_slim): Prototype here near DEBUG_FUNCTION marker.
Use dump_bb.
(debug_bb_n_slim): Prototype here near DEBUG_FUNCTION marker.
* tree-cfg.c (gimple_can_merge_blocks_p): Use EDGE_COMPLEX.
(remove_bb): Update dump_bb call.
(gimple_debug_bb): Use dump_bb.
(dump_function_to_file): Update gimple_dump_bb call.
(print_loops_bb): Likewise.
* tree-flow.h (gimple_dump_bb): Update prototype.
* gimple-pretty-print.c (dump_bb_header): Rename to
dump_gimple_bb_header. Write to a stream instead of a pretty
printer. Use dump_bb_info to dump basic block info.
(dump_bb_end): Rename to dump_gimple_bb_footer. Write to a
stream instead of a pretty printer. Use dump_bb_info.
(gimple_dump_bb_buff): Do not call dump_bb_header and dump_bb_end.
(gimple_dump_bb): Do it here with dump_gimple_bb_header and
dump_gimple_bb_footer.
* cfgrtl.c (rtl_dump_bb): Update prototype. Only dump DF if the
dump flags have TDF_DETAILS. Use dump_insn_slim if TDF_SLIM.
(print_rtl_with_bb): Take a flags argument and pass it around.
Use dump_insn_slim if TDF_SLIM.
(dump_bb_info): Removed and re-incarnated in cfg.c.
(dump_flow_info): Moved to cfghooks.c.
(debug_flow_info): Moved to cfghooks.c.
* passes.c (execute_function_dump): Unconditionally use
print_rtl_with_bb for RTL dumps, now that it understands TDF_SLIM.
* final.c (dump_basic_block_info): Update dump_edge_info calls.
* tree-vrp.c (dump_asserts_for): Likewise.
* ifcvt.c (if_convert): Unconditionally use print_rtl_with_bb.
* tree-if-conv.c (if_convertible_bb_p): Don't look at
EDGE_ABNORMAL_CALL, it has no meaning in the GIMPLE world.
* trans-mem.c (make_tm_edge): Don't set EDGE_ABNORMAL_CALL,
for the same reason.
* config/rl78/rl78.c (rl78_reorg): Update print_rtl_with_bb calls.
Bernhard Reutner-Fischer
2012-07-17 07:40:15 UTC
Permalink
Post by Steven Bosscher
Post by Steven Bosscher
Hello,
There are comments in basic-block.h that advise to update certain
-/* Always update the table in cfg.c dump_edge_info. */
and
- Always update the table in cfg.c dump_bb_info. */
Apparently this is not enough, because there are more places where the
BB flags are dumped. For instance, cfg.c:dump_cfg_bb_info does not
handle BB_MODIFIED, BB_VISITED, and BB_IN_TRANSACTION, and
dump_bb_info doesn't even exist in cfg.c (it's in cfgrtl.c). The flags
also aren't documented very well in the code.
Furthermore, there are multiple places where "common" (i.e. IR
agnostic) basic block information is dumped, with some functions
taking TDF_* flags and others not, some functions taking a FILE as the
Unnecessarily messy.
* A new file cfg-flags.def is used to define the BB_* and EDGE_*
flags. The names are the full string of the name of the flag in
uppercase, that's a change from before. I can add a separate name
field to DEF_EDGE_FLAG and DEF_BB_FLAG if necessary.
* Now that there is dumpfile.h for the TDF_* masks, it's easier to use
them everywhere. I have added one new flag to dump with the ";;"
prefix (I think it should be used in more places, but that's something
for later, perhaps).
* All basic block header/footer and edge dumping is consolidated in
dump_edge_info and dump_bb_info. This affects GIMPLE dump the most,
because it uses a different form of dumping for basic block
predecessors and successors. I expect some fall-out in the test suite
(patterns that no longer match) that I'll have to address before the
patch is ready for mainline.
* Slim RTL printing is better integrated: print_rtl_with_bb takes
flags and uses dump_rtl_slim if TDF_SLIM is passed. The same happens
in rtl_dump_bb, which always dumps non-slim RTL without this patch.
Bootstrapped on powerpc64-unknown-linux-gnu. Testing will probably
reveal some test suite pattern mismatches, and I also still want to
bootstrap&test this on x86_64.
I'd like to hear what people think of the cfg-flags.def change.
As it turns out, the test suite passes without new regressions on
powerpc64-unknown-linux-gnu and on x86_64-unknown-linux-gnu.
Apparently there aren't any patterns checking edge or bb info. Good
for me :-)
s/anem/name/g
Post by Steven Bosscher
Post by Steven Bosscher
* tree-cfg.c (gimple_can_merge_blocks_p): Use EDGE_COMPLEX.
I take it you added EDGE_ABNORMAL_CALL on purpose?
Post by Steven Bosscher
Post by Steven Bosscher
(dump_bb_info): Removed and re-incarnated in cfg.c.
+ if (flags & TDF_COMMENT)
+ fputs (";; ", outf);

This emits an ugly trailing space, perhaps we can remove this now?

+ fprintf (outf, "%s prev block ", s_indent);
+ if (bb->prev_bb)
+ fprintf (outf, "%d, ", bb->prev_bb->index);
+ else
+ fprintf (outf, "(nil), ");
+ fprintf (outf, "next block ");
+ if (bb->next_bb)
+ fprintf (outf, "%d", bb->next_bb->index);
+
+ fputs (", flags:", outf);

This looks like it could emit oddly spaced output, think
"next block , flags:\n".
It would be nice to alway have the required spaces _before_ the
actual string in this function, imho.

cheers,
Steven Bosscher
2012-07-17 23:11:59 UTC
Permalink
On Tue, Jul 17, 2012 at 9:40 AM, Bernhard Reutner-Fischer
Post by Bernhard Reutner-Fischer
s/anem/name/g
Good catch.
Post by Bernhard Reutner-Fischer
Post by Steven Bosscher
* tree-cfg.c (gimple_can_merge_blocks_p): Use EDGE_COMPLEX.
I take it you added EDGE_ABNORMAL_CALL on purpose?
Yes. I don't think it matters in practice because such edges will
have EDGE_EH or EDGE_ABNORMAL set also, but this flag does belong to
the set.
Post by Bernhard Reutner-Fischer
Post by Steven Bosscher
(dump_bb_info): Removed and re-incarnated in cfg.c.
+ if (flags & TDF_COMMENT)
+ fputs (";; ", outf);
This emits an ugly trailing space, perhaps we can remove this now?
The old code had this too:

- fprintf (outf, ";;%s basic block %d, loop depth %d, count ",

It's necessary to guarantee there's always a space between ";;" and
whatever follows next, also with ident==0.

So I've not change this.
Post by Bernhard Reutner-Fischer
+ fprintf (outf, "%s prev block ", s_indent);
+ if (bb->prev_bb)
+ fprintf (outf, "%d, ", bb->prev_bb->index);
+ else
+ fprintf (outf, "(nil), ");
+ fprintf (outf, "next block ");
+ if (bb->next_bb)
+ fprintf (outf, "%d", bb->next_bb->index);
+
+ fputs (", flags:", outf);
This looks like it could emit oddly spaced output, think
"next block , flags:\n".
+ else
+ fprintf (outf, "(nil), ");
Fixed that before commit.
Post by Bernhard Reutner-Fischer
It would be nice to alway have the required spaces _before_ the
actual string in this function, imho.
Most of the function comes from old cfgrtl.c:dump_bb_info, and bits
from old gimple-pretty-print.c:dump_bb_header. I'm not really
interested in making the dumps pretty. Just complete and correct is a
good start. Aesthetically, all dumps are ugly, but I admit that some
dumps are more ugly than others. Patches welcome ;-)

Thanks for your review!

Ciao!
Steven
Tobias Burnus
2012-07-18 07:00:08 UTC
Permalink
Steven,

I think your patch broke bootstrapping with Graphite enabled.

Tobias

PS: Possible patch, I haven't checked whether "0" makes sense or
something else should be used.

--- a/gcc/graphite-poly.c
+++ b/gcc/graphite-poly.c
@@ -675,3 +675,3 @@ print_pbb_body (FILE *file, poly_bb_p pbb, int
verbosity,
fprintf (file, "{\n");
- dump_bb (pbb_bb (pbb), file, 0);
+ dump_bb (file, pbb_bb (pbb), 0, 0);
fprintf (file, "}\n");



/projects/tob/gcc-git/gcc/gcc/graphite-poly.c: In function 'print_pbb_body':
/projects/tob/gcc-git/gcc/gcc/graphite-poly.c:676:3: warning: passing
argument 1 of 'dump_bb' from incompatible pointer type [enabled by default]
dump_bb (pbb_bb (pbb), file, 0);
^
In file included from /projects/tob/gcc-git/gcc/gcc/basic-block.h:832:0,
from /projects/tob/gcc-git/gcc/gcc/tree-flow.h:27,
from /projects/tob/gcc-git/gcc/gcc/graphite-poly.c:38:
/projects/tob/gcc-git/gcc/gcc/cfghooks.h:144:13: note: expected 'struct
FILE *' but argument is of type 'basic_block'
extern void dump_bb (FILE *, basic_block, int, int);
^
/projects/tob/gcc-git/gcc/gcc/graphite-poly.c:676:3: warning: passing
argument 2 of 'dump_bb' from incompatible pointer type [enabled by default]
dump_bb (pbb_bb (pbb), file, 0);
^
In file included from /projects/tob/gcc-git/gcc/gcc/basic-block.h:832:0,
from /projects/tob/gcc-git/gcc/gcc/tree-flow.h:27,
from /projects/tob/gcc-git/gcc/gcc/graphite-poly.c:38:
/projects/tob/gcc-git/gcc/gcc/cfghooks.h:144:13: note: expected
'basic_block' but argument is of type 'struct FILE *'
extern void dump_bb (FILE *, basic_block, int, int);
^
/projects/tob/gcc-git/gcc/gcc/graphite-poly.c:676:3: error: too few
arguments to function 'dump_bb'
dump_bb (pbb_bb (pbb), file, 0);
^
In file included from /projects/tob/gcc-git/gcc/gcc/basic-block.h:832:0,
from /projects/tob/gcc-git/gcc/gcc/tree-flow.h:27,
from /projects/tob/gcc-git/gcc/gcc/graphite-poly.c:38:
/projects/tob/gcc-git/gcc/gcc/cfghooks.h:144:13: note: declared here
extern void dump_bb (FILE *, basic_block, int, int);
^
make[3]: *** [graphite-poly.o] Error 1
Steven Bosscher
2012-07-18 08:10:07 UTC
Permalink
Post by Tobias Burnus
Steven,
I think your patch broke bootstrapping with Graphite enabled.
Yes it did. That's twice in one week, because Graphite isn't enabled
for builds on the compile farm. I'll see if I can install the
necessary libraries on the machines I use to avoid breaking Graphite
in the future.
Post by Tobias Burnus
Tobias
PS: Possible patch, I haven't checked whether "0" makes sense or something
else should be used.
I thought this morning that 0 is fine, and I've committed the same
patch as yours a few minutes ago.

Ciao!
Steven

Richard Guenther
2012-07-17 09:37:23 UTC
Permalink
Post by Steven Bosscher
Post by Steven Bosscher
Hello,
There are comments in basic-block.h that advise to update certain
-/* Always update the table in cfg.c dump_edge_info. */
and
- Always update the table in cfg.c dump_bb_info. */
Apparently this is not enough, because there are more places where the
BB flags are dumped. For instance, cfg.c:dump_cfg_bb_info does not
handle BB_MODIFIED, BB_VISITED, and BB_IN_TRANSACTION, and
dump_bb_info doesn't even exist in cfg.c (it's in cfgrtl.c). The flags
also aren't documented very well in the code.
Furthermore, there are multiple places where "common" (i.e. IR
agnostic) basic block information is dumped, with some functions
taking TDF_* flags and others not, some functions taking a FILE as the
Unnecessarily messy.
* A new file cfg-flags.def is used to define the BB_* and EDGE_*
flags. The names are the full string of the name of the flag in
uppercase, that's a change from before. I can add a separate name
field to DEF_EDGE_FLAG and DEF_BB_FLAG if necessary.
* Now that there is dumpfile.h for the TDF_* masks, it's easier to use
them everywhere. I have added one new flag to dump with the ";;"
prefix (I think it should be used in more places, but that's something
for later, perhaps).
* All basic block header/footer and edge dumping is consolidated in
dump_edge_info and dump_bb_info. This affects GIMPLE dump the most,
because it uses a different form of dumping for basic block
predecessors and successors. I expect some fall-out in the test suite
(patterns that no longer match) that I'll have to address before the
patch is ready for mainline.
* Slim RTL printing is better integrated: print_rtl_with_bb takes
flags and uses dump_rtl_slim if TDF_SLIM is passed. The same happens
in rtl_dump_bb, which always dumps non-slim RTL without this patch.
Bootstrapped on powerpc64-unknown-linux-gnu. Testing will probably
reveal some test suite pattern mismatches, and I also still want to
bootstrap&test this on x86_64.
I'd like to hear what people think of the cfg-flags.def change.
As it turns out, the test suite passes without new regressions on
powerpc64-unknown-linux-gnu and on x86_64-unknown-linux-gnu.
Apparently there aren't any patterns checking edge or bb info. Good
for me :-)
OK for trunk?
Nice.

Ok!

Thanks,
Richard.
Post by Steven Bosscher
Ciao!
Steven
Post by Steven Bosscher
* dumpfile.h (TDF_COMMENT): New define.
* basic-block.h (EDGE_FALLTHRU, EDGE_ABNORMAL, EDGE_ABNORMAL_CALL,
EDGE_EH, EDGE_FAKE, EDGE_DFS_BACK, EDGE_CAN_FALLTHRU,
EDGE_IRREDUCIBLE_LOOP, EDGE_SIBCALL, EDGE_LOOP_EXIT, EDGE_TRUE_VALUE,
Move to new file cfg-flags.h.
(enum cfg_edge_flags): New enum, using cfg-flags.h.
(EDGE_ALL_FLAGS): Compute value automatically.
(BB_NEW, BB_REACHABLE, BB_IRREDUCIBLE_LOOP, BB_SUPERBLOCK,
BB_DISABLE_SCHEDULE, BB_HOT_PARTITION, BB_COLD_PARTITION,
BB_DUPLICATED, BB_NON_LOCAL_GOTO_TARGET, BB_RTL,
BB_FORWARDER_BLOCK, BB_NONTHREADABLE_BLOCK, BB_MODIFIED, BB_VISITED,
BB_IN_TRANSACTION): Move to new file cfg-flags.h.
(enum bb_flags): Rename to cfg_bb_flags. Use cfg-flags.h.
(BB_ALL_FLAGS): New, compute value automatically.
(dump_bb_info): Update prototype.
(dump_edge_info): Update prototype.
* cfg-flags.h: New file.
* cfg.c (dump_edge_info): Take flags argument. Be verbose only if
TDF_DETAILS and not TDF_SLIM. Include cfg-flags.h for bitnames.
Check that the edge flags are within the range of EDGE_ALL_FLAGS.
(debug_bb): Update dump_bb call.
(dump_cfg_bb_info): Remove.
(dump_bb_info): New function. Use cfg-flags.h for bitnames.
Adjust verbosity using TDF_* flags. Check that the basic block flags
are within the range of BB_ALL_FLAGS.
(brief_dump_cfg): Use dump_bb_info instead of dump_cfg_bb_info.
* cfghooks.h (struct cfghooks): Update dump_bb hook, take a FILE
first for consistency with other dump functions.
(dump_bb): Update prototype accordingly.
* cfghooks.c: Include dumpfile.h.
(verify_flow_info): Update dump_edge_info calls.
(dump_bb): Take a flags argument and pass it around.
Use dump_bb_info to dump common information about a basic block.
(dump_flow_info): Moved here from cfgrtl.c. Make IL agnostic.
(debug_flow_info): Moved here from cfgrtl.c.
* profile.c (is_edge_inconsistent): Update dump_bb calls.
* loop-invariant.c (find_defs): Update print_rtl_with_bb call.
* rtl.h (debug_bb_n_slim, debug_bb_slim, print_rtl_slim,
print_rtl_slim_with_bb): Remove prototypes.
(dump_insn_slim): Adjust prototype to take a const_rtx.
(print_rtl_with_bb): Adjust prototype.
* sched-rgn.c (debug_region): Use dump_bb instead of debug_bb_n_slim.
* sched-vis.c (dump_insn_slim): Take a const_rtx.
(debug_insn_slim): Prototype here near DEBUG_FUNCTION marker.
(print_rtl_slim_with_bb): Remove.
(print_rtl_slim): Rename to debug_rtl_slim. Print only insn info,
not basic block info (print_rtl_with_bb with TDF_SLIM should be used
for that. Prototype here near DEBUG_FUNCTION marker.
(debug_bb_slim): Prototype here near DEBUG_FUNCTION marker.
Use dump_bb.
(debug_bb_n_slim): Prototype here near DEBUG_FUNCTION marker.
* tree-cfg.c (gimple_can_merge_blocks_p): Use EDGE_COMPLEX.
(remove_bb): Update dump_bb call.
(gimple_debug_bb): Use dump_bb.
(dump_function_to_file): Update gimple_dump_bb call.
(print_loops_bb): Likewise.
* tree-flow.h (gimple_dump_bb): Update prototype.
* gimple-pretty-print.c (dump_bb_header): Rename to
dump_gimple_bb_header. Write to a stream instead of a pretty
printer. Use dump_bb_info to dump basic block info.
(dump_bb_end): Rename to dump_gimple_bb_footer. Write to a
stream instead of a pretty printer. Use dump_bb_info.
(gimple_dump_bb_buff): Do not call dump_bb_header and dump_bb_end.
(gimple_dump_bb): Do it here with dump_gimple_bb_header and
dump_gimple_bb_footer.
* cfgrtl.c (rtl_dump_bb): Update prototype. Only dump DF if the
dump flags have TDF_DETAILS. Use dump_insn_slim if TDF_SLIM.
(print_rtl_with_bb): Take a flags argument and pass it around.
Use dump_insn_slim if TDF_SLIM.
(dump_bb_info): Removed and re-incarnated in cfg.c.
(dump_flow_info): Moved to cfghooks.c.
(debug_flow_info): Moved to cfghooks.c.
* passes.c (execute_function_dump): Unconditionally use
print_rtl_with_bb for RTL dumps, now that it understands TDF_SLIM.
* final.c (dump_basic_block_info): Update dump_edge_info calls.
* tree-vrp.c (dump_asserts_for): Likewise.
* ifcvt.c (if_convert): Unconditionally use print_rtl_with_bb.
* tree-if-conv.c (if_convertible_bb_p): Don't look at
EDGE_ABNORMAL_CALL, it has no meaning in the GIMPLE world.
* trans-mem.c (make_tm_edge): Don't set EDGE_ABNORMAL_CALL,
for the same reason.
* config/rl78/rl78.c (rl78_reorg): Update print_rtl_with_bb calls.
H.J. Lu
2012-07-18 00:24:48 UTC
Permalink
Post by Steven Bosscher
Hello,
There are comments in basic-block.h that advise to update certain
-/* Always update the table in cfg.c dump_edge_info. */
and
- Always update the table in cfg.c dump_bb_info. */
Apparently this is not enough, because there are more places where the
BB flags are dumped. For instance, cfg.c:dump_cfg_bb_info does not
handle BB_MODIFIED, BB_VISITED, and BB_IN_TRANSACTION, and
dump_bb_info doesn't even exist in cfg.c (it's in cfgrtl.c). The flags
also aren't documented very well in the code.
Furthermore, there are multiple places where "common" (i.e. IR
agnostic) basic block information is dumped, with some functions
taking TDF_* flags and others not, some functions taking a FILE as the
Unnecessarily messy.
* A new file cfg-flags.def is used to define the BB_* and EDGE_*
flags. The names are the full string of the name of the flag in
uppercase, that's a change from before. I can add a separate name
field to DEF_EDGE_FLAG and DEF_BB_FLAG if necessary.
* Now that there is dumpfile.h for the TDF_* masks, it's easier to use
them everywhere. I have added one new flag to dump with the ";;"
prefix (I think it should be used in more places, but that's something
for later, perhaps).
* All basic block header/footer and edge dumping is consolidated in
dump_edge_info and dump_bb_info. This affects GIMPLE dump the most,
because it uses a different form of dumping for basic block
predecessors and successors. I expect some fall-out in the test suite
(patterns that no longer match) that I'll have to address before the
patch is ready for mainline.
* Slim RTL printing is better integrated: print_rtl_with_bb takes
flags and uses dump_rtl_slim if TDF_SLIM is passed. The same happens
in rtl_dump_bb, which always dumps non-slim RTL without this patch.
Bootstrapped on powerpc64-unknown-linux-gnu. Testing will probably
reveal some test suite pattern mismatches, and I also still want to
bootstrap&test this on x86_64.
I'd like to hear what people think of the cfg-flags.def change.
Ciao!
Steven
* dumpfile.h (TDF_COMMENT): New define.
* basic-block.h (EDGE_FALLTHRU, EDGE_ABNORMAL, EDGE_ABNORMAL_CALL,
EDGE_EH, EDGE_FAKE, EDGE_DFS_BACK, EDGE_CAN_FALLTHRU,
EDGE_IRREDUCIBLE_LOOP, EDGE_SIBCALL, EDGE_LOOP_EXIT, EDGE_TRUE_VALUE,
Move to new file cfg-flags.h.
(enum cfg_edge_flags): New enum, using cfg-flags.h.
(EDGE_ALL_FLAGS): Compute value automatically.
(BB_NEW, BB_REACHABLE, BB_IRREDUCIBLE_LOOP, BB_SUPERBLOCK,
BB_DISABLE_SCHEDULE, BB_HOT_PARTITION, BB_COLD_PARTITION,
BB_DUPLICATED, BB_NON_LOCAL_GOTO_TARGET, BB_RTL,
BB_FORWARDER_BLOCK, BB_NONTHREADABLE_BLOCK, BB_MODIFIED, BB_VISITED,
BB_IN_TRANSACTION): Move to new file cfg-flags.h.
(enum bb_flags): Rename to cfg_bb_flags. Use cfg-flags.h.
(BB_ALL_FLAGS): New, compute value automatically.
(dump_bb_info): Update prototype.
(dump_edge_info): Update prototype.
* cfg-flags.h: New file.
* cfg.c (dump_edge_info): Take flags argument. Be verbose only if
TDF_DETAILS and not TDF_SLIM. Include cfg-flags.h for bitnames.
Check that the edge flags are within the range of EDGE_ALL_FLAGS.
(debug_bb): Update dump_bb call.
(dump_cfg_bb_info): Remove.
(dump_bb_info): New function. Use cfg-flags.h for bitnames.
Adjust verbosity using TDF_* flags. Check that the basic block flags
are within the range of BB_ALL_FLAGS.
(brief_dump_cfg): Use dump_bb_info instead of dump_cfg_bb_info.
* cfghooks.h (struct cfghooks): Update dump_bb hook, take a FILE
first for consistency with other dump functions.
(dump_bb): Update prototype accordingly.
* cfghooks.c: Include dumpfile.h.
(verify_flow_info): Update dump_edge_info calls.
(dump_bb): Take a flags argument and pass it around.
Use dump_bb_info to dump common information about a basic block.
(dump_flow_info): Moved here from cfgrtl.c. Make IL agnostic.
(debug_flow_info): Moved here from cfgrtl.c.
* profile.c (is_edge_inconsistent): Update dump_bb calls.
* loop-invariant.c (find_defs): Update print_rtl_with_bb call.
* rtl.h (debug_bb_n_slim, debug_bb_slim, print_rtl_slim,
print_rtl_slim_with_bb): Remove prototypes.
(dump_insn_slim): Adjust prototype to take a const_rtx.
(print_rtl_with_bb): Adjust prototype.
* sched-rgn.c (debug_region): Use dump_bb instead of debug_bb_n_slim.
* sched-vis.c (dump_insn_slim): Take a const_rtx.
(debug_insn_slim): Prototype here near DEBUG_FUNCTION marker.
(print_rtl_slim_with_bb): Remove.
(print_rtl_slim): Rename to debug_rtl_slim. Print only insn info,
not basic block info (print_rtl_with_bb with TDF_SLIM should be used
for that. Prototype here near DEBUG_FUNCTION marker.
(debug_bb_slim): Prototype here near DEBUG_FUNCTION marker.
Use dump_bb.
(debug_bb_n_slim): Prototype here near DEBUG_FUNCTION marker.
* tree-cfg.c (gimple_can_merge_blocks_p): Use EDGE_COMPLEX.
(remove_bb): Update dump_bb call.
(gimple_debug_bb): Use dump_bb.
(dump_function_to_file): Update gimple_dump_bb call.
(print_loops_bb): Likewise.
* tree-flow.h (gimple_dump_bb): Update prototype.
* gimple-pretty-print.c (dump_bb_header): Rename to
dump_gimple_bb_header. Write to a stream instead of a pretty
printer. Use dump_bb_info to dump basic block info.
(dump_bb_end): Rename to dump_gimple_bb_footer. Write to a
stream instead of a pretty printer. Use dump_bb_info.
(gimple_dump_bb_buff): Do not call dump_bb_header and dump_bb_end.
(gimple_dump_bb): Do it here with dump_gimple_bb_header and
dump_gimple_bb_footer.
* cfgrtl.c (rtl_dump_bb): Update prototype. Only dump DF if the
dump flags have TDF_DETAILS. Use dump_insn_slim if TDF_SLIM.
(print_rtl_with_bb): Take a flags argument and pass it around.
Use dump_insn_slim if TDF_SLIM.
(dump_bb_info): Removed and re-incarnated in cfg.c.
(dump_flow_info): Moved to cfghooks.c.
(debug_flow_info): Moved to cfghooks.c.
* passes.c (execute_function_dump): Unconditionally use
print_rtl_with_bb for RTL dumps, now that it understands TDF_SLIM.
* final.c (dump_basic_block_info): Update dump_edge_info calls.
* tree-vrp.c (dump_asserts_for): Likewise.
* ifcvt.c (if_convert): Unconditionally use print_rtl_with_bb.
* tree-if-conv.c (if_convertible_bb_p): Don't look at
EDGE_ABNORMAL_CALL, it has no meaning in the GIMPLE world.
* trans-mem.c (make_tm_edge): Don't set EDGE_ABNORMAL_CALL,
for the same reason.
* config/rl78/rl78.c (rl78_reorg): Update print_rtl_with_bb calls.
This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54008
--
H.J.
Steven Bosscher
2012-07-18 08:08:04 UTC
Permalink
Post by H.J. Lu
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54008
Yes, they failed in my testing, too. I must have been blind to overlook them...

I received some comments in private about the "new look" of the dumps,
that I'll be addressing with a patch later today. I'll fix these two
test cases with that patch also.

Ciao!
Steven
Loading...