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

optAssertionProp_Ind should eliminate GTF_EXCEPT better. #32248

Closed
sandreenko opened this issue Feb 13, 2020 · 3 comments
Closed

optAssertionProp_Ind should eliminate GTF_EXCEPT better. #32248

sandreenko opened this issue Feb 13, 2020 · 3 comments
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI optimization
Milestone

Comments

@sandreenko
Copy link
Contributor

sandreenko commented Feb 13, 2020

Consider a tree like:

[000189] -A-XG-------              |  |  +--*  COMMA     void  
[000184] -A-XG---R---              |  |  |  +--*  not important trees here
[000188] -A-X----R---              |  |  |  \--*  ASG       ref   
[000185] D------N----              |  |  |     +--*  LCL_VAR   ref    V20 tmp15        d:2
[000187] ---X--------              |  |  |     \--*  IND       ref   
[000186] ------------              |  |  |        \--*  LCL_VAR   byref  V25 tmp20        u:2 Zero Fseq[Syntax]
[000195] ---X--------              |  |  \--*  COMMA     void  
[000194] ---X--------              |  |     +--*  IND       int   
[000193] -------N----              |  |     |  \--*  ADD       byref 
[000191] ------------              |  |     |     +--*  LCL_VAR   byref  V25 tmp20        u:2
[000192] ------------              |  |     |     \--*  CNS_INT   long   8 Fseq[PrecedingInitializersLength] 

We access V25 through indirection twice, once to get its field [Syntax] at 0 offset and then to get [PrecedingInitializersLength] with 8 offset.

Obviously, the second indirection can't fail, so [000194] should be deleted, but the current implementation of optAssertionProp_Ind can see that because [000191] and [000186] have different value numbers, because one of them has Zero Fseq[Syntax] attached.

category:cq
theme:assertion-prop
skill-level:expert
cost:medium

@sandreenko sandreenko added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI optimization labels Feb 13, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Feb 13, 2020
@AndyAyersMS AndyAyersMS removed the untriaged New issue has not been triaged by the area owner label Feb 18, 2020
@AndyAyersMS AndyAyersMS added this to the Future milestone Feb 18, 2020
@AndyAyersMS
Copy link
Member

Marking as future for now.

@sandreenko is this something you're going to pursue?

@sandreenko
Copy link
Contributor Author

@AndyAyersMS yes, but I am not sure that it will fit into 5.0, so I think it is ok to keep it as future for now.

@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@BruceForstall BruceForstall removed the JitUntriaged CLR JIT issues needing additional triage label Nov 25, 2020
@SingleAccretion
Copy link
Contributor

Zero-offset sequences have been deleted in #71455, making this issue obsolete.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 25, 2022
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 optimization
Projects
None yet
Development

No branches or pull requests

5 participants