Skip to content

Make atomics GenTreeIndir-derived #93295

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
jakobbotsch merged 7 commits into from
Oct 16, 2023
Merged

Make atomics GenTreeIndir-derived #93295

jakobbotsch merged 7 commits into from
Oct 16, 2023

Conversation

@SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Oct 10, 2023 •
edited
Loading

This is the first (zero-diff, hence quirks) part of fixing #92784.

We make all atomics OperIsIndir - they are a lot like stores and should share the common machinery like GTF_IND_NONFAULTING handling seamlessly this way.

@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Oct 10, 2023
@ghost
Copy link

ghost commented Oct 10, 2023

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

Issue Details

This is the first (zero-diff, hence quirks) part of fixing #92784.

We make all atomics OperIsIndir - they are a lot like stores and should share the common machinery like GTF_IND_NONFAULTING handling.

There is a bit of a TP diff locally that looks like inlining noise.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr, community-contribution
Milestone: -

@SingleAccretion SingleAccretion force-pushed the Atomics-Are-Indirs-Part-One branch 2 times, most recently from e8aba0e to 4bb303c Compare October 11, 2023 11:32
@SingleAccretion SingleAccretion force-pushed the Atomics-Are-Indirs-Part-One branch from 4bb303c to 783e017 Compare October 11, 2023 12:08
@SingleAccretion
Copy link
Contributor Author

SingleAccretion commented Oct 11, 2023 •
edited
Loading

@dotnet/jit-contrib

Diffs - none in code, as expected.

There is an anomalous TP regression on Windows x64; it is not yet clear why that is - it does not reproduce locally. I will need to pull the latest main and recheck, however, it may also be due to the difference in MSVC versions. In either case, I believe it to be due to some different inlining decisions.

Edit: I have confirmed the above. GenTree::OperRequiresCallFlag is no longer inlined into all of its callers because of the change in the opcode order (which, perhaps, makes MSVC's IR for it use a switch and be considered more complex...).

@SingleAccretion SingleAccretion marked this pull request as ready for review October 11, 2023 15:19
@jakobbotsch jakobbotsch merged commit 45ed0ab into dotnet:main Oct 16, 2023
@SingleAccretion SingleAccretion mentioned this pull request Oct 17, 2023
Merged
@SingleAccretion SingleAccretion deleted the Atomics-Are-Indirs-Part-One branch October 27, 2023 17:50
@ghost ghost locked as resolved and limited conversation to collaborators Nov 26, 2023
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.

2 participants

@SingleAccretion @jakobbotsch