[llvm-commits] [PATCH] Asm Loop Nest Comments
Dan Gohman
gohman at apple.com
Sat Aug 1 13:01:44 PDT 2009
Hi Dave,
This looks good! Here are some comments:
Instead of using LoopInfo and Loop, this should use
MachineLoopInfo and MachineLoop.
Would be be practical to compute loop headers just-in-time
instead of precomputing it and storing it in a map? Switching
to MachineLoop would simplify this also.
Would you mind making the default scale be 2 instead of 3?
That's what LoopInfo's print function uses, for example. But
it's not critical.
I'm a little confused about this:
+ inline Indent indent(int lvl, int amt = 3) {
+ return(Indent(lvl, amt));
+ }
This defines a function named indent which wraps a
constructor for Indent? Why not just have clients invoke
the class constructor directly?
Minor style point: return values shouldn't be enclosed in parens.
Thanks,
Dan
On Jul 30, 2009, at 3:39 PM, David Greene wrote:
> This patch adds some comments on basic blocks detailing their loop
> nesting
> information. It's rather verbose and expensive to generate so I put
> it
> under an -asm-exuberant flag.
>
> This output allows static analysis of various codegen problems and
> their
> performance implications.
>
> Feedback requested.
>
> -Dave
>
>
> Index: include/llvm/CodeGen/AsmPrinter.h
> ===================================================================
> --- include/llvm/CodeGen/AsmPrinter.h (revision 77464)
> +++ include/llvm/CodeGen/AsmPrinter.h (working copy)
> @@ -29,6 +29,8 @@
> class ConstantVector;
> class GCMetadataPrinter;
> class GlobalVariable;
> + class LoopInfo;
> + class Loop;
> class MachineConstantPoolEntry;
> class MachineConstantPoolValue;
> class MachineModuleInfo;
> @@ -60,6 +62,15 @@
> typedef gcp_map_type::iterator gcp_iterator;
> gcp_map_type GCMetadataPrinters;
>
> + LoopInfo *LI;
> +
> + typedef DenseMap<const Loop *, MachineFunction::iterator>
> + LoopHeaderMapType;
> + LoopHeaderMapType LoopHeaderMap;
> +
> + void MapLoops(MachineFunction &Fn);
> + void PrintChildLoopComment(const Loop *loop) const;
> +
> protected:
> /// MMI - If available, this is a pointer to the current
> MachineModuleInfo.
> MachineModuleInfo *MMI;
> @@ -123,6 +134,11 @@
> ///
> bool VerboseAsm;
>
> + /// ExuberantAsm - Emit many more comments in assembly output if
> + /// this is true.
> + ///
> + bool ExuberantAsm;
> +
> /// Private state for PrintSpecial()
> // Assign a unique ID to this machine instruction.
> mutable const MachineInstr *LastMI;
> @@ -226,11 +242,10 @@
> unsigned AsmVariant,
> const char *ExtraCode);
>
> -
> /// PrintGlobalVariable - Emit the specified global variable and
> its
> /// initializer to the output stream.
> virtual void PrintGlobalVariable(const GlobalVariable *GV) = 0;
> -
> +
> /// SetupMachineFunction - This should be called when a new
> MachineFunction
> /// is being processed from runOnMachineFunction.
> void SetupMachineFunction(MachineFunction &MF);
> @@ -355,6 +370,8 @@
> void EmitComments(const MachineInstr &MI) const;
> /// EmitComments - Pretty-print comments for instructions
> void EmitComments(const MCInst &MI) const;
> + /// EmitComments - Pretty-print comments for basic blocks
> + void EmitComments(const MachineBasicBlock &MBB) const;
>
> protected:
> /// EmitZeros - Emit a block of zeros.
> Index: include/llvm/Support/IOManip.h
> ===================================================================
> --- include/llvm/Support/IOManip.h (revision 0)
> +++ include/llvm/Support/IOManip.h (revision 0)
> @@ -0,0 +1,47 @@
> +//===----------------- IOManip.h - iostream manipulators ---------
> *- C++
> -*===//
> +//
> +// The LLVM Compiler Infrastructure///
> +//
> +// This file is distributed under the University of Illinois Open
> Source
> +// License. See LICENSE.TXT for details.
> +//
> +//
> =
> =
> =
> ----------------------------------------------------------------------=
> ==//
> +//
> +// Manipulators to do special-purpose formatting.
> +//
> +//
> =
> =
> =
> ----------------------------------------------------------------------=
> ==//
> +
> +namespace llvm {
> + /// Indent - Insert spaces into the character output stream. The
> + /// "level" is multiplied by the "scale" to calculate the number of
> + /// spaces to insert. "level" can represent something like loop
> + /// nesting level, for example.
> + ///
> + class Indent {
> + public:
> + explicit Indent(int lvl, int amt = 3)
> + : level(lvl), scale(amt) {}
> +
> + template<typename OStream>
> + OStream &operator()(OStream &out) const {
> + for(int i = 0; i < level*scale; ++i) {
> + out << " ";
> + }
> + return(out);
> + }
> +
> + private:
> + int level;
> + int scale;
> + };
> +
> + template<typename OStream>
> + OStream &operator<<(OStream &out, const Indent &indent)
> + {
> + return(indent(out));
> + }
> +
> + inline Indent indent(int lvl, int amt = 3) {
> + return(Indent(lvl, amt));
> + }
> +}
> Index: lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/AsmPrinter.cpp (revision 77464)
> +++ lib/CodeGen/AsmPrinter/AsmPrinter.cpp (working copy)
> @@ -22,12 +22,14 @@
> #include "llvm/CodeGen/MachineModuleInfo.h"
> #include "llvm/CodeGen/DwarfWriter.h"
> #include "llvm/Analysis/DebugInfo.h"
> +#include "llvm/Analysis/LoopInfo.h"
> #include "llvm/MC/MCContext.h"
> #include "llvm/MC/MCStreamer.h"
> #include "llvm/MC/MCInst.h"
> #include "llvm/Support/CommandLine.h"
> #include "llvm/Support/ErrorHandling.h"
> #include "llvm/Support/FormattedStream.h"
> +#include "llvm/Support/IOManip.h"
> #include "llvm/Support/Mangler.h"
> #include "llvm/Target/TargetAsmInfo.h"
> #include "llvm/Target/TargetData.h"
> @@ -45,6 +47,10 @@
> AsmVerbose("asm-verbose", cl::desc("Add comments to directives."),
> cl::init(cl::BOU_UNSET));
>
> +static cl::opt<cl::boolOrDefault>
> +AsmExuberant("asm-exuberant", cl::desc("Add many comments."),
> + cl::init(cl::BOU_FALSE));
> +
> char AsmPrinter::ID = 0;
> AsmPrinter::AsmPrinter(formatted_raw_ostream &o, TargetMachine &tm,
> const TargetAsmInfo *T, bool VDef)
> @@ -62,6 +68,11 @@
> case cl::BOU_TRUE: VerboseAsm = true; break;
> case cl::BOU_FALSE: VerboseAsm = false; break;
> }
> + switch (AsmExuberant) {
> + case cl::BOU_UNSET: ExuberantAsm = false; break;
> + case cl::BOU_TRUE: ExuberantAsm = true; break;
> + case cl::BOU_FALSE: ExuberantAsm = false; break;
> + }
> }
>
> AsmPrinter::~AsmPrinter() {
> @@ -170,6 +181,9 @@
> void AsmPrinter::getAnalysisUsage(AnalysisUsage &AU) const {
> MachineFunctionPass::getAnalysisUsage(AU);
> AU.addRequired<GCModuleInfo>();
> + if (ExuberantAsm) {
> + AU.addRequired<LoopInfo>();
> + }
> }
>
> bool AsmPrinter::doInitialization(Module &M) {
> @@ -299,6 +313,12 @@
> // What's my mangled name?
> CurrentFnName = Mang->getMangledName(MF.getFunction());
> IncrementFunctionNumber();
> +
> + if (ExuberantAsm) {
> + LI = &getAnalysis<LoopInfo>();
> + LoopHeaderMap.clear();
> + MapLoops(MF);
> + }
> }
>
> namespace {
> @@ -1626,9 +1646,9 @@
> << MBB->getNumber();
> if (printColon)
> O << ':';
> - if (printComment && MBB->getBasicBlock())
> - O << '\t' << TAI->getCommentString() << ' '
> - << MBB->getBasicBlock()->getNameStr();
> + if (printComment) {
> + EmitComments(*MBB);
> + }
> }
>
> /// printPICJumpTableSetLabel - This method prints a set label for the
> @@ -1759,7 +1779,7 @@
> if (NameString->isString()) {
> O << NameString->getAsString() << " ";
> }
> - }
> + }
> O << DLT.Line;
> if (DLT.Col != 0)
> O << ":" << DLT.Col;
> @@ -1790,3 +1810,106 @@
> }
> }
> }
> +
> +/// EmitComments - Pretty-print comments for basic blocks
> +void AsmPrinter::EmitComments(const MachineBasicBlock &MBB) const
> +{
> + O.PadToColumn(TAI->getCommentColumn(), 1);
> +
> + if (MBB.getBasicBlock())
> + O << '\t' << TAI->getCommentString() << ' '
> + << MBB.getBasicBlock()->getNameStr();
> +
> + if (ExuberantAsm) {
> + // Add loop depth information
> + const Loop *loop = LI->getLoopFor(MBB.getBasicBlock());
> + if (loop) {
> + O << "\n";
> + O.PadToColumn(TAI->getCommentColumn(), 1);
> + O << TAI->getCommentString() << " Loop Depth " << loop-
> >getLoopDepth()
> + << '\n';
> +
> + LoopHeaderMapType::iterator i = LoopHeaderMap.find(loop);
> +
> + O.PadToColumn(TAI->getCommentColumn(), 1);
> + if (i != LoopHeaderMap.end()) {
> + if (i->second == &MBB) {
> + O << TAI->getCommentString() << " Loop Header";
> + PrintChildLoopComment(loop);
> + }
> + else {
> + O << TAI->getCommentString() << " Loop Header is BB"
> + << getFunctionNumber() << "_" << i->second->getNumber();
> + }
> + }
> + else {
> + O << TAI->getCommentString() << " Unknown Loop Header BB "
> + << CurrentFnName << "_<unmapped basic block>";
> + }
> +
> + if (loop->empty()) {
> + O << '\n';
> + O.PadToColumn(TAI->getCommentColumn(), 1);
> + O << TAI->getCommentString() << " Inner Loop";
> + }
> +
> + // Add parent loop information
> + for (const Loop *CurLoop = loop->getParentLoop();
> + CurLoop;
> + CurLoop = CurLoop->getParentLoop()) {
> + LoopHeaderMapType::iterator j = LoopHeaderMap.find(CurLoop);
> +
> + O << '\n';
> + O.PadToColumn(TAI->getCommentColumn(), 1);
> + if (j != LoopHeaderMap.end()) {
> + O << TAI->getCommentString() << indent(CurLoop-
> >getLoopDepth()-1)
> + << " Inside Loop BB" << getFunctionNumber() << "_"
> + << j->second->getNumber() << " Depth " <<
> CurLoop->getLoopDepth();
> + }
> + else {
> + O << TAI->getCommentString() << indent(CurLoop-
> >getLoopDepth()-1)
> + << " Inside Loop BB " << CurrentFnName << "_<unmapped
> basic
> block>"
> + << " Depth " << CurLoop->getLoopDepth();
> + }
> + }
> + }
> + }
> +}
> +
> +void AsmPrinter::MapLoops(MachineFunction &Fn) {
> + // Find which machine basic block is the header for each loop
> + for(MachineFunction::iterator MBBI = Fn.begin(), MBBIEnd =
> Fn.end();
> + MBBI != MBBIEnd;
> + ++MBBI) {
> + Loop *loop = LI->getLoopFor(MBBI->getBasicBlock());
> + if (loop && loop->getHeader() == MBBI->getBasicBlock()) {
> + LoopHeaderMap.insert(std::make_pair(loop, MBBI));
> + }
> + }
> +}
> +
> +void AsmPrinter::PrintChildLoopComment(const Loop *loop) const {
> + // Add child loop information
> + for(Loop::iterator cl = loop->begin(),
> + clend = loop->end();
> + cl != clend;
> + ++cl) {
> + LoopHeaderMapType::const_iterator cli = LoopHeaderMap.find(*cl);
> +
> + O << '\n';
> + O.PadToColumn(TAI->getCommentColumn(), 1);
> +
> + if (cli != LoopHeaderMap.end()) {
> + O << TAI->getCommentString() << indent((*cl)->getLoopDepth()-1)
> + << " Child Loop BB" << getFunctionNumber() << "_"
> + << cli->second->getNumber() << " Depth " << (*cl)-
> >getLoopDepth();
> + }
> + else {
> + O << TAI->getCommentString() << indent((*cl)->getLoopDepth()-1)
> + << " Child Loop BB " << CurrentFnName << "_<unmapped basic
> block>"
> + << " Depth " << (*cl)->getLoopDepth();
> + }
> +
> + PrintChildLoopComment(*cl);
> + }
> +}
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list