On Fri, 2020-01-10 at 10:24 -0700, Jeff Law wrote: > On Wed, 2020-01-08 at 04:02 -0500, David Malcolm wrote: > > Needs review. > > > > Changed in v5: > > - update ChangeLog path > > - updated copyright years to include 2020 > > > > Changed in v4: > > - Rework includes to avoid gcc-plugin.h > > - Wrap everything with #if ENABLE_ANALYZER > > - Replace auto_client_timevar with TV_ANALYZER_SUPERGRAPH > > - Fix .dot output > > https://gcc.gnu.org/ml/gcc-patches/2019-11/msg02461.html > > - Update for move of digraph.h > > > > This patch adds a "supergraph" class that combines CFGs and > > callgraph into > > one directed graph, along with "supernode" and "superedge" classes. > > > > gcc/analyzer/ChangeLog: > > * supergraph.cc: New file. > > * supergraph.h: New file. > So in the back of my mind has always been how else can we use the > infrastructure you're designing & implementing. I'd always hoped the > supergraph in particular would prove useful and it may. But I just > realized something as I was looking at the implementation. > > It appears that you copy phis/statements from basic blocks into the > supernodes. That has important implications. For example, we don't > have mechanisms to keep the two views in sync. So if we need the > supergraph for something, we're probably going to need to rebuild it. > > I don't think that implies we need to do anything now, just an > important restriction we need to keep in mind if we want to re-use > some > of this stuff later.
Right - the analyzer pass builds a representation that's useful for it, does a bunch of things to it, then tears it down; there's no interaction with changes to the IR - but for the analyzer there doesn't need to be. > I don't see any EH handling mechansisms. I realize we haven't > focused > on C++ and thus EH hasn't been the top of our minds, but are we going > to have to handle that at some point? Fair point. The scope of the analyzer is C only right now, though I think the only place I've documented that is in the internal docs, where the Limitations section has "Only for C so far". But I want to support C++ in future releases. > > diff --git a/gcc/analyzer/supergraph.h b/gcc/analyzer/supergraph.h > > new file mode 100644 > > index 000000000000..81ea3ac08198 > > --- /dev/null > > +++ b/gcc/analyzer/supergraph.h > > @@ -0,0 +1,564 @@ > > +/* "Supergraph" classes that combine CFGs and callgraph into one > > digraph. > > + Copyright (C) 2019-2020 Free Software Foundation, Inc. > > + Contributed by David Malcolm <dmalc...@redhat.com>. > > + > > +This file is part of GCC. > > + > > +GCC is free software; you can redistribute it and/or modify it > > +under the terms of the GNU General Public License as published by > > +the Free Software Foundation; either version 3, or (at your > > option) > > +any later version. > > + > > +GCC is distributed in the hope that it will be useful, but > > +WITHOUT ANY WARRANTY; without even the implied warranty of > > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > +General Public License for more details. > > + > > +You should have received a copy of the GNU General Public License > > +along with GCC; see the file COPYING3. If not see > > +<http://www.gnu.org/licenses/>;;. */ > > + > > +#ifndef GCC_ANALYZER_SUPERGRAPH_H > > +#define GCC_ANALYZER_SUPERGRAPH_H > > + > > +#include "ordered-hash-map.h" > > +#include "options.h" > > +#include "cgraph.h" > > +#include "function.h" > > +#include "cfg.h" > > +#include "basic-block.h" > > +#include "gimple.h" > > +#include "gimple-iterator.h" > > +#include "digraph.h" > Ugh. Policy is to avoid doing this kind of thing. Instead the > includes are supposed to be in the .cc files. <rant> FWIW, I've never understood the benefit of this policy; it feels like a violation of the DRY principle, and I always find myself wondering things like "what do I need to include before I can include gimple.h?". </rant> But time is short; I'll copy and paste the includes. > This can be fixed > without going through another review cycle. > > OK with the #includes fixed. Thanks; will do. > jeff >