Skip to content

Take into account zero-offset field sequences when propagating locals - #64701

Merged
jakobbotsch merged 3 commits into
dotnet:mainfrom
SingleAccretion:Prop-Loc-Fix
Feb 8, 2022
Merged

Take into account zero-offset field sequences when propagating locals#64701
jakobbotsch merged 3 commits into
dotnet:mainfrom
SingleAccretion:Prop-Loc-Fix

Conversation

@SingleAccretion

@SingleAccretionSingleAccretion commented Feb 2, 2022

Copy link
Copy Markdown
Contributor

Not doing so runs into the problem described in #64700.

Some small number of diffs from more/fewer propagations.

Fixes#64700.
Fixes#64761.

@ghostghost added 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 labels Feb 2, 2022
@ghost

ghost commented Feb 2, 2022

Copy link
Copy Markdown

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

Issue Details

Using tree VNs runs into the problem described in #64700.

We're expecting some small number of diffs from more/fewer propagations.

Fixes #64700. Also should fix the a != b asserts that have started appearing in our fuzzer runs.

Author:SingleAccretion
Assignees:-
Labels:

area-CodeGen-coreclr

Milestone:-

@SingleAccretionSingleAccretion changed the title Use SSA def VNs to determine legality of copy propagation (instead of tree VNs)Take into account zero-offset field sequences when propagating localsFeb 3, 2022
@SingleAccretion
SingleAccretionforce-pushed the Prop-Loc-Fix branch 2 times, most recently from 724543b to c5451c1CompareFebruary 3, 2022 21:07
@SingleAccretion
SingleAccretion marked this pull request as ready for review February 4, 2022 09:27
@SingleAccretion

Copy link
Copy Markdown
ContributorAuthor

@dotnet/jit-contrib

@jakobbotschjakobbotsch self-assigned this Feb 4, 2022
@jakobbotsch
jakobbotsch self-requested a review February 4, 2022 11:14
LCL_VAR use VN != VN of its SSA def.
Using one for replacement can run afoul of substituting
into a LCL_VAR that has a zero-offset field sequence on
top and thus duplicate the sequence (as the replacement
def will already have this sequence incorporated into its
VN).
Just one diff, missing null check elimination: we propagated a field
sequence for an array address (PtrToArrElem) that later wasn't recognized
as implying non-nullness of another PtrToArrElem to the same array.
Comment threadsrc/coreclr/jit/assertionprop.cpp
@jakobbotsch
jakobbotsch merged commit 713c8dd into dotnet:mainFeb 8, 2022
@SingleAccretion
SingleAccretion deleted the Prop-Loc-Fix branch February 8, 2022 16:21
@ghostghost locked as resolved and limited conversation to collaborators Mar 10, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMIcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Assertion failed 'a != b' during 'Do value numbering' Copy propagation needs to propagate variables, not values

2 participants

@SingleAccretion@jakobbotsch