Skip to content

Commit 23e63e9

Browse files
hvitvedhvitved
committed
C#: Improve join order in conditionalAssign()
Fixes a bad join-order in `Guards::Internal::conditionalAssign#ffff#antijoin_rhs#1`: ``` [2019-01-25 14:12:03] (377s) Starting to evaluate predicate Guards::Internal::conditionalAssign#ffff#antijoin_rhs#1 [2019-01-25 14:20:41] (895s) Tuple counts: 9302551 ~1% {7} r1 = JOIN ControlFlowGraph::ControlFlow::Internal::PreSsa::Definition::getAPhiInput_dispred#ff WITH Guards::Internal::conditionalAssign#ffff#shared#1 ON ControlFlowGraph::ControlFlow::Internal::PreSsa::Definition::getAPhiInput_dispred#ff.<0>=Guards::Internal::conditionalAssign#ffff#shared#1.<0> OUTPUT FIELDS {ControlFlowGraph::ControlFlow::Internal::PreSsa::Definition::getAPhiInput_dispred#ff.<1>,Guards::Internal::conditionalAssign#ffff#shared#1.<1>,Guards::Internal::conditionalAssign#ffff#shared#1.<2>,Guards::Internal::conditionalAssign#ffff#shared#1.<0>,Guards::Internal::conditionalAssign#ffff#shared#1.<3>,Guards::Internal::conditionalAssign#ffff#shared#1.<4>,Guards::Internal::conditionalAssign#ffff#shared#1.<5>} 9302551 ~7% {8} r2 = JOIN r1 WITH ControlFlowGraph::ControlFlow::Internal::PreSsa::Definition::getBasicBlock_dispred#ff ON r1.<0>=ControlFlowGraph::ControlFlow::Internal::PreSsa::Definition::getBasicBlock_dispred#ff.<0> OUTPUT FIELDS {r1.<1>,ControlFlowGraph::ControlFlow::Internal::PreSsa::Definition::getBasicBlock_dispred#ff.<1>,r1.<2>,r1.<3>,r1.<4>,r1.<5>,r1.<6>,r1.<0>} 1223774650 ~0% {8} r3 = JOIN r2 WITH Guards::Internal::Guard::preControlsDirect_dispred#fff ON r2.<0>=Guards::Internal::Guard::preControlsDirect_dispred#fff.<0> AND r2.<1>=Guards::Internal::Guard::preControlsDirect_dispred#fff.<1> OUTPUT FIELDS {r2.<6>,Guards::Internal::Guard::preControlsDirect_dispred#fff.<2>,r2.<0>,r2.<2>,r2.<3>,r2.<4>,r2.<5>,r2.<7>} 80626 ~0% {7} r4 = JOIN r3 WITH Guards::AbstractValue::getDualValue_dispred#ff ON r3.<0>=Guards::AbstractValue::getDualValue_dispred#ff.<0> AND r3.<1>=Guards::AbstractValue::getDualValue_dispred#ff.<1> OUTPUT FIELDS {r3.<2>,r3.<3>,r3.<4>,r3.<5>,r3.<6>,r3.<0>,r3.<7>} 9293564 ~0% {7} r5 = Guards::Internal::conditionalAssign#ffff#shared#2 AND NOT Guards::Internal::conditionalAssign#ffff#antijoin_rhs(Guards::Internal::conditionalAssign#ffff#shared#2.<0>,Guards::Internal::conditionalAssign#ffff#shared#2.<1>,Guards::Internal::conditionalAssign#ffff#shared#2.<2>,Guards::Internal::conditionalAssign#ffff#shared#2.<3>,Guards::Internal::conditionalAssign#ffff#shared#2.<4>,Guards::Internal::conditionalAssign#ffff#shared#2.<5>,Guards::Internal::conditionalAssign#ffff#shared#2.<6>) 9293564 ~1% {7} r6 = SCAN r5 OUTPUT FIELDS {r5.<6>,r5.<0>,r5.<1>,r5.<2>,r5.<3>,r5.<4>,r5.<5>} 9293564 ~2% {8} r7 = JOIN r6 WITH ControlFlowGraph::ControlFlow::Internal::PreSsa::Definition::getBasicBlock_dispred#ff ON r6.<0>=ControlFlowGraph::ControlFlow::Internal::PreSsa::Definition::getBasicBlock_dispred#ff.<0> OUTPUT FIELDS {ControlFlowGraph::ControlFlow::Internal::PreSsa::Definition::getBasicBlock_dispred#ff.<1>,r6.<2>,r6.<1>,r6.<3>,r6.<4>,r6.<5>,r6.<6>,r6.<0>} 1940 ~2% {7} r8 = JOIN r7 WITH ControlFlowGraph::ControlFlow::Internal::PreBasicBlocks::PreBasicBlock::dominates_dispred#ff ON r7.<0>=ControlFlowGraph::ControlFlow::Internal::PreBasicBlocks::PreBasicBlock::dominates_dispred#ff.<0> AND r7.<1>=ControlFlowGraph::ControlFlow::Internal::PreBasicBlocks::PreBasicBlock::dominates_dispred#ff.<1> OUTPUT FIELDS {r7.<2>,r7.<1>,r7.<3>,r7.<4>,r7.<5>,r7.<6>,r7.<7>} 82566 ~0% {7} r9 = r4 \/ r8 return r9 ```
1 parent 47ad280 commit 23e63e9

File tree

1 file changed

+49
-10
lines changed
  • csharp/ql/src/semmle/code/csharp/controlflow

1 file changed

+49
-10
lines changed

‎csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll‎

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -895,6 +895,48 @@ module Internal {
895895
)
896896
}
897897

898+
pragma[noinline]
899+
private predicate conditionalAssign0(
900+
Guard guard, AbstractValue vGuard, PreSsa::Definition def, Expr e, PreSsa::Definition upd,
901+
PreBasicBlocks::PreBasicBlock bbGuard
902+
) {
903+
e = upd.getDefinition().getSource() and
904+
upd = def.getAPhiInput() and
905+
guard.preControlsDirect(upd.getBasicBlock(), vGuard) and
906+
bbGuard.getAnElement() = guard and
907+
bbGuard.strictlyDominates(def.getBasicBlock()) and
908+
not guard.preControlsDirect(def.getBasicBlock(), vGuard)
909+
}
910+
911+
pragma[noinline]
912+
private predicate conditionalAssign1(
913+
Guard guard, AbstractValue vGuard, PreSsa::Definition def, Expr e, PreSsa::Definition upd,
914+
PreBasicBlocks::PreBasicBlock bbGuard, PreSsa::Definition other
915+
) {
916+
conditionalAssign0(guard, vGuard, def, e, upd, bbGuard) and
917+
other != upd and
918+
other = def.getAPhiInput()
919+
}
920+
921+
pragma[noinline]
922+
private predicate conditionalAssign2(
923+
Guard guard, AbstractValue vGuard, PreSsa::Definition def, Expr e, PreSsa::Definition upd,
924+
PreBasicBlocks::PreBasicBlock bbGuard, PreSsa::Definition other
925+
) {
926+
conditionalAssign1(guard, vGuard, def, e, upd, bbGuard, other) and
927+
guard.preControlsDirect(other.getBasicBlock(), vGuard.getDualValue())
928+
}
929+
930+
pragma[noinline]
931+
private predicate conditionalAssign3(
932+
Guard guard, AbstractValue vGuard, PreSsa::Definition def, Expr e, PreSsa::Definition upd,
933+
PreBasicBlocks::PreBasicBlock bbGuard, PreSsa::Definition other
934+
) {
935+
conditionalAssign1(guard, vGuard, def, e, upd, bbGuard, other) and
936+
other.getBasicBlock().dominates(bbGuard) and
937+
not PreSsa::ssaDefReachesEndOfBlock(guard.getConditionalSuccessor(vGuard), other, _)
938+
}
939+
898940
/**
899941
* Holds if the evaluation of `guard` to `vGuard` implies that `def` is assigned
900942
* expression `e`.
@@ -916,28 +958,25 @@ module Internal {
916958
)
917959
or
918960
exists(PreSsa::Definition upd, PreBasicBlocks::PreBasicBlock bbGuard |
919-
e = upd.getDefinition().getSource() and
920-
upd = def.getAPhiInput() and
921-
guard.preControlsDirect(upd.getBasicBlock(), vGuard) and
922-
bbGuard.getAnElement() = guard and
923-
bbGuard.strictlyDominates(def.getBasicBlock()) and
924-
not guard.preControlsDirect(def.getBasicBlock(), vGuard) and
925-
forall(PreSsa::Definition other | other != upd and other = def.getAPhiInput() |
961+
conditionalAssign0(guard, vGuard, def, e, upd, bbGuard)
962+
|
963+
forall(PreSsa::Definition other |
964+
conditionalAssign1(guard, vGuard, def, e, upd, bbGuard, other)
965+
|
926966
// For example:
927967
// if (guard)
928968
// upd = a;
929969
// else
930970
// other = b;
931971
// def = phi(upd, other)
932-
guard.preControlsDirect(other.getBasicBlock(), vGuard.getDualValue())
972+
conditionalAssign2(guard, vGuard, def, e, upd, bbGuard, other)
933973
or
934974
// For example:
935975
// other = a;
936976
// if (guard)
937977
// upd = b;
938978
// def = phi(other, upd)
939-
other.getBasicBlock().dominates(bbGuard) and
940-
not PreSsa::ssaDefReachesEndOfBlock(guard.getConditionalSuccessor(vGuard), other, _)
979+
conditionalAssign3(guard, vGuard, def, e, upd, bbGuard, other)
941980
)
942981
)
943982
}

0 commit comments

Comments
 (0)