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

Strange boxing deopt #82291

Closed
NinoFloris opened this issue Feb 17, 2023 · 4 comments · Fixed by #101303
Closed

Strange boxing deopt #82291

NinoFloris opened this issue Feb 17, 2023 · 4 comments · Fixed by #101303
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI help wanted [up-for-grabs] Good issue for external contributors in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@NinoFloris
Copy link
Contributor

Description

Given the following code, only the first function optimizes acceptably here. I'm not quite sure why it can't kick in for the other cases?

Reproduction Steps

public class C {
    
    [SharpLab.Runtime.JitGeneric(typeof(int))]
    public bool GetValueOrNull<T>()
    {
        if (default(T) != null)
         return true;
         
       return false;
    }    
    
    [SharpLab.Runtime.JitGeneric(typeof(int))]
    public bool GetValueOrNullSlow<T>()
    {
        if (default(T) != null)
         return false;
         
       return false;
    }    

    
    [SharpLab.Runtime.JitGeneric(typeof(int))]
    public void GetValueOrNullSlow2<T>()
    {
        if (default(T) != null)
         return;
         
       return;
    }    
}

https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEBDAzgWwB8ABAJgEYBYAKGIAYACY8gOgCUBXAOwwEt8YLAMIR8AB14AbGFADKMgG68wMXAG4a9JuQCsG6poDMTUgyEMA3jQY2G12wG1ZAC2xQxAGWzB23PgJYAKV4MAHEYLhllAAoMAE8xGAgAM2jeHgBKDIBdextiY2AICEkGcIwANWxJDhgAeSgAOQ5JSQAeABUAPmiMvMt+2wZeZIZogBMYZOwWjGiOjIYAQgBeBi4WyT7qId3iAHYGDCha/V2hwfzD6clcGDPbAF8Lndt+p1d3Lx9OHn5BYJhCJRMCxBJJVLpDBZXKvfKFYqlcpVGr1JqbWSSCAAd06PW2QyscN2IzGk2ms3mi1W602BPOVwYNzuDwZlyY12qLP6zzeBmJ7xcbk83l8fwCgPCkSgMXiiRSaUyOX6BSYKDKMEq1VqDWarUxONIeN6/SJDOGowmUxmkjmC2Waw2rXpDIOrPO7LdPJejyAA

Expected behavior

GetValueOrNullSlow and GetValueOrNullSlow2 are both optimized like GetValueOrNull

Actual behavior

They don't

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 17, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 17, 2023
@benaadams
Copy link
Member

Aside as workaround; replacing default(T) != null with typeof(T).IsValueType is optimised in all three cases

@vonzshik
Copy link
Contributor

@benaadams that would be incorrect for nullable types (like int?).

@SingleAccretion
Copy link
Contributor

The reason is that Roslyn (helpfully) optimizes the method itself:

IL_0000: ldloca.s 0
IL_0002: initobj !!T
IL_0008: ldloc.0
IL_0009: box !!T
IL_000e: pop
IL_000f: ldc.i4.0
IL_0010: ret

The Jit then doesn't drop the unused box.

The fix would be to simplify BOXes when importing pops; it would also be good to share that code more generally for "importing unused values" (this handling exists for unused inlinee parameters, for example).

@SingleAccretion SingleAccretion added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Feb 17, 2023
@SingleAccretion SingleAccretion added this to the Future milestone Feb 17, 2023
@ghost
Copy link

ghost commented Feb 17, 2023

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

Issue Details

Description

Given the following code, only the first function optimizes acceptably here. I'm not quite sure why it can't kick in for the other cases?

Reproduction Steps

public class C {
    
    [SharpLab.Runtime.JitGeneric(typeof(int))]
    public bool GetValueOrNull<T>()
    {
        if (default(T) != null)
         return true;
         
       return false;
    }    
    
    [SharpLab.Runtime.JitGeneric(typeof(int))]
    public bool GetValueOrNullSlow<T>()
    {
        if (default(T) != null)
         return false;
         
       return false;
    }    

    
    [SharpLab.Runtime.JitGeneric(typeof(int))]
    public void GetValueOrNullSlow2<T>()
    {
        if (default(T) != null)
         return;
         
       return;
    }    
}

https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEBDAzgWwB8ABAJgEYBYAKGIAYACY8gOgCUBXAOwwEt8YLAMIR8AB14AbGFADKMgG68wMXAG4a9JuQCsG6poDMTUgyEMA3jQY2G12wG1ZAC2xQxAGWzB23PgJYAKV4MAHEYLhllAAoMAE8xGAgAM2jeHgBKDIBdextiY2AICEkGcIwANWxJDhgAeSgAOQ5JSQAeABUAPmiMvMt+2wZeZIZogBMYZOwWjGiOjIYAQgBeBi4WyT7qId3iAHYGDCha/V2hwfzD6clcGDPbAF8Lndt+p1d3Lx9OHn5BYJhCJRMCxBJJVLpDBZXKvfKFYqlcpVGr1JqbWSSCAAd06PW2QyscN2IzGk2ms3mi1W602BPOVwYNzuDwZlyY12qLP6zzeBmJ7xcbk83l8fwCgPCkSgMXiiRSaUyOX6BSYKDKMEq1VqDWarUxONIeN6/SJDOGowmUxmkjmC2Waw2rXpDIOrPO7LdPJejyAA

Expected behavior

GetValueOrNullSlow and GetValueOrNullSlow2 are both optimized like GetValueOrNull

Actual behavior

They don't

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

Author: NinoFloris
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged

Milestone: -

@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Feb 17, 2023
@SingleAccretion SingleAccretion added help wanted [up-for-grabs] Good issue for external contributors untriaged New issue has not been triaged by the area owner and removed untriaged New issue has not been triaged by the area owner labels Feb 17, 2023
@ghost ghost added in-pr There is an active PR which will close this issue when it is merged and removed in-pr There is an active PR which will close this issue when it is merged labels Feb 17, 2023
@AndyAyersMS AndyAyersMS modified the milestones: Future, 9.0.0 Oct 24, 2023
AndyAyersMS added a commit to AndyAyersMS/runtime that referenced this issue Apr 19, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Apr 19, 2024
matouskozak pushed a commit to matouskozak/runtime that referenced this issue Apr 30, 2024
@github-actions github-actions bot locked and limited conversation to collaborators May 21, 2024
Ruihan-Yin pushed a commit to Ruihan-Yin/runtime that referenced this issue May 30, 2024
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 help wanted [up-for-grabs] Good issue for external contributors in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
6 participants