From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 43292 invoked by alias); 17 Mar 2016 09:20:40 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 43232 invoked by uid 89); 17 Mar 2016 09:20:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=1086, glob, atm X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 17 Mar 2016 09:20:34 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-02.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1agU6a-0003iI-Iu from Tom_deVries@mentor.com ; Thu, 17 Mar 2016 02:20:32 -0700 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-02.mgc.mentorg.com (137.202.0.106) with Microsoft SMTP Server id 14.3.224.2; Thu, 17 Mar 2016 09:19:31 +0000 Subject: Re: [PATCH, PR70185] Only finalize dot files that have been initialized To: Richard Biener References: <56E93C26.7070400@mentor.com> CC: GCC Patches From: Tom de Vries Message-ID: <56EA768D.8040706@mentor.com> Date: Thu, 17 Mar 2016 09:20:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------040705090000070104030701" X-SW-Source: 2016-03/txt/msg00974.txt.bz2 --------------040705090000070104030701 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2158 On 16/03/16 12:34, Richard Biener wrote: > On Wed, Mar 16, 2016 at 11:57 AM, Tom de Vries wrote: >> Hi, >> >> Atm, using fdump-tree-all-graph produces invalid dot files: >> ... >> $ rm *.c.* ; gcc test.c -O2 -S -fdump-tree-all-graph >> $ for f in *.dot; do dot -Tpdf $f -o dot.pdf; done >> Warning: test.c.006t.omplower.dot: syntax error in line 1 near '}' >> Warning: test.c.007t.lower.dot: syntax error in line 1 near '}' >> Warning: test.c.010t.eh.dot: syntax error in line 1 near '}' >> Warning: test.c.292t.statistics.dot: syntax error in line 1 near '}' >> $ cat test.c.006t.omplower.dot >> } >> $ >> ... >> These dot files are finalized, but never initialized or used. >> >> The 006/007/010 files are not used because '(fn->curr_properties & PROP_cfg) >> == 0' at the corresponding passes. >> >> And the file test.c.292t.statistics.dot is not used, because it doesn't >> belong to a single pass. >> >> The current finalization code doesn't handle these cases: >> ... >> /* Do whatever is necessary to finish printing the graphs. */ >> for (i = TDI_end; (dfi = dumps->get_dump_file_info (i)) != NULL; ++i) >> if (dumps->dump_initialized_p (i) >> && (dfi->pflags & TDF_GRAPH) != 0 >> && (name = dumps->get_dump_file_name (i)) != NULL) >> { >> finish_graph_dump_file (name); >> free (name); >> } >> ... >> >> The patch fixes this by simply testing for pass->graph_dump_initialized >> instead. >> >> [ That fix exposes the lack of initialization of graph_dump_initialized. It >> seems to be initialized for static passes, but for dynamically added passes, >> such as f.i. vzeroupper the value is uninitialized. The patch also fixes >> this. ] >> >> Bootstrapped and reg-tested on x86_64. >> >> OK for stage1? > > Seeing this I wonder if it makes more sense to move ->graph_dump_initialized > from pass to dump_file_info? Done. > Also in the above shouldn't it use > dfi->pfilename rather than dumps->get_dump_file_name (i)? That one isn't defined anymore once we get to finish_optimization_passes. OK for stage1 if bootstrap and reg-test succeeds? Thanks, - Tom --------------040705090000070104030701 Content-Type: text/x-patch; name="0001-Only-finalize-dot-files-that-have-been-initialized.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename*0="0001-Only-finalize-dot-files-that-have-been-initialized.patc"; filename*1="h" Content-length: 5160 Only finalize dot files that have been initialized 2016-03-16 Tom de Vries PR other/70185 * tree-pass.h (class opt_pass): Remove graph_dump_initialized member. * dumpfile.h (struct dump_file_info): Add graph_dump_initialized field. * dumpfile.c (dump_files): Initialize graph_dump_initialized field. * passes.c (finish_optimization_passes): Only call finish_graph_dump_file if dfi->graph_dump_initialized. (execute_function_dump, pass_init_dump_file): Use dfi->graph_dump_initialized instead of pass->graph_dump_initialized. --- gcc/dumpfile.c | 22 +++++++++++----------- gcc/dumpfile.h | 4 ++++ gcc/passes.c | 16 ++++++++++------ gcc/tree-pass.h | 5 ----- 4 files changed, 25 insertions(+), 22 deletions(-) diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c index 144e371..74522a6 100644 --- a/gcc/dumpfile.c +++ b/gcc/dumpfile.c @@ -50,29 +50,29 @@ int dump_flags; TREE_DUMP_INDEX enumeration in dumpfile.h. */ static struct dump_file_info dump_files[TDI_end] = { - {NULL, NULL, NULL, NULL, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, false}, + {NULL, NULL, NULL, NULL, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, false, false}, {".cgraph", "ipa-cgraph", NULL, NULL, NULL, NULL, NULL, TDF_IPA, - 0, 0, 0, 0, 0, false}, + 0, 0, 0, 0, 0, false, false}, {".type-inheritance", "ipa-type-inheritance", NULL, NULL, NULL, NULL, NULL, TDF_IPA, - 0, 0, 0, 0, 0, false}, + 0, 0, 0, 0, 0, false, false}, {".tu", "translation-unit", NULL, NULL, NULL, NULL, NULL, TDF_TREE, - 0, 0, 0, 0, 1, false}, + 0, 0, 0, 0, 1, false, false}, {".class", "class-hierarchy", NULL, NULL, NULL, NULL, NULL, TDF_TREE, - 0, 0, 0, 0, 2, false}, + 0, 0, 0, 0, 2, false, false}, {".original", "tree-original", NULL, NULL, NULL, NULL, NULL, TDF_TREE, - 0, 0, 0, 0, 3, false}, + 0, 0, 0, 0, 3, false, false}, {".gimple", "tree-gimple", NULL, NULL, NULL, NULL, NULL, TDF_TREE, - 0, 0, 0, 0, 4, false}, + 0, 0, 0, 0, 4, false, false}, {".nested", "tree-nested", NULL, NULL, NULL, NULL, NULL, TDF_TREE, - 0, 0, 0, 0, 5, false}, + 0, 0, 0, 0, 5, false, false}, #define FIRST_AUTO_NUMBERED_DUMP 6 {NULL, "tree-all", NULL, NULL, NULL, NULL, NULL, TDF_TREE, - 0, 0, 0, 0, 0, false}, + 0, 0, 0, 0, 0, false, false}, {NULL, "rtl-all", NULL, NULL, NULL, NULL, NULL, TDF_RTL, - 0, 0, 0, 0, 0, false}, + 0, 0, 0, 0, 0, false, false}, {NULL, "ipa-all", NULL, NULL, NULL, NULL, NULL, TDF_IPA, - 0, 0, 0, 0, 0, false}, + 0, 0, 0, 0, 0, false, false}, }; /* Define a name->number mapping for a dump flag value. */ diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h index c168cbf..3f08b16 100644 --- a/gcc/dumpfile.h +++ b/gcc/dumpfile.h @@ -120,6 +120,10 @@ struct dump_file_info bool owns_strings; /* fields "suffix", "swtch", "glob" can be const strings, or can be dynamically allocated, needing free. */ + bool graph_dump_initialized; /* When a given dump file is being initialized, + this flag is set to true if the corresponding + TDF_graph dump file has also been + initialized. */ }; /* In dumpfile.c */ diff --git a/gcc/passes.c b/gcc/passes.c index 9d90251..5a98e50 100644 --- a/gcc/passes.c +++ b/gcc/passes.c @@ -364,10 +364,9 @@ finish_optimization_passes (void) /* Do whatever is necessary to finish printing the graphs. */ for (i = TDI_end; (dfi = dumps->get_dump_file_info (i)) != NULL; ++i) - if (dumps->dump_initialized_p (i) - && (dfi->pflags & TDF_GRAPH) != 0 - && (name = dumps->get_dump_file_name (i)) != NULL) + if (dfi->graph_dump_initialized) { + name = dumps->get_dump_file_name (dfi); finish_graph_dump_file (name); free (name); } @@ -1756,10 +1755,13 @@ execute_function_dump (function *fn, void *data) if ((fn->curr_properties & PROP_cfg) && (dump_flags & TDF_GRAPH)) { - if (!pass->graph_dump_initialized) + gcc::dump_manager *dumps = g->get_dumps (); + struct dump_file_info *dfi + = dumps->get_dump_file_info (pass->static_pass_number); + if (!dfi->graph_dump_initialized) { clean_graph_dump_file (dump_file_name); - pass->graph_dump_initialized = true; + dfi->graph_dump_initialized = true; } print_graph_cfg (dump_file_name, fn); } @@ -2103,7 +2105,9 @@ pass_init_dump_file (opt_pass *pass) && cfun && (cfun->curr_properties & PROP_cfg)) { clean_graph_dump_file (dump_file_name); - pass->graph_dump_initialized = true; + struct dump_file_info *dfi + = dumps->get_dump_file_info (pass->static_pass_number); + dfi->graph_dump_initialized = true; } timevar_pop (TV_DUMP); return initializing_dump; diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h index 5f5055d..cd8c339 100644 --- a/gcc/tree-pass.h +++ b/gcc/tree-pass.h @@ -108,11 +108,6 @@ public: /* Static pass number, used as a fragment of the dump file name. */ int static_pass_number; - /* When a given dump file is being initialized, this flag is set to - true if the corresponding TDF_graph dump file has also been - initialized. */ - bool graph_dump_initialized; - protected: gcc::context *m_ctxt; }; --------------040705090000070104030701--