Skip to content

C++: Don't use GVN in AST DataFlow BarrierNode#2755

Merged
MathiasVP merged 1 commit into
github:masterfrom
jbj:BarrierGuard-SSA
Feb 4, 2020
Merged

C++: Don't use GVN in AST DataFlow BarrierNode#2755
MathiasVP merged 1 commit into
github:masterfrom
jbj:BarrierGuard-SSA

Conversation

@jbj

@jbj jbj commented Feb 4, 2020

Copy link
Copy Markdown
Contributor

It turns out that the evaluator will evaluate the GVN stage even when no predicate from it is needed after optimization of the subsequent stages. The GVN library is expensive to evaluate, and it'll become even more expensive when we switch its implementation to IR.

This PR disables the use of GVN in DataFlow::BarrierNode for the AST data-flow library, which should improve performance when evaluating a single data-flow query on a snapshot with no cache. Precision decreases slightly, leading to a new FP in the qltests.

There is no corresponding change for the IR data-flow library since IR GVN is not very expensive.

It turns out that the evaluator will evaluate the GVN stage even when no
predicate from it is needed after optimization of the subsequent stages.
The GVN library is expensive to evaluate, and it'll become even more
expensive when we switch its implementation to IR.

This PR disables the use of GVN in `DataFlow::BarrierNode` for the AST
data-flow library, which should improve performance when evaluating a
single data-flow query on a snapshot with no cache. Precision decreases
slightly, leading to a new FP in the qltests.

There is no corresponding change for the IR data-flow library since IR
GVN is not very expensive.
@jbj jbj added the C++ label Feb 4, 2020
@jbj jbj requested a review from geoffw0 February 4, 2020 07:54
@jbj jbj requested a review from a team as a code owner February 4, 2020 07:54
@jbj

jbj commented Feb 4, 2020

Copy link
Copy Markdown
Contributor Author

This is the change I talked about in the team meeting yesterday. The BarrierGuard feature was introduced in #2213.

@MathiasVP MathiasVP 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!

@MathiasVP MathiasVP merged commit 0276c97 into github:master Feb 4, 2020
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