This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch][rfc] Clean up CFG dumping
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.gcc@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,