Skip to content

JIT: Improve wrapper method inliner heuristic#113266

Closed
hez2010 wants to merge 7 commits intodotnet:mainfrom
hez2010:inline-wrapper
Closed

JIT: Improve wrapper method inliner heuristic#113266
hez2010 wants to merge 7 commits intodotnet:mainfrom
hez2010:inline-wrapper

Conversation

@hez2010
Copy link
Contributor

@hez2010 hez2010 commented Mar 7, 2025 •
edited
Loading

Extracted from #113256, with additional improvements for boxed structs return.

  • Extend support for newarr, newobj and initobj
  • Recognize {{ldloc.*} + box} + ret

@EgorBo

Copilot AI review requested due to automatic review settings March 7, 2025 17:49
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 7, 2025
Copilot AI reviewed Mar 7, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 7, 2025
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Mar 7, 2025

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

@hez2010
Copy link
Contributor Author

hez2010 commented Mar 7, 2025

@MihuBot

@MihuBot MihuBot mentioned this pull request Mar 7, 2025
Open
@hez2010
Copy link
Contributor Author

hez2010 commented Mar 7, 2025

Diffs from MihuBot look promising to me.

This was referenced Mar 7, 2025
Open
Closed
@hez2010
Copy link
Contributor Author

hez2010 commented Mar 8, 2025

@MichalStrehovsky NativeAOT failures seem like an ilc preexisting bug?

@MichalStrehovsky
Copy link
Member

MichalStrehovsky commented Mar 10, 2025

@MichalStrehovsky NativeAOT failures seem like an ilc preexisting bug?

The scanner needs to precompute everything RyuJIT is going to look at. Looks like in this case RyuJIT decided to inline past a generic recursion cutoff.

Maybe a fix would be to add a call to NodeFactory.TypeSystemContext.DetectGenericCycles(caller, callee) some place like here:

runtime/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs

Lines 101 to 104 in c8f32e5

public bool CanInline(MethodDesc caller, MethodDesc callee)
{
return _inliningPolicy.CanInline(caller, callee);
}

But I'm afraid RyuJIT will ask canInline with something useless like Foo<_Canon> inlining Foo<_Canon> instead of a useful thing like Foo<Gen<Gen<Gen<X>>>. If so, I don't immediately know what the fix could be, besides throwing away the managed scanner and replacing it with RyuJIT.

@JulieLeeMSFT JulieLeeMSFT requested a review from EgorBo April 7, 2025 16:18
@EgorBo
Copy link
Member

EgorBo commented Apr 21, 2025

@MihuBot

@MihuBot MihuBot mentioned this pull request Apr 21, 2025
Open
EgorBo
EgorBo reviewed May 19, 2025
View reviewed changes
src/coreclr/jit/fgbasic.cpp
}

if ((codeAddr < codeEndp - sz) && (OPCODE)getU1LittleEndian(codeAddr + sz) == CEE_RET)
if ((codeAddr < codeEndp - sz) && doesMethodLookLikeWrapper(codeAddr + sz, codeEndp))
Copy link
Member

@EgorBo EgorBo May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given it's called only here, it only supports

call
ldloc
box
ret

?

is this some specific pattern we need to support?

Copy link
Contributor Author

@hez2010 hez2010 May 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope. it's just that doesMethodLookLikeWrapper can return true for a single ret so that it can be reused here.

@hez2010
Copy link
Contributor Author

hez2010 commented Jun 20, 2025

Closing for now as I don't have enough time to investigate the regression currently.

@hez2010 hez2010 closed this Jun 20, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Jul 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@EgorBo EgorBo EgorBo left review comments

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@hez2010 @MichalStrehovsky @EgorBo