[LLVMdev] Profiling in LLVM Patch Followup 1

[Daniel Dunbar]

Applied as r78477.

I do have a few comments on the patch, below, but I'll wait to see
where things are going before acting on them. :)

--

> Index: include/llvm/Analysis/ProfileInfo.h
> ===================================================================

> +    // MissingValue - The value that is returned for execution counts in case
> +    // no value is available.
> +    static const int MissingValue = -1;

This should be a double?

> +    // getFunction() - Returns the Function for an Edge, checking for validity.
> +    static const Function* getFunction(Edge e) {
> +      assert(e.second && "Invalid ProfileInfo::Edge");
> +      return e.second->getParent();
> +    }
> +
> +    // getEdge() - Creates an Edge from two BasicBlocks.
> +    static Edge getEdge(const BasicBlock* Src, const BasicBlock* Dest) {
> +      return std::make_pair(Src,Dest);
> +    }

I think these should be private; eventually they are just an
implementation detail of a particular ProfileInfo provider.


> -  // We don't want to create an std::set unless we are dealing with a block that
> -  // has a LARGE number of in-edges.  Handle the common case of having only a
> -  // few in-edges with special code.

Thank you for killing this premature optimization. :)

> +  // the sum of the edge frequencies from the incoming edges.

the -> The

>    std::set<const BasicBlock*> ProcessedPreds;
> -  ProcessedPreds.insert(FirstPred);
> -  ProcessedPreds.insert(SecondPred);
> -  ProcessedPreds.insert(ThirdPred);
> +  double Count = 0;
>    for (; PI != PE; ++PI)
> -    if (ProcessedPreds.insert(*PI).second)
> -      Count += getEdgeWeight(*PI, BB);
> +    if (ProcessedPreds.insert(*PI).second) {
> +      double w = getEdgeWeight(getEdge(*PI, BB));
> +      if (w == MissingValue) {
> +        Count = MissingValue; break;

Please break the line before 'break'.

This code also makes me thing MissingValue should be renamed,
MissingValue is something like "NoData", whereas MissingValue could
easily be interpreted as NeverExecuted. I think there should probably
be a comment in the doc for this method that the count goes to
MissingValue if any edge is missing data.

--

Thanks,
 - Daniel


On Sat, Aug 8, 2009 at 6:41 AM, Andreas
Neustifter<e0325716 at student.tuwien.ac.at> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> 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.
>
> 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
>
> iEYEARECAAYFAkp9gHUACgkQPiYq0rq7s/AILQCfcuSVw+jSRyX2E2olE4cgbZE7
> d0MAnRNj8LtvUJ+/3BpeLj023+5srbVm
> =5NqX
> -----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
>
>