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 EH dead code elimination #93428

Merged
merged 3 commits into from
Oct 14, 2023

Conversation

BruceForstall
Copy link
Member

Update fgRemoveDeadBlocks() to only treat EH handlers as reachable if their corresponding try block entry is reachable.

This causes more dead code to be eliminated in handlers that are not reachable.

There is a little more work to do to completely clean these up:

  1. If a try is not reachable, we should get rid of its entire EH table entry. Currently, its block is generated as int3 (on xarch).
  2. Even though the handler is not reachable, we still generate the prolog followed by int3 for the body.

Contributes to #82335

@ghost ghost assigned BruceForstall Oct 12, 2023
@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 Oct 12, 2023
@ghost
Copy link

ghost commented Oct 12, 2023

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

Issue Details

Update fgRemoveDeadBlocks() to only treat EH handlers as reachable if their corresponding try block entry is reachable.

This causes more dead code to be eliminated in handlers that are not reachable.

There is a little more work to do to completely clean these up:

  1. If a try is not reachable, we should get rid of its entire EH table entry. Currently, its block is generated as int3 (on xarch).
  2. Even though the handler is not reachable, we still generate the prolog followed by int3 for the body.

Contributes to #82335

Author: BruceForstall
Assignees: BruceForstall
Labels:

area-CodeGen-coreclr

Milestone: -

@BruceForstall
Copy link
Member Author

@AndyAyersMS PTAL
cc @dotnet/jit-contrib

Copy link
Member

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

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

LGTM

@AndyAyersMS
Copy link
Member

AndyAyersMS commented Oct 13, 2023

  1. If a try is not reachable, we should get rid of its entire EH table entry.

We encode EH region numbers into the AddCodeDsc entries (starting in morph) so if we remove things from the EH table we'll need to fix these up too. Or perhaps rework all this so these entries hang off the EH table somehow and migrate organically along with their related regions.

@BruceForstall
Copy link
Member Author

We encode EH region numbers into the AddCodeDsc entries

In particular, using bbThrowIndex(), to ensure fgAddCodeRef only shares throw blocks in the same EH region.

Update `fgRemoveDeadBlocks()` to only treat EH handlers as reachable
if their corresponding `try` block entry is reachable.

This causes more dead code to be eliminated in handlers that are
not reachable.

There is a little more work to do to completely clean these up:
1. If a `try` is not reachable, we should get rid of its entire EH table
entry. Currently, its block is generated as `int3` (on xarch).
2. Even though the handler is not reachable, we still generate the prolog
followed by `int3` for the body.

Contributes to dotnet#82335
fgRemoveBlockAsPred had special handling to not decrease the bbRefs count
of the filter-handler targeted by BBJ_EHFILTERRET. This is incorrect, as
the filter-handler already has an extra "beginning of handler" extra
ref count. Possibly this code was never invoked before as filters were
not previously removed.

Also, fix the JitDump output of the filter ret block in block dumps.
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.

Interesting that we don't see much of a TP win here.

I wonder if we should try doing this earlier?

@BruceForstall
Copy link
Member Author

Interesting that we don't see much of a TP win here.

Would you expect a TP win because of less IR? It seems like unreachable handlers should be rare, especially after the early EH opts. This still doesn't remove an unreachable try or the unreachable handler (it just turns them into int3), or the "dead" EH region.

I wonder if we should try doing this earlier?

Seems like it needs to be late enough for all the normal opts creating dead flow to run.

@BruceForstall
Copy link
Member Author

/azp run runtime-coreclr jitstress

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BruceForstall
Copy link
Member Author

Test failure is #93506

@BruceForstall BruceForstall merged commit d7c8198 into dotnet:main Oct 14, 2023
152 of 155 checks passed
@BruceForstall BruceForstall deleted the ImproveEHDeadCodeElimination branch October 14, 2023 05:07
@ghost ghost locked as resolved and limited conversation to collaborators Nov 13, 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.

3 participants