Skip to content

Commit 0a59045

Browse files
jbjjbj
committed
C++: Use unique aggregate in IRBlock computation
This gives a slight speedup, and I think it makes the code shorter and clearer. On Wireshark, the time from the beginning of the `IRBlock` stage until just before evaluation of `getInstruction` drops from 44s to 34s.
1 parent 4ce896b commit 0a59045

5 files changed

Lines changed: 90 additions & 115 deletions

File tree

‎cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRBlock.qll‎

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -101,23 +101,24 @@ class IRBlock extends IRBlockBase {
101101

102102
private predicate startsBasicBlock(Instruction instr) {
103103
not instr instanceof PhiInstruction and
104-
(
105-
count(Instruction predecessor | instr = predecessor.getASuccessor()) != 1 // Multiple predecessors or no predecessor
106-
or
107-
exists(Instruction predecessor |
108-
instr = predecessor.getASuccessor() and
109-
strictcount(Instruction other | other = predecessor.getASuccessor()) > 1
110-
) // Predecessor has multiple successors
111-
or
112-
exists(Instruction predecessor, EdgeKind kind |
113-
instr = predecessor.getSuccessor(kind) and
114-
not kind instanceof GotoEdge
115-
) // Incoming edge is not a GotoEdge
116-
or
117-
exists(Instruction predecessor |
118-
instr = Construction::getInstructionBackEdgeSuccessor(predecessor, _)
119-
) // A back edge enters this instruction
120-
)
104+
not adjacentInBlock(_, instr)
105+
}
106+
107+
/** Holds if `i2` follows `i1` in a `IRBlock`. */
108+
private predicate adjacentInBlock(Instruction i1, Instruction i2) {
109+
// - i2 must be the only successor of i1
110+
i2 = unique(Instruction i | i = i1.getASuccessor()) and
111+
// - i1 must be the only predecessor of i2
112+
i1 = unique(Instruction i | i.getASuccessor() = i2) and
113+
// - The edge between the two must be a GotoEdge. We just check that one
114+
// exists since we've already checked that it's unique.
115+
exists(GotoEdge edgeKind | exists(i1.getSuccessor(edgeKind))) and
116+
// - The edge must not be a back edge. This means we get the same back edges
117+
// in the basic-block graph as we do in the raw CFG.
118+
not exists(Construction::getInstructionBackEdgeSuccessor(i1, _))
119+
// This predicate could be simplified to remove one of the `unique`s if we
120+
// were willing to rely on the CFG being well-formed and thus never having
121+
// more than one successor to an instruction that has a `GotoEdge` out of it.
121122
}
122123

123124
private predicate isEntryBlock(TIRBlock block) {
@@ -129,12 +130,6 @@ private module Cached {
129130
cached
130131
newtype TIRBlock = MkIRBlock(Instruction firstInstr) { startsBasicBlock(firstInstr) }
131132

132-
/** Holds if `i2` follows `i1` in a `IRBlock`. */
133-
private predicate adjacentInBlock(Instruction i1, Instruction i2) {
134-
exists(GotoEdge edgeKind | i2 = i1.getSuccessor(edgeKind)) and
135-
not startsBasicBlock(i2)
136-
}
137-
138133
/** Holds if `i` is the `index`th instruction the block starting with `first`. */
139134
private Instruction getInstructionFromFirst(Instruction first, int index) =
140135
shortestDistances(startsBasicBlock/1, adjacentInBlock/2)(first, result, index)

‎cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRBlock.qll‎

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -101,23 +101,24 @@ class IRBlock extends IRBlockBase {
101101

102102
private predicate startsBasicBlock(Instruction instr) {
103103
not instr instanceof PhiInstruction and
104-
(
105-
count(Instruction predecessor | instr = predecessor.getASuccessor()) != 1 // Multiple predecessors or no predecessor
106-
or
107-
exists(Instruction predecessor |
108-
instr = predecessor.getASuccessor() and
109-
strictcount(Instruction other | other = predecessor.getASuccessor()) > 1
110-
) // Predecessor has multiple successors
111-
or
112-
exists(Instruction predecessor, EdgeKind kind |
113-
instr = predecessor.getSuccessor(kind) and
114-
not kind instanceof GotoEdge
115-
) // Incoming edge is not a GotoEdge
116-
or
117-
exists(Instruction predecessor |
118-
instr = Construction::getInstructionBackEdgeSuccessor(predecessor, _)
119-
) // A back edge enters this instruction
120-
)
104+
not adjacentInBlock(_, instr)
105+
}
106+
107+
/** Holds if `i2` follows `i1` in a `IRBlock`. */
108+
private predicate adjacentInBlock(Instruction i1, Instruction i2) {
109+
// - i2 must be the only successor of i1
110+
i2 = unique(Instruction i | i = i1.getASuccessor()) and
111+
// - i1 must be the only predecessor of i2
112+
i1 = unique(Instruction i | i.getASuccessor() = i2) and
113+
// - The edge between the two must be a GotoEdge. We just check that one
114+
// exists since we've already checked that it's unique.
115+
exists(GotoEdge edgeKind | exists(i1.getSuccessor(edgeKind))) and
116+
// - The edge must not be a back edge. This means we get the same back edges
117+
// in the basic-block graph as we do in the raw CFG.
118+
not exists(Construction::getInstructionBackEdgeSuccessor(i1, _))
119+
// This predicate could be simplified to remove one of the `unique`s if we
120+
// were willing to rely on the CFG being well-formed and thus never having
121+
// more than one successor to an instruction that has a `GotoEdge` out of it.
121122
}
122123

123124
private predicate isEntryBlock(TIRBlock block) {
@@ -129,12 +130,6 @@ private module Cached {
129130
cached
130131
newtype TIRBlock = MkIRBlock(Instruction firstInstr) { startsBasicBlock(firstInstr) }
131132

132-
/** Holds if `i2` follows `i1` in a `IRBlock`. */
133-
private predicate adjacentInBlock(Instruction i1, Instruction i2) {
134-
exists(GotoEdge edgeKind | i2 = i1.getSuccessor(edgeKind)) and
135-
not startsBasicBlock(i2)
136-
}
137-
138133
/** Holds if `i` is the `index`th instruction the block starting with `first`. */
139134
private Instruction getInstructionFromFirst(Instruction first, int index) =
140135
shortestDistances(startsBasicBlock/1, adjacentInBlock/2)(first, result, index)

‎cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRBlock.qll‎

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -101,23 +101,24 @@ class IRBlock extends IRBlockBase {
101101

102102
private predicate startsBasicBlock(Instruction instr) {
103103
not instr instanceof PhiInstruction and
104-
(
105-
count(Instruction predecessor | instr = predecessor.getASuccessor()) != 1 // Multiple predecessors or no predecessor
106-
or
107-
exists(Instruction predecessor |
108-
instr = predecessor.getASuccessor() and
109-
strictcount(Instruction other | other = predecessor.getASuccessor()) > 1
110-
) // Predecessor has multiple successors
111-
or
112-
exists(Instruction predecessor, EdgeKind kind |
113-
instr = predecessor.getSuccessor(kind) and
114-
not kind instanceof GotoEdge
115-
) // Incoming edge is not a GotoEdge
116-
or
117-
exists(Instruction predecessor |
118-
instr = Construction::getInstructionBackEdgeSuccessor(predecessor, _)
119-
) // A back edge enters this instruction
120-
)
104+
not adjacentInBlock(_, instr)
105+
}
106+
107+
/** Holds if `i2` follows `i1` in a `IRBlock`. */
108+
private predicate adjacentInBlock(Instruction i1, Instruction i2) {
109+
// - i2 must be the only successor of i1
110+
i2 = unique(Instruction i | i = i1.getASuccessor()) and
111+
// - i1 must be the only predecessor of i2
112+
i1 = unique(Instruction i | i.getASuccessor() = i2) and
113+
// - The edge between the two must be a GotoEdge. We just check that one
114+
// exists since we've already checked that it's unique.
115+
exists(GotoEdge edgeKind | exists(i1.getSuccessor(edgeKind))) and
116+
// - The edge must not be a back edge. This means we get the same back edges
117+
// in the basic-block graph as we do in the raw CFG.
118+
not exists(Construction::getInstructionBackEdgeSuccessor(i1, _))
119+
// This predicate could be simplified to remove one of the `unique`s if we
120+
// were willing to rely on the CFG being well-formed and thus never having
121+
// more than one successor to an instruction that has a `GotoEdge` out of it.
121122
}
122123

123124
private predicate isEntryBlock(TIRBlock block) {
@@ -129,12 +130,6 @@ private module Cached {
129130
cached
130131
newtype TIRBlock = MkIRBlock(Instruction firstInstr) { startsBasicBlock(firstInstr) }
131132

132-
/** Holds if `i2` follows `i1` in a `IRBlock`. */
133-
private predicate adjacentInBlock(Instruction i1, Instruction i2) {
134-
exists(GotoEdge edgeKind | i2 = i1.getSuccessor(edgeKind)) and
135-
not startsBasicBlock(i2)
136-
}
137-
138133
/** Holds if `i` is the `index`th instruction the block starting with `first`. */
139134
private Instruction getInstructionFromFirst(Instruction first, int index) =
140135
shortestDistances(startsBasicBlock/1, adjacentInBlock/2)(first, result, index)

‎csharp/ql/src/semmle/code/csharp/ir/implementation/raw/IRBlock.qll‎

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -101,23 +101,24 @@ class IRBlock extends IRBlockBase {
101101

102102
private predicate startsBasicBlock(Instruction instr) {
103103
not instr instanceof PhiInstruction and
104-
(
105-
count(Instruction predecessor | instr = predecessor.getASuccessor()) != 1 // Multiple predecessors or no predecessor
106-
or
107-
exists(Instruction predecessor |
108-
instr = predecessor.getASuccessor() and
109-
strictcount(Instruction other | other = predecessor.getASuccessor()) > 1
110-
) // Predecessor has multiple successors
111-
or
112-
exists(Instruction predecessor, EdgeKind kind |
113-
instr = predecessor.getSuccessor(kind) and
114-
not kind instanceof GotoEdge
115-
) // Incoming edge is not a GotoEdge
116-
or
117-
exists(Instruction predecessor |
118-
instr = Construction::getInstructionBackEdgeSuccessor(predecessor, _)
119-
) // A back edge enters this instruction
120-
)
104+
not adjacentInBlock(_, instr)
105+
}
106+
107+
/** Holds if `i2` follows `i1` in a `IRBlock`. */
108+
private predicate adjacentInBlock(Instruction i1, Instruction i2) {
109+
// - i2 must be the only successor of i1
110+
i2 = unique(Instruction i | i = i1.getASuccessor()) and
111+
// - i1 must be the only predecessor of i2
112+
i1 = unique(Instruction i | i.getASuccessor() = i2) and
113+
// - The edge between the two must be a GotoEdge. We just check that one
114+
// exists since we've already checked that it's unique.
115+
exists(GotoEdge edgeKind | exists(i1.getSuccessor(edgeKind))) and
116+
// - The edge must not be a back edge. This means we get the same back edges
117+
// in the basic-block graph as we do in the raw CFG.
118+
not exists(Construction::getInstructionBackEdgeSuccessor(i1, _))
119+
// This predicate could be simplified to remove one of the `unique`s if we
120+
// were willing to rely on the CFG being well-formed and thus never having
121+
// more than one successor to an instruction that has a `GotoEdge` out of it.
121122
}
122123

123124
private predicate isEntryBlock(TIRBlock block) {
@@ -129,12 +130,6 @@ private module Cached {
129130
cached
130131
newtype TIRBlock = MkIRBlock(Instruction firstInstr) { startsBasicBlock(firstInstr) }
131132

132-
/** Holds if `i2` follows `i1` in a `IRBlock`. */
133-
private predicate adjacentInBlock(Instruction i1, Instruction i2) {
134-
exists(GotoEdge edgeKind | i2 = i1.getSuccessor(edgeKind)) and
135-
not startsBasicBlock(i2)
136-
}
137-
138133
/** Holds if `i` is the `index`th instruction the block starting with `first`. */
139134
private Instruction getInstructionFromFirst(Instruction first, int index) =
140135
shortestDistances(startsBasicBlock/1, adjacentInBlock/2)(first, result, index)

‎csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/IRBlock.qll‎

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -101,23 +101,24 @@ class IRBlock extends IRBlockBase {
101101

102102
private predicate startsBasicBlock(Instruction instr) {
103103
not instr instanceof PhiInstruction and
104-
(
105-
count(Instruction predecessor | instr = predecessor.getASuccessor()) != 1 // Multiple predecessors or no predecessor
106-
or
107-
exists(Instruction predecessor |
108-
instr = predecessor.getASuccessor() and
109-
strictcount(Instruction other | other = predecessor.getASuccessor()) > 1
110-
) // Predecessor has multiple successors
111-
or
112-
exists(Instruction predecessor, EdgeKind kind |
113-
instr = predecessor.getSuccessor(kind) and
114-
not kind instanceof GotoEdge
115-
) // Incoming edge is not a GotoEdge
116-
or
117-
exists(Instruction predecessor |
118-
instr = Construction::getInstructionBackEdgeSuccessor(predecessor, _)
119-
) // A back edge enters this instruction
120-
)
104+
not adjacentInBlock(_, instr)
105+
}
106+
107+
/** Holds if `i2` follows `i1` in a `IRBlock`. */
108+
private predicate adjacentInBlock(Instruction i1, Instruction i2) {
109+
// - i2 must be the only successor of i1
110+
i2 = unique(Instruction i | i = i1.getASuccessor()) and
111+
// - i1 must be the only predecessor of i2
112+
i1 = unique(Instruction i | i.getASuccessor() = i2) and
113+
// - The edge between the two must be a GotoEdge. We just check that one
114+
// exists since we've already checked that it's unique.
115+
exists(GotoEdge edgeKind | exists(i1.getSuccessor(edgeKind))) and
116+
// - The edge must not be a back edge. This means we get the same back edges
117+
// in the basic-block graph as we do in the raw CFG.
118+
not exists(Construction::getInstructionBackEdgeSuccessor(i1, _))
119+
// This predicate could be simplified to remove one of the `unique`s if we
120+
// were willing to rely on the CFG being well-formed and thus never having
121+
// more than one successor to an instruction that has a `GotoEdge` out of it.
121122
}
122123

123124
private predicate isEntryBlock(TIRBlock block) {
@@ -129,12 +130,6 @@ private module Cached {
129130
cached
130131
newtype TIRBlock = MkIRBlock(Instruction firstInstr) { startsBasicBlock(firstInstr) }
131132

132-
/** Holds if `i2` follows `i1` in a `IRBlock`. */
133-
private predicate adjacentInBlock(Instruction i1, Instruction i2) {
134-
exists(GotoEdge edgeKind | i2 = i1.getSuccessor(edgeKind)) and
135-
not startsBasicBlock(i2)
136-
}
137-
138133
/** Holds if `i` is the `index`th instruction the block starting with `first`. */
139134
private Instruction getInstructionFromFirst(Instruction first, int index) =
140135
shortestDistances(startsBasicBlock/1, adjacentInBlock/2)(first, result, index)

0 commit comments

Comments
 (0)