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

Remove getFieldAddress #79060

Merged
merged 5 commits into from
Dec 3, 2022
Merged

Remove getFieldAddress #79060

merged 5 commits into from
Dec 3, 2022

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Nov 30, 2022

Reverts part of the #78296 to drop getFieldAddress API

@ghost ghost assigned EgorBo Nov 30, 2022
@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 30, 2022
@ghost
Copy link

ghost commented Nov 30, 2022

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

Issue Details

Reverts part of the #78296 to drop getFieldAddress API

Author: EgorBo
Assignees: EgorBo
Labels:

area-CodeGen-coreclr
Milestone: -

@EgorBo
Copy link
Member Author

EgorBo commented Dec 1, 2022

/azp run runtime-coreclr outerloop, runtime-coreclr jitstress, runtime-coreclr jitstressregs

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@EgorBo
Copy link
Member Author

EgorBo commented Dec 1, 2022

/azp run runtime-coreclr outerloop, runtime-coreclr jitstress, runtime-coreclr jitstressregs

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@EgorBo EgorBo marked this pull request as ready for review December 1, 2022 13:56
@build-analysis build-analysis bot mentioned this pull request Dec 1, 2022
Closed
@EgorBo
Copy link
Member Author

EgorBo commented Dec 1, 2022 •
edited
Loading

@jkotas can you please review the VM side? I've extracted only the part where we delete getFieldAddress API that doesn't throw any NRE (I had 4 runs of CI here without it + local runs). I'll investigate the NRE + Frozen statics separately. This PR should also unblock NativeAOT work for static fields.

Current CI failures are known, I'll list related issues here

jkotas
jkotas approved these changes Dec 1, 2022
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

@EgorBo
Copy link
Member Author

EgorBo commented Dec 2, 2022

@jakobbotsch @SingleAccretion can any of you sign off the jit side? it didn't change since #77737

SingleAccretion
SingleAccretion approved these changes Dec 2, 2022
View reviewed changes
Copy link
Contributor

@SingleAccretion SingleAccretion left a comment

Choose a reason for hiding this comment

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

Jit changes still look good.

@EgorBo
Copy link
Member Author

EgorBo commented Dec 2, 2022 •
edited
Loading

Failures are:
Runtime_76194 - #78758
Runtime_31615 - filed #79170
jitstress is pretty broken - #79132 (comment)

Going to have another round of CI just in case.

@EgorBo EgorBo merged commit 720ae03 into dotnet:main Dec 3, 2022
@EgorBo EgorBo deleted the drop-getfieldaddress branch December 3, 2022 11:06
This was referenced Dec 6, 2022
Open
Open
This was referenced Dec 6, 2022
Closed
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Jan 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@SingleAccretion SingleAccretion SingleAccretion approved these changes

@jkotas jkotas jkotas approved these changes

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

Assignees

@EgorBo EgorBo

Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@EgorBo @jkotas @SingleAccretion