[PATCH] D40891: Fix accidentally quadratic time in BlockFrequencyInfoImpl::propagateMassToSuccessors

Andrew Scheidecker via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 6 15:19:31 PST 2017


AndrewScheidecker added a comment.

I ran the patch through clang-format. But looking at the code again, I see that calling the `getEdgeProbability` overload with a successor index or iterator instead of the destination basic block actually does something different from the original code: passing a successor index/iterator gets the probability for a single edge, while passing a basic block sums the probability for all edges between the two blocks.

However, and I have to admit that I'm in over my head here, what the original code was doing doesn't seem correct. For each edge from A to B, it was summing the probability of all edges from A to B, then adding the sum to the mass for B. If there are N edges from A to B, and they have probabilities that sum to M, then this will distribute N*M mass to B instead of M. In my contrived case, N=196928.

I still think this change is good, but I didn't mean for it to change the results. What are your thoughts?


Repository:
  rL LLVM

https://reviews.llvm.org/D40891





More information about the llvm-commits mailing list