[PATCH] Add option for dumping to stderr (issue6190057)

[Richard Guenther]

On Wed, Sep 12, 2012 at 12:30 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Wed, Sep 12, 2012 at 10:12 AM, Sharad Singhai <singhai@google.com> wrote:
>> Thanks for your comments. Please see my responses inline.
>>
>> On Tue, Sep 11, 2012 at 1:16 PM, Xinliang David Li <davidxl@google.com> wrote:
>>> Can you resend your patch in text form (also need to resolve the
>>> latest conflicts) so that it can be commented inline?
>>
>> I tried to include inline patch earlier but my message was bounced
>> back from patches mailing list. I am trying it again.
>>
>>> Please also provide as summary a more up-to-date description of
>>> 1) Command line option syntax and semantics
>>
>> I added some documentation in the patch. Here are the relevant bits
>> from invoke.texi.
>>
>> `-fdump-tree-SWITCH-OPTIONS=FILENAME'
>>      Control the dumping at various stages of processing the
>>      intermediate language tree to a file.  The file name is generated
>>      by appending a switch-specific suffix to the source file name, and
>>      the file is created in the same directory as the output file. In
>>      case of `=FILENAME' option, the dump is output on the given file
>>      instead of the auto named dump files.
>>      ...
>>
>>     `=FILENAME'
>>           Instead of an auto named dump file, output into the given file
>>           name. The file names `stdout' and `stderr' are treated
>>           specially and are considered already open standard streams.
>>           For example,
>>
>>                gcc -O2 -ftree-vectorize -fdump-tree-vect-details=foo.dump
>>                     -fdump-tree-pre=stderr file.c
>>
>>           outputs vectorizer dump into `foo.dump', while the PRE dump
>>           is output on to `stderr'. If two conflicting dump filenames
>>           are given for the same pass, then the latter option
>>           overrides the earlier one.
>>
>> `-fopt-info-PASS'
>> `-fopt-info-PASS-OPTIONS'
>> `-fopt-info-PASS-OPTIONS=FILENAME'
>>      Controls optimization dumps from various passes. If the `-OPTIONS'
>>      form is used, OPTIONS is a list of `-' separated options which
>>      controls the details of the dump.  If OPTIONS is not specified, it
>>      defaults to `optimized'. If the FILENAME is not specified, it
>>      defaults to `stderr'. Note that the output FILENAME will be
>>      overwritten in case of multiple translation units. If a combined
>>      output from multiple the translation units is desired, `stderr'
>>      should be used instead.
>>
>>      The PASS could be one of the tree or rtl passes. The following
>>      options are available
>
> I don't like that we have -PASS here.  That makes it awfully similar
> to -fdump-PASS-OPTIONS=FILENAME.  Are we merely having
> -fopt-info because OPTIONS are "different"?
>
>>     `optimized'
>>           Print information when a particular optimization is
>>           successfully applied. It is up to the pass to decide which
>>           information is relevant. For example, the vectorizer pass
>>           prints the location of loop which got vectorized.
>>
>>     `missed'
>>           Print information about missed optimizations. Individual
>>           passes control which information to include in the output.
>>           For example,
>>
>>                gcc -O2 -ftree-vectorize -fopt-info-tree-vect-missed
>
> At least for -PASS better names should be available.  And given the
> lack of pass support for the new scheme (apart from the vectorizer)
> we should enumerate the set of -PASS values we accept, currently
> -vec only.  IMHO it does not make sense to provide -fopt-info for
> the myriad of passes we have - usually only high-level ones are interesting
> to the user?
>
>>           will print information about missed vectorization
>>           opportunities on to stderr.
>>
>>     `note'
>>           Print verbose information about optimizations, such as certain
>>           transformations, more detailed information about decisions
>>           etc.
>>
>>     `details'
>>           Print detailed information from a particular pass. This
>>           includes OPTIMIZED, MISSED, and NOTE. For example,
>>
>>                gcc -O2 -ftree-vectorize
>> -fopt-info-tree-vect-details=vect.details
>>
>>           outputs detailed optimization report from the vectorization
>>           pass into `vect.details'.
>
> Can options be chained?  like -fopt-info-vec-missed-note?
>
>> `-fopt-info-tree-all'
>> `-fopt-info-tree-all-OPTIONS'
>> `-fopt-info-tree-all-OPTIONS=FILENAME'
>>      This is similar to `-fopt-info' but instead of a single pass, it
>>      applies the dump options to all the tree passes. If the FILENAME
>>      is provided, the dump from all the passes is concatenated,
>>      otherwise the dump is output onto `stderr'. If OPTIONS is omitted,
>>      it defaults to `optimized'.
>>
>>           gcc -O3 -fopt-info-tree-all-optimized-missed=tree.optdump
>>
>>      This will output information about missed optimizations as well as
>>      optimized locations from all the tree passes into `tree.optdump'.
>
> That should be -fopt-info, thus omitting -PASS should default to enabling
> all.
>
>>> 2) New dumping APIs and semantics
>>
>> New dumping API:
>>
>> dump_kind_p ==> predicate for whether a particular type of dump is enabled
>> or not. Uses both dump_flags (TDF_*) and opt_info_flags (MSG_*).
>>
>> There are two variants, dump_xxx and dump_xxx_loc, the _loc variant is similar,
>> except it also prints the source location. The source location is
>> useful in certain
>> type of dumps, specially for -fopt-info.
>>
>> Most of these are straightforward.
>>
>> dump_gimple_stmt
>> dump_gimple_stmt_loc
>> dump_generic_expr
>> dump_generic_expr_loc
>> dump_printf
>> dump_printf_loc
>> dump_basic_block
>> dump_combine_total_stats
>>
>> dump_start/dump_finish:
>>
>> Before beginning of each pass, one needs to call dump_start ()
>> and at the end must call dump_finish (). I have already added the required
>> calls in passes.c.
>>
>> Instead of using dump_file directly, use one of the dump_* functions
>> declared in dumpfile.h. For example, dump_basic_block (...)
>> should be used for printing a basic block instead of accessing a dump stream
>> directly.
>>
>>> 3) Conversion changes
>>
>> I have converted vectorizer passes to use the new API. Here is an example
>>
>> -  if (vect_print_dump_info (REPORT_DETAILS))
>> -    fprintf (vect_dump, "=== vect_do_peeling_for_loop_bound ===");
>> +  if (dump_kind_p (MSG_OPTIMIZED_LOCATIONS))
>> +    dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, vect_location,
>> +                     "=== vect_do_peeling_for_loop_bound ===");
>>
>> Instead of calling fprintf on vect_dump directly, use
>> dump_printf_loc. This dump is done only when the
>> MSG_OPTIMIZED_LOCATIONS dump is enabled. Both -fopt-info dumps and
>> regular pass dumps are handled uniformly by treating MSG_* flags as
>> extensions of dump flags.
>>
>> Unfortunately, for vectorizer passes the change was not mechanical. I had to
>> decide for each individual dump site what type of dump was appropriate there.
>> Another added wrinkle was that the source location was printed as a side-effect.
>> Thus I had to convert some calls to use dump_*_loc variant, but not others.
>
> That was expected I guess.
>
>>> Looking at the patch briefly, I am confused with the opt-info syntax.
>>> I thought the following is desired:
>>>
>>> -fopt-info=pass-flags
>>>
>>> where pass is the pass name, and flags is one of [optimized, notes,
>>> missed].  Both pass and flags can be omitted.
>>>
>>> Is it implemented this way in your patch?
>>
>> Close to it but not quite the same way.
>> I considered this syntax and in fact implemented it. But later updated it to
>> use -fopt-info-PASS-FLAGS=filename instead, since it was consistent with the
>> -fdump-PASS-FLAGS=filename. Thus it is very easy to specify opt info
>> flags for a specific
>> pass. For example,
>>
>> -fdump-tree-pre=filename ==> dump PRE pass info
>> -fopt-info-tree-pre=filename  ==> dump PRE optimization info
>>
>> Hopefully this syntax is not confusing.
>
> See above ;)  I think the -fdump- interface is somewhat confusing, so we should
> take the chance to design something better for -fopt-info which is targeted at
> GCC users and not developers.
>
> Aside from the -fopt-info flags management the patch is ok.  To go forward
> and allow others to adjust passes besides the vectorizer -fopt-info syntax
> can be massaged as a followup.
>
> Thus, ok if it bootstraps and regtests ok.

Re-sent, shortened for the list where it hits the message limit ...  you might
want to send the vectorizer pieces separately.

Richard.