Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ssa: Refactor shared SSA in preparation for eliminating phi-read definitions #18819

Merged
merged 30 commits into from
Feb 21, 2025
Merged
Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
3822d14
SSA: Elaborate qldoc.
aschackmull Feb 11, 2025
36613e1
SSA: Remove superfluous conjunct (implied by refRank).
aschackmull Feb 11, 2025
b62432f
SSA: Remove superfluous boolean.
aschackmull Feb 14, 2025
c5e2884
SSA: Remove superfluous column from liveAtRank.
aschackmull Feb 14, 2025
5723d27
SSA: Make inReadDominanceFrontier a bit smaller without affecting phi…
aschackmull Feb 14, 2025
f80cd97
SSA: Factor out phi-read construction in its own predicate.
aschackmull Feb 17, 2025
81b3035
SSA: Inline predicate
aschackmull Feb 17, 2025
613323e
SSA: Copy parts of SsaDefReaches verbatim to new module.
aschackmull Feb 17, 2025
ea08c60
SSA: Copy 3 predicates from the outer scope verbatim into the new Rea…
aschackmull Feb 17, 2025
6e272d0
SSA: Ignore phi-reads in the new def-reaches module.
aschackmull Feb 17, 2025
77ccff6
SSA: Replace exported def-reaches predicates (behaviour-preserving).
aschackmull Feb 17, 2025
7e441d9
SSA: Fold getImmediateBasicBlockDominator into loop-invariant predicate.
aschackmull Feb 17, 2025
a4fee2e
SSA: Minor perf tweak to reduce tuple duplication.
aschackmull Feb 17, 2025
d5ac5b4
SSA: Replace uncertainWriteDefinitionInput implementation.
aschackmull Feb 17, 2025
11166fc
SSA: Restrict phi-read creation to be based on reachable reads.
aschackmull Feb 17, 2025
411aff6
SSA: Refactor ranking into parameterised module.
aschackmull Feb 17, 2025
d6dc91d
SSA: Inline predicate to simplify negation.
aschackmull Feb 17, 2025
35f50ba
SSA: Reimplement use-use.
aschackmull Feb 17, 2025
194afbb
Java: Simplify SSA for variable capture.
aschackmull Feb 18, 2025
cf2136f
SSA: Export simple firstUse and adjacentUseUse predicates.
aschackmull Feb 18, 2025
5379506
Java: Use firstUse and adjacentUseUse predicates.
aschackmull Feb 18, 2025
291ea6f
Java: Move SSA data flow test and extend it to cover phi-read input e…
aschackmull Feb 18, 2025
ed40035
C#/Ruby/Rust: Fix bug in adjacentReadPairSameVar.
aschackmull Feb 19, 2025
17ae747
C#: Switch use-use predicates to new implementation.
aschackmull Feb 19, 2025
b0a5e62
C#: Clean up unused.
aschackmull Feb 19, 2025
4ddc5c9
Ruby: Switch use-use predicates to new implementation.
aschackmull Feb 19, 2025
7e59603
Rust: Switch use-use predicates to new implementation.
aschackmull Feb 19, 2025
b76e5f5
SSA: Deprecate unused predicate.
aschackmull Feb 19, 2025
8e609b1
Ruby: Accept qltest change.
aschackmull Feb 20, 2025
8c0cc07
Ssa: Fix qldoc duplicate word.
aschackmull Feb 20, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions shared/ssa/codeql/ssa/Ssa.qll
Original file line number Diff line number Diff line change
Expand Up @@ -359,13 +359,17 @@ module Make<LocationSig Location, InputSig<Location> Input> {
* either a read (when `k` is `SsaActualRead()`) or an SSA definition (when
* `k` is `SsaDef()`).
*
* Unlike `Liveness::ref`, this includes `phi` nodes.
* Unlike `Liveness::ref`, this includes `phi` nodes and pseudo-reads
* associated with uncertain writes.
*/
pragma[nomagic]
predicate ssaRef(BasicBlock bb, int i, SourceVariable v, SsaRefKind k) {
variableRead(bb, i, v, _) and
k = SsaActualRead()
or
variableWrite(bb, i, v, false) and
k = SsaActualRead()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it that we have "pseudo-reads associated with uncertain writes"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes it much simpler to calculate uncertainWriteDefinitionInput.

or
any(Definition def).definesAt(v, bb, i) and
k = SsaDef()
}
Expand Down Expand Up @@ -483,6 +487,13 @@ module Make<LocationSig Location, InputSig<Location> Input> {
ssaDefReachesEndOfBlock(getImmediateBasicBlockDominator(bb), def, v) and
not ssaDefReachesReadWithinBlock(v, _, bb, i)
}

predicate uncertainWriteDefinitionInput(UncertainWriteDefinition def, Definition inp) {
exists(SourceVariable v, BasicBlock bb, int i |
def.definesAt(v, bb, i) and
ssaDefReachesRead(v, inp, bb, i)
)
}
}

private module SsaDefReaches {
Expand Down Expand Up @@ -861,7 +872,10 @@ module Make<LocationSig Location, InputSig<Location> Input> {
*
* Same as `ssaDefReachesReadExt`, but ignores phi-reads.
*/
predicate ssaDefReachesRead = SsaDefReachesNew::ssaDefReachesRead/4;
predicate ssaDefReachesRead(SourceVariable v, Definition def, BasicBlock bb, int i) {
SsaDefReachesNew::ssaDefReachesRead(v, def, bb, i) and
variableRead(bb, i, v, _)
}

/**
* NB: If this predicate is exposed, it should be cached.
Expand Down Expand Up @@ -994,10 +1008,7 @@ module Make<LocationSig Location, InputSig<Location> Input> {
* `def`. Since `def` is uncertain, the value from the preceding definition might
* still be valid.
*/
pragma[nomagic]
predicate uncertainWriteDefinitionInput(UncertainWriteDefinition def, Definition inp) {
lastRefRedef(inp, _, _, def)
}
predicate uncertainWriteDefinitionInput = SsaDefReachesNew::uncertainWriteDefinitionInput/2;

/** Holds if `bb` is a control-flow exit point. */
private predicate exitBlock(BasicBlock bb) { not exists(getABasicBlockSuccessor(bb)) }
Expand Down