Re: [PATCH, PR70185] Only finalize dot files that have been initialized

[Richard Biener <richard dot guenther at gmail dot com>]

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
>