[llvm-commits] [PATCH] DOTGraphTraits improvements

[Tobias Grosser]

On Sun, 2009-11-29 at 12:12 -0800, Chris Lattner wrote:
> 
> On Nov 27, 2009, at 11:44 AM, Tobias Grosser wrote:
> 
> > Hi,
> > 
> > I worked on some dotty improvements. The main changes are:
> > 
> > * Better layout
> > * Allow to hide details in -only mode
> > * Do not print unneeded stuff
> > * Several small bug fixes
> > 
> > The effects can be seen on this page:
> > http://students.fim.uni-passau.de/~grosser/llvm/dotty_patch/dotty.html
> > 
> 
> 
> Looks nice!  The only thing I don't like is the removal of the T/F
> labels from the "only" CFG graph.
> 
> 
> 
> > These are the changes in detail:
> > 
> > *    Remove ":" after BB name in -view-cfg-only
> > 
> 
> 
> Ok.
> 
> > *    Small PostDominatorTree improvements
> > 
> >     * Do not SEGFAULT if tree entryNode() is NULL
> >     * Print function names in dotty printer
> > 
> 
> 
> Ok.
> 
> 
> > *    Only print edgeSourceLabels if they are not empty
> > 
> 
> 
> Ok.
> 
> > *    Do not point edge heads to source labels
> > 
> >     If no destination label is available, just point to the node
> > itself
> >     instead of pointing to some source label. Source and
> > destination   
> >     labels are not related in any way.
> > 
> 
> 
> Ok.
> 
> > *    Instantiate DefaultDOTGraphTraits
> > 
> 
> 
> Ok.
> 
> > *    Remove ShortNames from getNodeLabel
> > 
> >     Convert ShortNames to the general isSimple(). isSimple() is only
> >     called in the classes where it actually makes a difference.
> > 
> 
> 
> +  std::string getNodeLabel(const BasicBlock *Node,
> +                                  const Function *Graph) {
> 
> 
> Please line up the second line with the (
> 
> 
> +    if (isSimple())
> +      return getSimpleNodeLabel (Node, Graph);
> +    else
> +      return getCompleteNodeLabel (Node, Graph);
> +
> 
> 
> No spaces before the '(' in function calls.
> 
> 
> Otherwise, ok.
I always overlook these. Thanks for showing me these style issues.

> > *    Do not print edge source labels in -view-cfg-only &
> > -dot-cfg-only
> > 
> >     Without branch instructions the labels are not informative and
> > at
> >     the same time complicate the layout of the graph. This uses the
> > new
> >     isSimple.
> > 
> 
> I'd prefer to keep these.  I frequently look at a .ll file and a
> cfg-only graph at the same time, and the edges are really useful for
> this.
OK. I will propose another solution. It complicates graph layout. 

> Please commit the other patches, if you need commit access, please
> send me the info requested in the dev policy document off list.
I will do this.