On Mon, Jul 16, 2012 at 09:55:30PM +0200, Steven Bosscher wrote: >On Mon, Jul 16, 2012 at 5:57 PM, Steven Bosscher <stevenb....@gmail.com> wrote: >> 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. > >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 >> * tree-cfg.c (gimple_can_merge_blocks_p): Use EDGE_COMPLEX. I take it you added EDGE_ABNORMAL_CALL on purpose? >> (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,