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

JIT: Avoid unnecessary GTF_GLOB_REFs #84349

Merged

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Apr 5, 2023

  • Avoid setting GTF_GLOB_REF on GT_FIELD_ADDR nodes
  • Avoid setting GTF_GLOB_REF on GT_FIELD nodes off of implicit byrefs. This is ok now since implicit byref morphing indiscriminately sets GTF_GLOB_REF.
  • Manually clone a "pointer to span" in span intrinsic expansion when it points to a local. Unfortunately this does not fall out from the above since gtClone does not handle FIELD_ADDR, and making it handle this needs some more work.

These changes are necessary to avoid address exposure in the two user benchmarks in #83388:

Fix #74563
Fix #856

* Avoid setting GTF_GLOB_REF on GT_FIELD_ADDR nodes
* Avoid setting GTF_GLOB_REF on GT_FIELD nodes off of implicit byrefs.
  This is ok now since implicit byref morphing indiscriminately sets
  GTF_GLOB_REF for these.

These changes are necessary to avoid address exposure in the two user
benchmarks in dotnet#83388.

Fix dotnet#74563
@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 5, 2023
@ghost ghost assigned jakobbotsch Apr 5, 2023
@ghost
Copy link

ghost commented Apr 5, 2023

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

Issue Details
  • Avoid setting GTF_GLOB_REF on GT_FIELD_ADDR nodes
  • Avoid setting GTF_GLOB_REF on GT_FIELD nodes off of implicit byrefs. This is ok now since implicit byref morphing indiscriminately sets GTF_GLOB_REF.

These changes are necessary to avoid address exposure in the two user benchmarks in #83388:

Both of these cases are fixed with this PR.

Fix #74563

Author: jakobbotsch
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch
Copy link
Member Author

/azp run Fuzzlyn, runtime-coreclr jitstress, runtime-coreclr libraries-jitstress

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jakobbotsch jakobbotsch marked this pull request as ready for review April 6, 2023 22:15
@jakobbotsch
Copy link
Member Author

jakobbotsch commented Apr 6, 2023

The hardware intrinsics test failure should be fixed by #84352. The Fuzzlyn failure is preexisting.

Diffs.

cc @dotnet/jit-contrib PTAL @SingleAccretion @EgorBo

@SingleAccretion
Copy link
Contributor

SingleAccretion commented Apr 7, 2023

Almost forgot - this also fixes #856.

@jakobbotsch jakobbotsch merged commit 15c7022 into dotnet:main Apr 7, 2023
@jakobbotsch jakobbotsch deleted the avoid-setting-glob-ref-on-field-refs branch April 7, 2023 19:05
@ghost ghost locked as resolved and limited conversation to collaborators May 8, 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
3 participants