[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