Skip to content

CPP: Speed up inconsistentLoopDirection.ql.#401

Merged
jbj merged 2 commits into
github:masterfrom
geoffw0:loopdir
Nov 5, 2018
Merged

CPP: Speed up inconsistentLoopDirection.ql.#401
jbj merged 2 commits into
github:masterfrom
geoffw0:loopdir

Conversation

@geoffw0

@geoffw0 geoffw0 commented Nov 2, 2018

Copy link
Copy Markdown
Contributor

Speed up the illDefined*ForStmt predicates in inconsistentLoopDirection.ql. I believe the issue was a cartesian product between v.getAnAccess() and v.getAnAssignedValue(), which caused these predicates alone to take 3m49s when I ran the query with a clean cache on libretro_libretro-uae. They now take negligible time.

The overall query time dropped from 1337s to 1115s - I believe at least 700s of the remaining time may be consumed by a similar issue in FlowVar in the DataFlow library.

@jbj @pavgust

@geoffw0 geoffw0 added the C++ label Nov 2, 2018
@geoffw0 geoffw0 requested a review from a team as a code owner November 2, 2018 16:36
jbj
jbj previously approved these changes Nov 5, 2018

@jbj jbj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. I also tested this on FrodeSolheim/fs-uae, where this query now takes negligible time. Before this change, the query took longer than I was willing to wait.

Can you make the tests pass? Maybe push a whitespace fix to re-trigger the Azure tests.

predicate candidateForStmt(ForStmt forStmt, Variable v, CrementOperation update, RelationalOperation rel) {
update = forStmt.getUpdate() and
update.getAnOperand() = v.getAnAccess() and
rel = forStmt.getCondition()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

trailing space

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@jbj jbj merged commit ba91f3e into github:master Nov 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants