Skip to content

Commit

Permalink
[BOLT] Fix SCTC execution count assertion
Browse files Browse the repository at this point in the history
Summary:
SCTC is currently asserting (my fault :-) when running in
combination with hot jump table entries optimization. This optimization
sets the frequency for edges connecting basic blocks it creates and jump
table targets based on the execution count of the original BB containing
the indirect jump.

This is OK as an estimation, but it breaks our assumption that the sum of
the frequency of preds edges equals to our BB frequency. This happens
because the frequency of the BB is rarely equal to its outgoing edges
frequency.

SCTC, in turn, was updating the execution count for BBs with tail calls
by subtracting the frequency count of predecessor edges. Because hot
jump table entries optimization broke the BB exec count = sum(preds freq)
invariant, SCTC was asserting.

To trigger this, the input program must have a jump table where each
entry contains a tail call. This happens in the HHVM binary for func
_ZN4HPHP11collections5issetEPNS_10ObjectDataEPKNS_10TypedValueE.

(cherry picked from FBD5222504)
  • Loading branch information
rafaelauler authored and maksfb committed Jun 9, 2017
1 parent eb63a0b commit eeea415
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions bolt/Passes/BinaryPasses.cpp
Expand Up @@ -650,9 +650,10 @@ uint64_t SimplifyConditionalTailCalls::fixTailCalls(BinaryContext &BC,
// if there are no other users.
PredBB->removeSuccessor(BB);
// Update BB execution count
if (BB->getKnownExecutionCount() > 0) {
assert(CTCTakenFreq <= BB->getKnownExecutionCount());
if (CTCTakenFreq && CTCTakenFreq <= BB->getKnownExecutionCount()) {
BB->setExecutionCount(BB->getExecutionCount() - CTCTakenFreq);
} else if (CTCTakenFreq > BB->getKnownExecutionCount()) {
BB->setExecutionCount(0);
}

++NumLocalCTCs;
Expand Down

0 comments on commit eeea415

Please sign in to comment.