[LLVMdev] [patch] Dotty printer for dependency trees

[Tobias Grosser]

On Wed, 2009-07-15 at 22:14 -0700, an unknown sender wrote: 
> Tobias Grosser wrote:
> > On Wed, 2009-07-15 at 00:20 +0200, Tobias Grosser wrote:
> >> Hi,
> >>
> >> I started to work with llvm and liked the CFG dotty printer a lot.
> >> However there was none for the dominance trees.
> >>
> >> I implemented dotty printing for dominance trees
> 
> Great! I'm a huge fan of graphviz integration in the compiler.
> 
> > And here is the patch. 
> 
> What's up with copying from CFGPrinter.cpp into CFGPrinter.h? You 
> shouldn't need that.

CFGPrinter.cpp implements the printing for a graph of basic blocks, in
this case for an CFG. The nodes in the case of dominance information are
also basic blocks, so I want to use the same code to print these.
As the template is defined in the *.cpp file it can not be accessed,
therefore I moved it to the .h file.

The function I am calling is:

DOTGraphTraits<const Function*>::getNodeLabel(const BasicBlock*, const
Function *, bool);

Is there any way to access the template in the *.cpp file? I do not want
to just copy the code or write my own version of basic block printing.
It would not be different from the current one.

> +  static nodes_iterator nodes_begin (DomTreeNode * N) {
> +    return df_begin<DomTreeNode *> (N);
> +  }
> +
> +  static nodes_iterator nodes_end  (DomTreeNode *N) {
> +    return df_end<DomTreeNode *> (N);
> +  }
> 
> No spaces between the function name and the parenthesis. This occurs a 
> few times.

Fixed.

> @@ -112,7 +59,7 @@ namespace {
>       CFGOnlyViewer() : FunctionPass(&ID) {}
> 
>       virtual bool runOnFunction(Function &F) {
> -      F.viewCFG();
> +      F.viewCFGOnly();
>         return false;
>       }
> 
> Put that back. viewCFG shows the CFG with full instructions inside it 
> while viewCFGOnly shows just the block without the instruction listing 
> inside. This pass is intended to show the details.

I thought the CFGViewer pass triggered on '-view-cfg' should show
details, whereas the CFGOnlyViewer triggered on '-view-cfg-only' should
show the blocks without instructions.
At the moment CFGOnlyViewer as triggered by '-view-cfg-only' shows the
instructions, that seems to be wrong.

> +  template<>
> +    struct DOTGraphTraits<DomTreeNode*> : public DefaultDOTGraphTraits {
> +      static std::string getNodeLabel(DomTreeNode *Node,
> +				      DomTreeNode *Graph,
> +				      bool ShortNames) {
> 
> Tabs are forbidden in LLVM. Also, neither "namespace llvm {" nor 
> "template<>" increase indent level.
Fixed.

> 
> +	return DOTGraphTraits<const Function*>::getNodeLabel(
> +	  (const BasicBlock*) BB, (const Function *) BB->getParent(),
> +	   ShortNames);
> +      }
> 
> If you're just changing constness, please use const_cast<>. But why 
> isn't this using DOMGraphTraits<Function*> instead?

I removed them, as they are not required. However I still have to use
the DOTGraphTraits<const Function*> template, as there does not exist
any template for DOTGraphTraits<Function*> and using
DOTGraphTraits<Function*> will not fall back to DOTGraphTraits<const
Function*>, but to the default implementation of DOTGraphTraits. That is
not what I want. As the graph printer does not modify the graph it seems
correct to define everything as 'const', so defining
DOTGraphTraits<Function*> does not seem to be a solution.

> +  struct DomViewer: public FunctionPass {
> 
> Missing space before colon.

Fixed.

Thanks for your review

Tobias
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-dot-dominance-printer.patch
Type: text/x-patch
Size: 19833 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090716/c593ff52/attachment.bin>