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

JIT: Clean up BB successor iteration #86839

Merged
merged 2 commits into from
Jun 6, 2023

Conversation

jakobbotsch
Copy link
Member

  • Remove old code
  • Use visitor with an array for AllSuccessorEnumerator that is still used by SSA

* Remove old code
* Use visitor with an array as part of AllSuccessorEnumerator
@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 May 27, 2023
@ghost ghost assigned jakobbotsch May 27, 2023
@ghost
Copy link

ghost commented May 27, 2023

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

Issue Details
  • Remove old code
  • Use visitor with an array for AllSuccessorEnumerator that is still used by SSA
Author: jakobbotsch
Assignees: jakobbotsch
Labels:

area-CodeGen-coreclr

Milestone: -

Comment on lines +1861 to +1864
// We store up to 4 successors inline in the enumerator. For ASP.NET
// and libraries.pmi this is enough in 99.7% of cases.
BasicBlock* m_successors[4];
BasicBlock** m_pSuccessors;
Copy link
Member Author

Choose a reason for hiding this comment

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

Some histograms for count of all-successors:
asp.net:

     <=          0 ===>  113751 count ( 15% of total)
      1 ..       1 ===>  232465 count ( 48% of total)
      2 ..       2 ===>  342117 count ( 96% of total)
      3 ..       3 ===>   23292 count ( 99% of total)
      4 ..       4 ===>    3215 count ( 99% of total)
      5 ..       5 ===>     700 count ( 99% of total)
      6 ..       6 ===>     392 count ( 99% of total)
      7 ..       7 ===>     165 count ( 99% of total)
      8 ..       8 ===>      99 count ( 99% of total)
      9 ..       9 ===>      76 count ( 99% of total)
     10 ..      10 ===>     152 count (100% of total)
      >         10 ===>     299 count (100% of total)

libraries.pmi:

     <=          0 ===>  291559 count ( 26% of total)
      1 ..       1 ===>  321737 count ( 56% of total)
      2 ..       2 ===>  438938 count ( 96% of total)
      3 ..       3 ===>   27017 count ( 99% of total)
      4 ..       4 ===>    4750 count ( 99% of total)
      5 ..       5 ===>    1642 count ( 99% of total)
      6 ..       6 ===>     522 count ( 99% of total)
      7 ..       7 ===>     261 count ( 99% of total)
      8 ..       8 ===>     161 count ( 99% of total)
      9 ..       9 ===>     130 count ( 99% of total)
     10 ..      10 ===>      79 count (100% of total)
      >         10 ===>     398 count (100% of total)

@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib PTAL @AndyAyersMS. Gets rid of the old iterator and changes the enumerator (that's still used for DFS purposes by SSA) to read all the successors into an array.

No diffs except for some minor TP improvements.

@jakobbotsch
Copy link
Member Author

Failure is known according to build analysis.

@jakobbotsch jakobbotsch merged commit 10222f9 into dotnet:main Jun 6, 2023
@jakobbotsch jakobbotsch deleted the block-successors-cleanup branch June 6, 2023 17:18
@ghost ghost locked as resolved and limited conversation to collaborators Jul 6, 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.

2 participants