This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 03/11] Handwritten part of conversion of passes to C++ classes


On Sun, 2013-07-28 at 10:37 +0200, Basile Starynkevitch wrote:
> On Fri, 2013-07-26 at 11:04 -0400, David Malcolm wrote:
> > This patch is the hand-written part of the conversion of passes from
> > C structs to C++ classes.  It does not work without the subsequent
> > autogenerated part, which is huge.
> > 
> > Given that the autogenerated part of the conversion is very large
> > (500k), for the sake of human comprehension I have kept the change as
> > two separate patches to keep the hand-written changes separate from the
> > automatically-generated ones.  I would commit these as two separate
> > changes to SVN in order to keep this readability for posterity in the
> > logs as well as at review-time.
> > 
> > This pair of patches eliminates the mutable global variables
> > representing the passes, allowing for multiple compilation contexts in
> > one process, potentially with different combinations of passes, and with
> > pass instance owning additional data.
> > 
> > It converts the hierarchy of opt_pass types into an actual C++ class
> > hierarchy, where each of:
> > 
> >   * gimple_opt_pass
> >   * rtl_opt_pass
> >   * ipa_opt_pass_d
> >   * simple_ipa_opt_pass
> > 
> > all become subclasses of opt_pass.
> [...]
> 
> This looks good to me. I suggest adding into the `opt_pass` class two
> constant fields for the approximate source location of the pass, e.g. a
> field const char* _file; and another const unsigned _lineno; initialized
> with __FILE__ and __LINE__ respectively.
> 
> This won't cost much (we don't have zillions of instances of
> opt_pass....) and would help a lot finding where (in which source file)
> an actual pass is.
> 
> This is particularly useful for newbies writing plugins (which are
> trying to add new passes). It takes a lot of time to them to find which
> actual source file inside the compiler is implementing a given
> (existing) pass.

Thanks - I like this idea.  As a relative newbie myself, I've often
found myself hunting down the implementation of a specific pass, and it
seems like your idea is something we'd want to expose in plugins such as
MELT and the Python plugin - I wrote a script to draw a "subway map" of
GCC's passes:
https://gcc-python-plugin.readthedocs.org/en/latest/tables-of-passes.html
and it would be great to be able to add hyperlinks to the relevant
passes.

So this would be an extra (const char*) and int per pass, or per pass
instance, giving about 2 kilobytes of extra memory usage, which sounds
acceptable to me.  We could either put it in the pass_data instance, so
that this could look like this (and be purely const):

  const pass_data pass_data_vrp =
  {
    GIMPLE_PASS, /* type */
    "vrp", /* name */
    OPTGROUP_NONE, /* optinfo_flags */
    true, /* has_gate */
    true, /* has_execute */
    TV_TREE_VRP, /* tv_id */
    PROP_ssa, /* properties_required */
    0, /* properties_provided */
    0, /* properties_destroyed */
    0, /* todo_flags_start */
    ( TODO_cleanup_cfg | TODO_update_ssa
      | TODO_verify_ssa
      | TODO_verify_flow ), /* todo_flags_finish */
    __FILE__, __LINE__,
*   ^^^^^^^^^^^^^^^^^^^ New stuff here
  };

or in the opt_pass itself, looking like:

  class pass_vrp : public gimple_opt_pass
  {
  public:
    pass_vrp(gcc::context *ctxt)
      : gimple_opt_pass(pass_data_vrp, ctxt, __FILE__, __LINE__)
*                                            ^^^^^^^^^^^^^^^^^^
*                                            ^ New stuff here
    {}

A related idea occurred to me: adding links to the *documentation* of
the specific passes: right now, if you want to go from a pass to the
documentation of said pass, there doesn't seem to be a consistent URL
pattern.  For example, tree-vrp.c defines "pass_vrp", with name "vrp",
but the documentation is in 
  @item Value range propagation
within passes.texi
In the HTML build of the documentation, this is documented on this page:
http://gcc.gnu.org/onlinedocs/gccint/Tree-SSA-passes.html#Tree-SSA-passes
though the generated HTML appears to have no per-pass anchors; all I see
is:
  <li>Value range propagation
followed by a <p> tag (it's also not-well-formed as XHTML, but that's a
whole other issue).
So it would be great if 
(a) the pass docs had per-pass anchors and
(b) the runtime pass metadata contained enough information so that we
could generate URLs to the docs, so that e.g. you could ask for docs at
the command-line and get sane URLs.

It may be possible to achieve (b) by making the anchors in (a) be the
pass names.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]