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

JIT: Methods with switches aren't inlineable #55336

Closed
EgorBo opened this issue Jul 8, 2021 · 2 comments · Fixed by #55478
Closed

JIT: Methods with switches aren't inlineable #55336

EgorBo opened this issue Jul 8, 2021 · 2 comments · Fixed by #55478
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI tenet-performance Performance related issue
Milestone

Comments

@EgorBo
Copy link
Member

EgorBo commented Jul 8, 2021

Example:

public int Test1(char format) =>
    format switch
    {
        'A' => 1,
        'B' => 2,
        'C' => 3,
        'D' => 4,
        _ => 0,
    };

public int Test2() => Test1('B');

Codegen for Test2:

       mov      edx, 66
       jmp      ConsoleApp26.Program:Test1(ushort):int:this
; Total bytes of code: 10

Currently it's failed to inline into Test2:

INLINER: during 'fgInline' result 'failed this callee' reason 'has switch'

I guess it's tricky for the importer, but at least we could import it as a single JTRUE block when we pass a constant at the callsite (and that arg feeds that switch).

/cc @AndyAyersMS

@EgorBo EgorBo added the tenet-performance Performance related issue label Jul 8, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner labels Jul 8, 2021
@EgorBo EgorBo added this to the 7.0.0 milestone Jul 8, 2021
@EgorBo EgorBo self-assigned this Jul 8, 2021
@EgorBo EgorBo removed the untriaged New issue has not been triaged by the area owner label Jul 8, 2021
@MichalPetryka
Copy link
Contributor

MichalPetryka commented Jul 8, 2021

This is also a case for a switch on a sizeof(T) in a generic method with an unmanaged constraint, those switches should be completely ignored by the inliner as the JIT always knows which path will be taken.

@AndyAyersMS
Copy link
Member

See #10634 and #10658 (comment) -- latter has a prototype of the changes needed to enable inlining of switches.

There is a cache of switch info kept in the jit (m_switchDescMap) that might need updating/invalidating too, I haven't looked.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 11, 2021
@EgorBo EgorBo modified the milestones: 7.0.0, 6.0.0 Jul 12, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 14, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 13, 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 tenet-performance Performance related issue
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants