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.