[llvm-commits] [llvm] r78567 - in /llvm/trunk: include/llvm/CodeGen/AsmPrinter.h include/llvm/Support/IOManip.h lib/CodeGen/AsmPrinter/AsmPrinter.cpp

[Chris Lattner]

On Aug 10, 2009, at 9:38 AM, David Greene wrote:
> Author: greened
> Date: Mon Aug 10 11:38:07 2009
> New Revision: 78567
>
> URL: http://llvm.org/viewvc/llvm-project?rev=78567&view=rev
> Log:
>
> Add support for printing loop structure information in asm comments.
> This definitely slows down asm output so put it under an -asm- 
> exuberant
> flag.

Hi David,

Thanks for working on this, why not just put this under -asm-verbose?   
The whole idea is that -asm-verbose is used when we don't care about  
compile time, I don't see a need for yet-another flag.  Also:

> +++ llvm/trunk/include/llvm/Support/IOManip.h Mon Aug 10 11:38:07 2009
> @@ -0,0 +1,43 @@
> +//===----------------- IOManip.h - iostream manipulators --------- 
> *- C++ -*===//

Here we go again.  *again*, please do not add this sort of thing.  It  
is not appreciated.  As discussed several times before, I do not want  
manipulators like this.  Thanks.

> +/// EmitComments - Pretty-print comments for basic blocks
> +void AsmPrinter::EmitComments(const MachineBasicBlock &MBB) const
> +{
> +  if (ExuberantAsm) {

Please use early exit on this and "loop", per:
http://llvm.org/docs/CodingStandards.html#hl_earlyexit

> +    // Add loop depth information
> +    const MachineLoop *loop = LI->getLoopFor(&MBB);
> +
> +    if (loop) {
> +      // Print a newline after bb# annotation.
> +      O << "\n";

'\n' is a strength-reduced form of "\n".

> +      O.PadToColumn(TAI->getCommentColumn(), 1);
> +      O << TAI->getCommentString() << " Loop Depth " << loop- 
> >getLoopDepth()
> +        << '\n';
> +
> +      O.PadToColumn(TAI->getCommentColumn(), 1);
> +
> +      MachineBasicBlock *Header = loop->getHeader();
> +      assert(Header && "No header for loop");
> +
> +      if (Header == &MBB) {
> +        O << TAI->getCommentString() << " Loop Header";
> +        PrintChildLoopComment(loop);
> +      }
> +      else {
> +        O << TAI->getCommentString() << " Loop Header is BB"
> +          << getFunctionNumber() << "_" << loop->getHeader()- 
> >getNumber();
> +      }
> +
> +      if (loop->empty()) {

I don't think that loops can be empty, they have to at least have a  
header.  I don't know why MachineLoop has an empty() predicate.

> +        O << '\n';
> +        O.PadToColumn(TAI->getCommentColumn(), 1);
> +        O << TAI->getCommentString() << " Inner Loop";
> +      }
> +
> +      // Add parent loop information
> +      for (const MachineLoop *CurLoop = loop->getParentLoop();
> +           CurLoop;
> +           CurLoop = CurLoop->getParentLoop()) {
> +        MachineBasicBlock *Header = CurLoop->getHeader();
> +        assert(Header && "No header for loop");
> +
> +        O << '\n';
> +        O.PadToColumn(TAI->getCommentColumn(), 1);
> +        O << TAI->getCommentString() << Indent(CurLoop- 
> >getLoopDepth()-1)
> +          << " Inside Loop BB" << getFunctionNumber() << "_"
> +          << Header->getNumber() << " Depth " << CurLoop- 
> >getLoopDepth();

This output seems like it would be very verbose and redundant.

> +void AsmPrinter::PrintChildLoopComment(const MachineLoop *loop)  
> const {

Please add a comment and turn this into a static function instead of a  
method on AsmPrinter.

-Chris

> +  // Add child loop information
> +  for(MachineLoop::iterator cl = loop->begin(),
> +        clend = loop->end();
> +      cl != clend;
> +      ++cl) {
> +    MachineBasicBlock *Header = (*cl)->getHeader();
> +    assert(Header && "No header for loop");
> +
> +    O << '\n';
> +    O.PadToColumn(TAI->getCommentColumn(), 1);
> +
> +    O << TAI->getCommentString() << Indent((*cl)->getLoopDepth()-1)
> +      << " Child Loop BB" << getFunctionNumber() << "_"
> +      << Header->getNumber() << " Depth " << (*cl)->getLoopDepth();
> +
> +    PrintChildLoopComment(*cl);
> +  }
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits