Skip to content

JIT: Do not report duplicate EH clauses to VM #114895

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Apr 23, 2025
Merged

JIT: Do not report duplicate EH clauses to VM #114895

merged 13 commits into from
Apr 23, 2025

Conversation

filipnavara
Copy link
Member

Fixes #114891

@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 22, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Apr 22, 2025
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

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

@filipnavara filipnavara marked this pull request as draft April 22, 2025 08:23
@build-analysis build-analysis bot mentioned this pull request Apr 22, 2025
Open
3 tasks
jkotas
jkotas approved these changes Apr 22, 2025
View reviewed changes
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@filipnavara filipnavara force-pushed the no-duplicate-eh-clauses branch from c6cb6a4 to 07c0a5a Compare April 22, 2025 17:56
@filipnavara filipnavara marked this pull request as ready for review April 22, 2025 17:59
@jkotas @filipnavara
Update src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs
88b00cb 
Co-authored-by: Filip Navara <filip.navara@gmail.com>
BruceForstall
BruceForstall reviewed Apr 22, 2025
View reviewed changes
Copy link
Member

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

I think you should update the JIT-EE GUID

@BruceForstall
Copy link
Member

Also, update docs/design/coreclr/botr/clr-abi.md

@BruceForstall
Copy link
Member

It looks like SOS in https://github.com/dotnet/diagnostics knows about dumping duplicated clauses. What is the compat story there? Does it need to run against older runtimes, or is it tightly bound to a runtime version? Does this limit changing corhdr.h/corinfo.h? @mikem8361

@filipnavara
Copy link
Member Author

I think you should update the JIT-EE GUID

Will do.

Also, update docs/design/coreclr/botr/clr-abi.md

I removed the section for "duplicated clauses". There's another out-of-date section for "cloned finallys". Not sure how to reword it since it talks about JIT32, JIT64 and desktop CLR.

@filipnavara filipnavara marked this pull request as draft April 22, 2025 18:29
@filipnavara
Copy link
Member Author

Keeping it as draft until we resolve the question about dotnet/diagnostics and updates to clr-abi.md.

@jkotas
Copy link
Member

jkotas commented Apr 22, 2025

There's another out-of-date section for "cloned finallys". Not sure how to reword it since it talks about JIT32, JIT64 and desktop CLR.

Per the discussion in previous PR, it should be fine to delete everything that is not relevant to current main.

@filipnavara
Copy link
Member Author

It looks like SOS in https://github.com/dotnet/diagnostics knows about dumping duplicated clauses. What is the compat story there?

The code would continue to work but stop reporting the duplicate clauses and cloned finallys when running with newer runtime/DAC. I think that's fine since the runtime doesn't use the information so the value of reporting it is dubious at best. It's still useful for older runtimes.

I'd be happy to hear a second opinion though.

@filipnavara filipnavara marked this pull request as ready for review April 22, 2025 20:56
@jkotas jkotas mentioned this pull request Apr 23, 2025
Closed
@build-analysis build-analysis bot mentioned this pull request Apr 23, 2025
Closed
jkotas
jkotas approved these changes Apr 23, 2025
View reviewed changes
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thank you!

@jkotas jkotas merged commit 97b50b0 into dotnet:main Apr 23, 2025
114 of 116 checks passed
@filipnavara filipnavara deleted the no-duplicate-eh-clauses branch April 23, 2025 04:46
AndyAyersMS added a commit to AndyAyersMS/runtime that referenced this pull request Apr 25, 2025
@AndyAyersMS
JIT: fix ACD EH region removal updates for filters
2c8fdab 
A filter ACD may change region number because some other EH region was
deleted. Handle this properly.

Fixes dotnet#114895
AndyAyersMS added a commit to AndyAyersMS/runtime that referenced this pull request Apr 25, 2025
@AndyAyersMS
JIT: fix ACD EH region removal updates for filters
90f2ad7 
A filter ACD may change region number because some other EH region was
deleted. Handle this properly.

Fixes dotnet#114895
AndyAyersMS added a commit to AndyAyersMS/runtime that referenced this pull request Apr 25, 2025
@AndyAyersMS
JIT: fix ACD EH region removal updates for filters
33cae7d 
A filter ACD may change region number because some other EH region was
deleted. Handle this properly.

Fixes dotnet#114895
@AndyAyersMS AndyAyersMS mentioned this pull request Apr 25, 2025
Merged
AndyAyersMS added a commit that referenced this pull request Apr 25, 2025
@AndyAyersMS
JIT: fix ACD EH region removal updates for filters (#115054)
a1435f1 
A filter ACD may change region number because some other EH region was
deleted. Handle this properly.

Fixes #114895
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BruceForstall BruceForstall BruceForstall left review comments

@jkotas jkotas jkotas approved these changes

@MichalStrehovsky MichalStrehovsky Awaiting requested review from MichalStrehovsky MichalStrehovsky is a code owner

Assignees
No one assigned
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Eliminate duplicate EH clauses in JIT
3 participants
@filipnavara @BruceForstall @jkotas