On Fri, Mar 25, 2022 at 4:08 PM Alex Coplan via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
>
> I noticed that, while the C/C++ frontends invoke the GENERIC match.pd
> simplifications to do early folding, the debug output from
> generic-match.cc does not appear in the -fdump-tree-original output,
> even with -fdump-tree-original-folding or -fdump-tree-original-all. This
> patch fixes that.
>
> For example, before the patch, for the following code:
>
> int a[2];
> void bar ();
> void f()
> {
>     if ((unsigned long)(a + 1) == 0)
>         bar ();
> }
>
> on AArch64 at -O0, -fdump-tree-original-all would give:
>
> ;; Function f (null)
> ;; enabled by -tree-original
>
>
> {
>   if (0)
>     {
>       bar ();
>     }
> }
>
> After the patch, we get:
>
> Applying pattern match.pd:3774, generic-match.cc:24535
> Matching expression match.pd:146, generic-match.cc:23
> Applying pattern match.pd:5638, generic-match.cc:13388
>
> ;; Function f (null)
> ;; enabled by -tree-original
>
>
> {
>   if (0)
>     {
>       bar ();
>     }
> }
>
> The reason we don't get the match.pd output as it stands, is that the
> original dump is treated specially in c-opts.cc: it gets its own state
> which is independent from that used by other dump files in the compiler.
> Like most of the compiler, the generated generic-match.cc has code of
> the form:
>
>   if (dump_file && (dump_flags & TDF_FOLDING))
>     fprintf (dump_file, ...);
>
> But, as it stands, -fdump-tree-original has its own FILE * and flags in
> c-opts.cc (original_dump_{file,flags}) and never touches the global
> dump_{file,flags} (managed by dumpfile.{h,cc}). This patch adjusts the
> code in c-opts.cc to use the main dump infrastructure used by the rest
> of the compiler, instead of treating the original dump specially.
>
> We take the opportunity to make a small refactor: the code in
> c-gimplify.cc:c_genericize can, with this change, use the global dump
> infrastructure to get the original dump file and flags instead of using
> the bespoke get_dump_info function implemented in c-opts.cc. With this
> change, we remove the only use of get_dump_info, so this can be removed.
>
> Note that we also fix a leak of the original dump file in
> c_common_parse_file. I originally thought it might be possible to
> achieve this with only one static call to dump_finish () (by simply
> moving it earlier in the loop), but unfortunately the dump file is
> required to be open while c_parse_final_cleanups runs, as we (e.g.)
> perform some template instantiations here for C++, which need to appear
> in the original dump file.
>
> We adjust cgraph_node::get_create to avoid introducing noise in the
> original dump file: without this, these "Introduced new external node"
> lines start appearing in the original dump files, which breaks tests
> that do a scan-tree-dump-times on the original dump looking for a
> certain function name.
>
> Bootstrapped/regtested on aarch64-linux-gnu, OK for GCC 13?

Thanks for tackling this - the only part that I don't like is the cgraph.cc
one.   Can't we instead gate the dumping on symtab->state != PARSING
or symtab->state > CONSTRUCTION?

Richard.

> Thanks,
> Alex
>
> gcc/c-family/ChangeLog:
>
>         * c-common.h (get_dump_info): Delete.
>         * c-gimplify.cc (c_genericize): Get TDI_original dump file info
>         from the global dump_manager instead of the (now obsolete)
>         get_dump_info.
>         * c-opts.cc (original_dump_file): Delete.
>         (original_dump_flags): Delete.
>         (c_common_parse_file): Switch to using global dump_manager to
>         manage the original dump file; fix leak of dump file.
>         (get_dump_info): Delete.
>
> gcc/ChangeLog:
>
>         * cgraph.cc (cgraph_node::get_create): Don't dump if the current
>         dump file is that of -fdump-tree-original.

Reply via email to