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.

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

Do not add null checks for boxed statics #77796

Merged
merged 3 commits into from
Nov 3, 2022
Merged

Do not add null checks for boxed statics #77796

merged 3 commits into from
Nov 3, 2022

Conversation

SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Nov 2, 2022 •
edited
Loading

While investigating #77773, I noticed we were adding null checks for ref s_structStatic.Field, which is not necessary.

@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 Nov 2, 2022
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Nov 2, 2022
@ghost
Copy link

ghost commented Nov 2, 2022

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

Issue Details

While investigating #77773, I noticed we were adding null checks for ref structStatic.Field, which is not necessary.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr
Milestone: -

@SingleAccretion SingleAccretion marked this pull request as ready for review November 2, 2022 17:44
@SingleAccretion
Copy link
Contributor Author

SingleAccretion commented Nov 2, 2022 •
edited
Loading

This should clean the Fuzzlyn pipeline from some number of #77773-related failures; obviously, it does not solve the underlying issue.

@dotnet/jit-contrib

@EgorBo
Copy link
Member

EgorBo commented Nov 2, 2022

Failures seem related? Apparently some other type of GT_IND_NONNULL node has GTF_EXCEPT

@SingleAccretion
Copy link
Contributor Author

Diffs: a good amount of regressions due to the ARR_ELEM fix. We can take them here, or extract the ARR_ELEM change into a different PR.

@jakobbotsch
Copy link
Member

Diffs: a good amount of regressions due to the ARR_ELEM fix. We can take them here, or extract the ARR_ELEM change into a different PR.

I think it's fine to take it here. Thanks for looking at them.

@jakobbotsch jakobbotsch merged commit c1dea77 into dotnet:main Nov 3, 2022
@SingleAccretion SingleAccretion deleted the Do-Not-Add-Null-Checks-For-Boxed-Statics-Upstream branch November 3, 2022 18:19
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jakobbotsch jakobbotsch jakobbotsch approved these changes

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.
3 participants
@SingleAccretion @EgorBo @jakobbotsch