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

Improve reachability sets computation #84204

Merged

Conversation

BruceForstall
Copy link
Member

Two changes to improve the throughput of computing bbReach sets:

  1. In fgComputeReachabilitySets(), iterate over the blocks in reverse post-order. This leads to fewer outer do ... while(change) iterations. To do this, the fgDfsReversePostorder() function which creates the reverse post-order numbers and ordering was hoisted out of dominator creation and above reachability computation.
  2. Create a BlockSetOps::UnionDChanged() function that does a UnionD operation but also returns true if the target bitset changed value.

Some additional stats were added under COUNT_BASIC_BLOCKS regarding how many iterations dominators and reachability computations take to converge.

Two changes to improve the throughput of computing `bbReach` sets:
1. In `fgComputeReachabilitySets()`, iterate over the blocks in reverse
post-order. This leads to fewer outer `do ... while(change)` iterations.
To do this, the `fgDfsReversePostorder()` function which creates the
reverse post-order numbers and ordering was hoisted out of dominator
creation and above reachability computation.
2. Create a `BlockSetOps::UnionDChanged()` function that does a `UnionD`
operation but also returns `true` if the target bitset changed value.

Some additional stats were added under `COUNT_BASIC_BLOCKS` regarding
how many iterations dominators and reachability computations take to
converge.
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 1, 2023
@ghost ghost assigned BruceForstall Apr 1, 2023
@ghost
Copy link

ghost commented Apr 1, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak
See info in area-owners.md if you want to be subscribed.

Issue Details

Two changes to improve the throughput of computing bbReach sets:

  1. In fgComputeReachabilitySets(), iterate over the blocks in reverse post-order. This leads to fewer outer do ... while(change) iterations. To do this, the fgDfsReversePostorder() function which creates the reverse post-order numbers and ordering was hoisted out of dominator creation and above reachability computation.
  2. Create a BlockSetOps::UnionDChanged() function that does a UnionD operation but also returns true if the target bitset changed value.

Some additional stats were added under COUNT_BASIC_BLOCKS regarding how many iterations dominators and reachability computations take to converge.

Author: BruceForstall
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@BruceForstall
Copy link
Member Author

Diffs

TP improvements up to -0.09%

@BruceForstall
Copy link
Member Author

@AndyAyersMS PTAL
cc @dotnet/jit-contrib

Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

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

LGTM.

Surprised it didn't help more. Maybe the fussing about with BBF_GC_SAFE_POINT is inefficient?

Seems like that could be handled without branching, eg

predGcSafe &= (pred->bbFlags & BBF_GC_SAFE_POINT) != 0;

@@ -228,6 +228,8 @@ class BitSetOps

// Destructively modify "bs1" to be the union of "bs1" and "bs2".
static void UnionD(Env env, BitSetType& bs1, BitSetValueArgType bs2);
// Destructively modify "bs1" to be the union of "bs1" and "bs2"; return `true` if `bs1` changed.
static void UnionDChanged(Env env, BitSetType& bs1, BitSetValueArgType bs2);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
static void UnionDChanged(Env env, BitSetType& bs1, BitSetValueArgType bs2);
static bool UnionDChanged(Env env, BitSetType& bs1, BitSetValueArgType bs2);

@BruceForstall BruceForstall merged commit e65ebc9 into dotnet:main Apr 1, 2023
@BruceForstall BruceForstall deleted the ImproveReachabilityComputations branch April 1, 2023 18:20
@ghost ghost locked as resolved and limited conversation to collaborators May 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants