Crossgen2 shouldn't compile inlineable internal methods

[EgorBo]

If an internal method is inlined at all callsites where it's called - we shouldn't prejit it. It was noticed in #52708 (comment) (cc @jkotas)

Repro:
Create a console library with a single class:

public class MyLibrary
{
    public int Div(int a, int b) => InternalDiv(a, b);

    internal int InternalDiv(int a, int b) => a / b;
}

And prejit it using Checked version of runtime:

.\crossgen2.exe -o:foo.dll -r:*.dll --codegenopt NgenDisasm=* .\ClassLibrary1.dll --parallelism 1

Actual output:

; Assembly listing for method MyLibrary:.ctor():this
; Emitting BLENDED_CODE for X64 CPU with SSE2 - Windows
; ReadyToRun compilation
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; 0 inlinees with PGO data; 1 single block inlinees; 0 inlinees without PGO data
; Final local variable assignments
;
;* V00 this         [V00    ] (  0,  0   )     ref  ->  zero-ref    this class-hnd
;# V01 OutArgs      [V01    ] (  1,  1   )  lclBlk ( 0) [rsp+00H]   "OutgoingArgSpace"
;
; Lcl frame size = 0

G_M54674_IG01:              ;; offset=0000H
						;; bbWeight=1    PerfScore 0.00
G_M54674_IG02:              ;; offset=0000H
       C3                   ret      
						;; bbWeight=1    PerfScore 1.00

; Total bytes of code 1, prolog size 0, PerfScore 1.10, instruction count 1, allocated bytes for code 1 (MethodHash=52a12a6d) for method MyLibrary:.ctor():this
; ============================================================

; Assembly listing for method MyLibrary:InternalDiv(int,int):int:this
; Emitting BLENDED_CODE for X64 CPU with SSE2 - Windows
; ReadyToRun compilation
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; Final local variable assignments
;
;* V00 this         [V00    ] (  0,  0   )     ref  ->  zero-ref    this class-hnd
;  V01 arg1         [V01,T00] (  3,  3   )     int  ->  rdx        
;  V02 arg2         [V02,T01] (  3,  3   )     int  ->   r8        
;# V03 OutArgs      [V03    ] (  1,  1   )  lclBlk ( 0) [rsp+00H]   "OutgoingArgSpace"
;
; Lcl frame size = 0

G_M11215_IG01:              ;; offset=0000H
						;; bbWeight=1    PerfScore 0.00
G_M11215_IG02:              ;; offset=0000H
       8BC2                 mov      eax, edx
       99                   cdq      
       41F7F8               idiv     edx:eax, r8d
						;; bbWeight=1    PerfScore 26.25
G_M11215_IG03:              ;; offset=0006H
       C3                   ret      
						;; bbWeight=1    PerfScore 1.00

; Total bytes of code 7, prolog size 0, PerfScore 27.95, instruction count 4, allocated bytes for code 7 (MethodHash=5009d430) for method MyLibrary:InternalDiv(int,int):int:this
; ============================================================

; Assembly listing for method MyLibrary:Div(int,int):int:this
; Emitting BLENDED_CODE for X64 CPU with SSE2 - Windows
; ReadyToRun compilation
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; 0 inlinees with PGO data; 1 single block inlinees; 0 inlinees without PGO data
; Final local variable assignments
;
;* V00 this         [V00    ] (  0,  0   )     ref  ->  zero-ref    this class-hnd
;  V01 arg1         [V01,T00] (  3,  3   )     int  ->  rdx        
;  V02 arg2         [V02,T01] (  3,  3   )     int  ->   r8        
;# V03 OutArgs      [V03    ] (  1,  1   )  lclBlk ( 0) [rsp+00H]   "OutgoingArgSpace"
;
; Lcl frame size = 0

G_M11912_IG01:              ;; offset=0000H
						;; bbWeight=1    PerfScore 0.00
G_M11912_IG02:              ;; offset=0000H
       8BC2                 mov      eax, edx
       99                   cdq      
       41F7F8               idiv     edx:eax, r8d   ;;;; <--- InternalDiv was inlined here (the only usage)
						;; bbWeight=1    PerfScore 26.25
G_M11912_IG03:              ;; offset=0006H
       C3                   ret      
						;; bbWeight=1    PerfScore 1.00

; Total bytes of code 7, prolog size 0, PerfScore 27.95, instruction count 4, allocated bytes for code 7 (MethodHash=94c3d177) for method MyLibrary:Div(int,int):int:this
; ============================================================

Expected output:

InternalDiv is not precompiled.

[MichalStrehovsky]

Crossgen2 by default goes over everything in the assembly and adds it as a compilation root here:

runtime/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunLibraryRootProvider.cs

Lines 96 to 128 in 969840e

	foreach (MethodDesc method in type.GetAllMethods())
	{
	// Skip methods with no IL
	if (method.IsAbstract)
	continue;
	if (method.IsInternalCall)
	continue;
	MethodDesc methodToRoot = method;
	if (method.HasInstantiation)
	{
	methodToRoot = InstantiateIfPossible(method);
	if (methodToRoot == null)
	continue;
	}
	try
	{
	if (!CorInfoImpl.ShouldSkipCompilation(method))
	{
	CheckCanGenerateMethod(methodToRoot);
	rootProvider.AddCompilationRoot(methodToRoot, rootMinimalDependencies: false, reason: reason);
	}
	}
	catch (TypeSystemException)
	{
	// Individual methods can fail to load types referenced in their signatures.
	// Skip them in library mode since they're not going to be callable.
	continue;
	}
	}

I think if we were to scope this list of roots down to public/protected/virtual members, and whatever is specified as the assembly entrypoint, we would achieve what we need.

Other methods that are called from the rooted methods should be simply added to the compilation graph as the compiler makes transitive progress. Methods that got inlined will be simply ignored.

We might be able to do the same thing on all methods of non-public types.

We could treat assemblies with InternalsVisibleTo specially, or just ignore that problem.

Cc @dotnet/crossgen-contrib

[davidwrighton]

@EgorBo, if you put together such a change please also make private methods which implement interfaces explicitly be considered part of the surface area to precompile. (Also, in addition to this bit of code we have some logic that triggers some compilation of generics which will also need tweaking.

[EgorBo]

From my understanding, I was assigned to triage it :) but I can try if nobody else from the runtime side will pick it up, thanks for the initial pointers to look at!

[davidwrighton]

Well, my comments here really refer to whoever fixes it. Let us know about the triage decision. Its possible we could do this as part of the crossgen2 work.

[MichalStrehovsky]

please also make private methods which implement interfaces explicitly be considered part of the surface area to precompile

I think this would be covered by the "virtual members" part of my description. It's easier to check for.

[EgorBo]

Moving to 7.0.0 as it unlikely to make into 6.0.0

[AndyAyersMS]

Going to remove area-codegen-coreclr as the logic for this is outside of codegen.