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

JIT: Forward sub exposing missing containment safety checks in lower #64828

Closed
AndyAyersMS opened this issue Feb 4, 2022 · 1 comment
Closed

JIT: Forward sub exposing missing containment safety checks in lower #64828

AndyAyersMS opened this issue Feb 4, 2022 · 1 comment
Assignees
@AndyAyersMS
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone
7.0.0

Comments

@AndyAyersMS
Copy link
Member

AndyAyersMS commented Feb 4, 2022 •
edited
Loading

#63720 introduced a new optimization pass in the jit. We are seeing some failures because the more complex tree shapes this creates are mis-optimized downstream. The cases I know of are missing safety checks for contained memory operands.

See notes here: #64804 (comment)

So far just xarch is impacted, and it's possible these cases don't appear that often (the partial fix I have has no SPMI diffs).
@dotnet-issue-labeler dotnet-issue-labeler bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner labels Feb 4, 2022
@AndyAyersMS AndyAyersMS self-assigned this Feb 4, 2022
@ghost
Copy link

ghost commented Feb 4, 2022

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

Issue Details

#63720 introduced a new optimization pass in the jit. We are seeing some failures because the more complex tree shapes this creates are mis-optimized downstream. The cases I know of are missing safety checks for contained memory operands.

See notes here: #64804 (comment) and

So far just xarch is impacted, and it's possible these cases don't appear that often (the partial fix I have has no SPMI diffs).

Author: AndyAyersMS
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged
Milestone: -

@AndyAyersMS AndyAyersMS added this to the 7.0.0 milestone Feb 4, 2022
@AndyAyersMS AndyAyersMS removed the untriaged New issue has not been triaged by the area owner label Feb 4, 2022
AndyAyersMS added a commit to AndyAyersMS/runtime that referenced this issue Feb 5, 2022
@AndyAyersMS
JIT: contained memory safety analysis fixes and improvements
ad8feb9 
Fixes a couple of issues exposed by forward sub, where containment analysis
was allowing unsafe reordering of operands. More checks of this sort may be
needed.

See issues in dotnet#64828.

Generalize the safety check so that a store to a local not live into a handler
can be reordered with respect to node causing exceptions. Happily this leads
to almost uniformly better code despite the more stringent checking added above.

Add a workaround for the late callbacks into the containment checker made on
unlinked nodes. Assume these are always safe.
This was referenced Feb 5, 2022
Merged
Merged
@ghost ghost locked as resolved and limited conversation to collaborators Mar 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@AndyAyersMS AndyAyersMS

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

No branches or pull requests
1 participant
@AndyAyersMS