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

Improve value numbering for casts #59841

Merged
merged 4 commits into from
Oct 15, 2021

Conversation

SingleAccretion
Copy link
Contributor

@SingleAccretion SingleAccretion commented Sep 30, 2021

Now that we no longer fold overflowing conversions with implementation-defined behavior, it is possible to unify how the cast helpers are value numbered and how regular casts are numbered.

There are also changes that enable global constant propagation into calls, this was necessary to fix some of the regressions that showed up. The remaining regressions are due to #57033 (we propagate "just above" the helper and see it as a side effect).

Diffs: win-x64, win-x86, linux-arm

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

ghost commented Sep 30, 2021

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

Issue Details

Now that we no longer fold overflowing conversions with implementation-defined behavior, it is possible to unify how the cast helpers are value numbered and how regular casts are numbered.

There are also changes that enable global constant propagation into calls, this was necessary to fix some of the regressions that showed up. The remaining regressions are due to #57033 (we propagate "just above" the helper and see it as a side effect).

Diffs: pending.

Author: SingleAccretion
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@SingleAccretion SingleAccretion force-pushed the Refactor-Cast-VNs branch 3 times, most recently from b2d50ef to fc2cc3d Compare October 1, 2021 15:31
Remove duplication between VNPairForCast
and VNForCast, route the former through
the latter.

Also change the debug output for casts.

No diffs for this commit.
VN cast helpers just like casts.
This allows them to be folded.

In the past, this was dangerous because some of
the pessimization protected us from undefined
behavior in cases of out-of-bounds conversions,
but now we do not fold such cases anymore.
This solves some of the regressions seen due to
now-missing CSEs of these helper calls evaluated
as constants by VN.
This fixes the test failures revealed by
the more aggressive constant propagation.

This is very similar to a bug that was recently
fixed where checked casts were treated as non-checked
and could be observed without the assertion propagation
changes as well, with the redundant branches optimization.
@SingleAccretion SingleAccretion marked this pull request as ready for review October 1, 2021 22:09
@SingleAccretion
Copy link
Contributor Author

@dotnet/jit-contrib

Copy link
Member

@jakobbotsch jakobbotsch left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jakobbotsch jakobbotsch merged commit 7ecced4 into dotnet:main Oct 15, 2021
@SingleAccretion SingleAccretion deleted the Refactor-Cast-VNs branch October 15, 2021 15:54
@ghost ghost locked as resolved and limited conversation to collaborators Nov 14, 2021
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 community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants