Skip to content

Revert "Do not generate TYP_STRUCTLCL_FLD nodes (#66251)" - #67360

Merged
BruceForstall merged 1 commit into
dotnet:mainfrom
SingleAccretion:Revert-NoStrucLclFld
Mar 30, 2022
Merged

Revert "Do not generate TYP_STRUCTLCL_FLD nodes (#66251)"#67360
BruceForstall merged 1 commit into
dotnet:mainfrom
SingleAccretion:Revert-NoStrucLclFld

Conversation

@SingleAccretion

Copy link
Copy Markdown
Contributor

In #66251 I've enhanced IsLocalAddrExpr to look for zero-offset sequences on ADDRs.

This was a correctness fix.

Unfortunately, it also broke the structreturn test because we have the following tree there:

*****BB01, STMT00006(before)
N005 ( 10, 8) [000064] -A------R---*ASG long N004 ( 3, 2) [000062] D------N----+--*LCL_VAR long V04 tmp2 d:2N003 ( 6, 5) [000063] n-----------\--*IND long N002 ( 3, 3) [000043] ------------\--*ADDR byref ZeroFseq[a]
N001 ( 3, 2) [000044] -------N----\--*LCL_VAR struct<TestUnsafeCasts+largeStruct, 32> V00 loc0 u:6 (last use)

Where the field a does not belong to TestUnsafeCasts+largeStruct. This is the well-known VN problem with invalid field sequences, there is no expedient fix for it.

So I am reverting #66251, it seems TYP_STRUCTLCL_FLD is fully gated on fixing this problem (I knew it was gated at least partially, I did not anticipate it to break so quickly though).

Fixes#67346, #67355.

@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 Mar 30, 2022
@ghost

Copy link
Copy Markdown

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

Issue Details

In #66251 I've enhanced IsLocalAddrExpr to look for zero-offset sequences on ADDRs.

This was a correctness fix.

Unfortunately, it also broke the structreturn test because we have the following tree there:

*****BB01, STMT00006(before)
N005 ( 10, 8) [000064] -A------R---*ASG long N004 ( 3, 2) [000062] D------N----+--*LCL_VAR long V04 tmp2 d:2N003 ( 6, 5) [000063] n-----------\--*IND long N002 ( 3, 3) [000043] ------------\--*ADDR byref ZeroFseq[a]
N001 ( 3, 2) [000044] -------N----\--*LCL_VAR struct<TestUnsafeCasts+largeStruct, 32> V00 loc0 u:6 (last use)

Where the field a does not belong to TestUnsafeCasts+largeStruct. This is the well-known VN problem with invalid field sequences, there is no expedient fix for it.

So I am reverting #66251, it seems TYP_STRUCTLCL_FLD is fully gated on fixing this problem (I knew it was gated at least partially, I did not anticipate it to break so quickly though).

Fixes #67346, #67355.

Author:SingleAccretion
Assignees:-
Labels:

area-CodeGen-coreclr

Milestone:-

@SingleAccretion

Copy link
Copy Markdown
ContributorAuthor

@dotnet/jit-contrib

@BruceForstall
BruceForstall merged commit 59bb459 into dotnet:mainMar 30, 2022
@SingleAccretion
SingleAccretion deleted the Revert-NoStrucLclFld branch March 30, 2022 23:14
@ghostghost locked as resolved and limited conversation to collaborators Apr 30, 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 'lclNode->GetSsaNum() == SsaConfig::FIRST_SSA_NUM' during 'VN based copy prop'

3 participants

@SingleAccretion@EgorBo@BruceForstall