Skip to content

Commit 4642037

Browse files
committed
C++: Speed up IRGuardCondition::controlsBlock
The `controlsBlock` predicate had some dramatic bulges in its tuple counts. To make matters worse, those bulges were in materialized intermediate predicates like `#shared` and `#antijoin_rhs`, not just in the middle of a pipeline. The problem was particularly evident on kamailio/kamailio, where `controlsBlock` was the slowest predicate in the IR libraries: IRGuards::IRGuardCondition::controlsBlock_dispred#fff#shared#4 ........ 58.8s IRGuards::IRGuardCondition::controlsBlock_dispred#fff#antijoin_rhs .... 33.4s IRGuards::IRGuardCondition::controlsBlock_dispred#fff#antijoin_rhs#1 .. 26.7s The first of the above relations had 201M rows, and the others had intermediate bulges of similar size. The bulges could be observed even on small projects although they did not cause measurable performance issues there. The `controlsBlock_dispred#fff#shared#4` relation had 3M rows on git/git, which is a lot for a project with only 1.5M IR instructions. This commit borrows an efficient implementation from Java's `Guards.qll`, tweaking it slightly to fit into `IRGuards`. Performance is now much better: IRGuards::IRGuardCondition::controlsBlock_dispred#fff ................... 6.1s IRGuards::IRGuardCondition::hasDominatingEdgeTo_dispred#ff .............. 616ms IRGuards::IRGuardCondition::hasDominatingEdgeTo_dispred#ff#antijoin_rhs . 540ms After this commit, the biggest bulge in `controlsBlock` is the size of `IRBlock::dominates`. On kamailio/kamailio this is an intermediate tuple count of 18M rows in the calculation of `controlsBlock`, which in the end produces 11M rows.
1 parent cade3a3 commit 4642037

File tree

1 file changed

+29
-7
lines changed

1 file changed

+29
-7
lines changed

cpp/ql/src/semmle/code/cpp/controlflow/IRGuards.qll

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import semmle.code.cpp.ir.IR
1313
* has the AST for the `Function` itself, which tends to confuse mapping between the AST `BasicBlock`
1414
* and the `IRBlock`.
1515
*/
16+
pragma[noinline]
1617
private predicate isUnreachedBlock(IRBlock block) {
1718
block.getFirstInstruction() instanceof UnreachedInstruction
1819
}
@@ -405,16 +406,37 @@ class IRGuardCondition extends Instruction {
405406
*/
406407
private predicate controlsBlock(IRBlock controlled, boolean testIsTrue) {
407408
not isUnreachedBlock(controlled) and
408-
exists(IRBlock branchBlock | branchBlock.getAnInstruction() = branch |
409-
exists(IRBlock succ |
410-
this.hasBranchEdge(succ, testIsTrue) and
411-
succ.dominates(controlled) and
412-
forall(IRBlock pred | pred.getASuccessor() = succ |
413-
pred = branchBlock or succ.dominates(pred) or not pred.isReachableFromFunctionEntry()
414-
)
409+
// The following implementation is ported from `controls` in Java's
410+
// `Guards.qll`. See that file (at 398678a28) for further explanation and
411+
// correctness arguments.
412+
exists(IRBlock succ |
413+
this.hasBranchEdge(succ, testIsTrue) and
414+
this.hasDominatingEdgeTo(succ) and
415+
succ.dominates(controlled)
416+
)
417+
}
418+
419+
/**
420+
* Holds if `(this, succ)` is an edge that dominates `succ`, that is, all other
421+
* predecessors of `succ` are dominated by `succ`. This implies that `this` is the
422+
* immediate dominator of `succ`.
423+
*
424+
* This is a necessary and sufficient condition for an edge to dominate anything,
425+
* and in particular `bb1.hasDominatingEdgeTo(bb2) and bb2.dominates(bb3)` means
426+
* that the edge `(bb1, bb2)` dominates `bb3`.
427+
*/
428+
private predicate hasDominatingEdgeTo(IRBlock succ) {
429+
exists(IRBlock branchBlock | branchBlock = this.getBranchBlock() |
430+
branchBlock.immediatelyDominates(succ) and
431+
branchBlock.getASuccessor() = succ and
432+
forall(IRBlock pred | pred = succ.getAPredecessor() and pred != branchBlock |
433+
succ.dominates(pred)
415434
)
416435
)
417436
}
437+
438+
pragma[noinline]
439+
private IRBlock getBranchBlock() { result = branch.getBlock() }
418440
}
419441

420442
private ConditionalBranchInstruction get_branch_for_condition(Instruction guard) {

0 commit comments

Comments
 (0)