* [PATCH, PR70185] Only finalize dot files that have been initialized
@ 2016-03-16 10:58 Tom de Vries
2016-03-16 11:34 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2016-03-16 10:58 UTC (permalink / raw)
To: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 1568 bytes --]
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?
Thanks,
- Tom
[-- Attachment #2: 0001-Only-finalize-dot-files-that-have-been-initialized.patch --]
[-- Type: text/x-patch, Size: 1437 bytes --]
Only finalize dot files that have been initialized
2016-03-16 Tom de Vries <tom@codesourcery.com>
PR other/70185
* passes.c (opt_pass::opt_pass): Add missing initialization of
graph_dump_initialized.
(finish_optimization_passes): Only call finish_graph_dump_file if
pass->graph_dump_initialized.
---
gcc/passes.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/gcc/passes.c b/gcc/passes.c
index 9d90251..5aa2b32 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -105,6 +105,7 @@ opt_pass::opt_pass (const pass_data &data, context *ctxt)
sub (NULL),
next (NULL),
static_pass_number (0),
+ graph_dump_initialized (false),
m_ctxt (ctxt)
{
}
@@ -363,14 +364,18 @@ finish_optimization_passes (void)
}
/* Do whatever is necessary to finish printing the graphs. */
+ gcc::pass_manager *passes = g->get_passes ();
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);
- }
+ {
+ opt_pass *pass = passes->get_pass_for_id (i);
+ if (pass == NULL
+ || !pass->graph_dump_initialized)
+ continue;
+
+ name = dumps->get_dump_file_name (i);
+ finish_graph_dump_file (name);
+ free (name);
+ }
timevar_pop (TV_DUMP);
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, PR70185] Only finalize dot files that have been initialized
2016-03-16 10:58 [PATCH, PR70185] Only finalize dot files that have been initialized Tom de Vries
@ 2016-03-16 11:34 ` Richard Biener
2016-03-17 9:20 ` Tom de Vries
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2016-03-16 11:34 UTC (permalink / raw)
To: Tom de Vries; +Cc: GCC Patches
On Wed, Mar 16, 2016 at 11:57 AM, Tom de Vries <Tom_deVries@mentor.com> 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? Also in the above shouldn't it use
dfi->pfilename rather than dumps->get_dump_file_name (i)?
Richard.
> Thanks,
> - Tom
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, PR70185] Only finalize dot files that have been initialized
2016-03-16 11:34 ` Richard Biener
@ 2016-03-17 9:20 ` Tom de Vries
2016-03-17 10:12 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2016-03-17 9:20 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 2158 bytes --]
On 16/03/16 12:34, Richard Biener wrote:
> On Wed, Mar 16, 2016 at 11:57 AM, Tom de Vries <Tom_deVries@mentor.com> 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
[-- Attachment #2: 0001-Only-finalize-dot-files-that-have-been-initialized.patch --]
[-- Type: text/x-patch, Size: 5160 bytes --]
Only finalize dot files that have been initialized
2016-03-16 Tom de Vries <tom@codesourcery.com>
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;
};
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, PR70185] Only finalize dot files that have been initialized
2016-03-17 9:20 ` Tom de Vries
@ 2016-03-17 10:12 ` Richard Biener
0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2016-03-17 10:12 UTC (permalink / raw)
To: Tom de Vries; +Cc: GCC Patches
On Thu, Mar 17, 2016 at 10:19 AM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 16/03/16 12:34, Richard Biener wrote:
>>
>> On Wed, Mar 16, 2016 at 11:57 AM, Tom de Vries <Tom_deVries@mentor.com>
>> 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?
Ok.
Richard.
> Thanks,
> - Tom
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-03-17 10:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-16 10:58 [PATCH, PR70185] Only finalize dot files that have been initialized Tom de Vries
2016-03-16 11:34 ` Richard Biener
2016-03-17 9:20 ` Tom de Vries
2016-03-17 10:12 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).