[PATCH] Propagation of profile samples through the CFG.

Diego Novillo dnovillo at google.com
Fri Jan 10 15:01:31 PST 2014


On Fri, Jan 10, 2014 at 5:04 PM, Chandler Carruth <chandlerc at gmail.com> wrote:

> ================
> Comment at: lib/Transforms/Scalar/SampleProfile.cpp:544-551
> @@ -451,3 +543,10 @@
>  ///
> -/// Once all the branch weights are computed, we emit the MD_prof
> -/// metadata on B using the computed values.
> +/// This traverses all the the deescendants of \p BB1 in
> +/// \p BB1's dominator tree (if \p CheckDT is true). For
> +/// each BB2 found that post-dominates \p BB1 and is
> +/// in the same loop as \p BB1, BB2's equivalence class
> +/// is set to BB1.
> +///
> +/// If \p CheckDT is false, the complementary check is
> +/// performed with \p BB1's post-dominator tree.
> +///
> ----------------
> This comment is a bit out of date.

Caught it after I sent the patch. Fixed.

> ================
> Comment at: lib/Transforms/Scalar/SampleProfile.cpp:558
> @@ +557,3 @@
> +    BasicBlock *BB1, SmallVector<BasicBlock *, 8> Descendants,
> +    DominatorTreeBase<BasicBlock> *DomTree) {
> +  for (SmallVectorImpl<BasicBlock *>::iterator I = Descendants.begin(),
> ----------------
> It would be good to clarify that this is the "other" dom tree from Descendants. Not sure twhat the best name is here. Maybe InverseDomTree? It's not really an inverse though.

I re-worded the description of the function. Hopefully, it's clearer now.


> ================
> Comment at: lib/Transforms/Scalar/SampleProfile.cpp:594-595
> @@ +593,4 @@
> +void SampleFunctionProfile::findEquivalenceClasses(Function &F) {
> +  SmallVector<BasicBlock *, 8> DominatedBBs;
> +  SmallVector<BasicBlock *, 8> PostDominatedBBs;
> +  DEBUG(dbgs() << "\nBlock equivalence classes\n");
> ----------------
> You only need one SmallVector? You can clear and re-use it.

Done.

Thanks. Diego.



More information about the llvm-commits mailing list