[LLVMdev] Profiling in LLVM Patch Followup 1

[Daniel Dunbar]

Applied as r78484, with some minor tweaks.

A few nits:
--
> --- a/include/llvm/Analysis/LoopInfo.h
> +++ b/include/llvm/Analysis/LoopInfo.h
> @@ -213,6 +213,25 @@ public:
>      return 0;
>    }
>
> +  /// getExitEdges - Return all pairs of (_inside_block_,_outside_block_).
> +  /// (Modelled after getExitingBlocks().)

Can you change this comment to be freestanding? You don't need to
explain what it was modeled after, just explain what it does.

> @@ -0,0 +1,233 @@
> +//===- ProfileEstimatorPass.cpp - LLVM Pass to estimate profile info ------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +// This file implements a concrete implementation of profiling information that
> +// estimates the profiling information in a very crude and unimaginative way.
> +//
> +//===----------------------------------------------------------------------===//

Cool!

One great thing about this is we can now test things like llvm-prof
without having to generate sampled profile data. It would be nice to
push that along, if only to have to sanity check subsequent patches.

Also, there are some obvious FIXMEs that are probably worth putting in
the code, for example it would be nice to do some estimates on the
relative frequency of invoke destinations.

> +static cl::opt<double>
> +ProfileInfoExecCount(
> +    "profile-estimator-loop-weight", cl::init(10),
> +    cl::value_desc("loop-weight"),
> +    cl::desc("Number of loop executions used for profile-estimator")
> +);

This variable should be renamed to match the option.

> +  // createProfileEstimatorPass - This function returns a Pass that estimates
> +  // profiling information using the given loop execution count.

// -> ///, for doxygen.

> +#define EDGE_ERROR(V1,V2) \
> +  DEBUG(DOUT<<"-- Edge ("<<(V1)->getName()<<","<<(V2)->getName() \
> +            <<") is not calculated, returning\n");
> +
> +#define EDGE_WEIGHT(E) \
> +  DEBUG(DOUT<<"-- Weight of Edge ("<<((E).first?(E).first->getName():"0") \
> +            <<","<<(E).second->getName()<<"):"<<getEdgeWeight(E)<<"\n");

Ok. I would just make these inline functions for readability. I did
switch the to use errs() instead of DOUT.

> +// recurseBasicBlock() - This calculates the ProfileInfo estimation for a
> +// single block and then recurses into the successors.
> +void ProfileEstimatorPass::recurseBasicBlock(BasicBlock *BB) {
> +
> +  // break recursion if already visited

Nit: Please make this a well formed sentence, i.e. capitalize and add
a period. Same for a bunch of other comments... :)

> +  // if block is an loop header, first subtract all weigths from edges that
> +  // exit this loop, then distribute remaining weight on to the edges exiting
> +  // this loop. finally the weight of the block is increased, to simulate
> +  // several executions of this loop

weigths -> weights

> +bool ProfileEstimatorPass::runOnFunction(Function &F) {
> +  if (F.isDeclaration()) return false;
> +
> +  LI = &getAnalysis<LoopInfo>();
> +  FunctionInformation.erase(&F);
> +  BlockInformation[&F].clear();
> +  EdgeInformation[&F].clear();
> +  BBisVisited.clear();
> +
> +  DEBUG(DOUT<<"Working on function "<<F.getName()<<"\n");
> +
> +  // since the entry block is the first one and has no predecessors, the edge
> +  // (0,entry) is inserted with the starting weight of 1
> +  BasicBlock *entry = &F.getEntryBlock();
> +  BlockInformation[&F][entry] = 1;
> +
> +  Edge edge = getEdge(0,entry);
> +  EdgeInformation[&F][edge] = 1; EDGE_WEIGHT(edge);
> +  recurseBasicBlock(entry);

I was hoping ProfileInfo would be be an abstract base class by now so
this pass could didn't have to convert information into the format
ProfileInfo is using to store things. In particular, it would be nice
to eventually make this pass not compute anything until someone asks.
Of course we can always fix this once things have settled.
--

 - Daniel

On Sat, Aug 8, 2009 at 7:11 AM, Andreas
Neustifter<e0325716 at student.tuwien.ac.at> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi!
>
> Andreas Neustifter wrote:
>> Daniel Dunbar wrote:
>>> Thanks, applied as r78247. Next? :)
>>
>> Oh my, first he is lagging and now it's all a hurry :-)
>>
>> Okay, here comes the next one.
>>
>> I'm taking a short vacation next week so I will try to prepare 1 or 2 of
>> the next patches so that you can work on them as you have time.
>
> As promised, here is the next one.
>
> Greetings, Andi
>
> - --
> ==========================================================================
> This email is signed, for more information see
> http://web.student.tuwien.ac.at/~e0325716/gpg.html
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>
> iEYEARECAAYFAkp9h5sACgkQPiYq0rq7s/BmcgCfRlOfTNMnwpsf70SpQPaqMSh3
> RdMAniuAD3HNnaydQbfgqnfHycMdth45
> =uDtb
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>