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

[David A. Greene]

On Tuesday 11 August 2009 12:22, Chris Lattner wrote:

> 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:

Because the nightly tester runs with -asm-verbose and I got into enough
trouble last time.  :)

> > +++ 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.

Ok, I didn't realize all manipulators are verboten.

> > +      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.

I'm going to leave this here for now.  When empty() goes away we can fix it.

> > +          << " Inside Loop BB" << getFunctionNumber() << "_"
> > +          << Header->getNumber() << " Depth " << CurLoop-
> >
> > >getLoopDepth();
>
> This output seems like it would be very verbose and redundant.

It is, on purpose.  It makes writing tools easier.

> > +void AsmPrinter::PrintChildLoopComment(const MachineLoop *loop)
> > const {
>
> Please add a comment and turn this into a static function instead of a
> method on AsmPrinter.

Ok.

                               -Dave