Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

JIT: remove boxing for interface call to shared generic struct #17006

Merged

Conversation

AndyAyersMS
Copy link
Member

Improve the jit's ability to optimize a box-interface call sequence
to handle cases where the unboxed entry point requires a method table
argument.

Added two test cases, one from the inspiring issue, and another where
the jit is then able to inline the method.

Closes #16982.

Improve the jit's ability to optimize a box-interface call sequence
to handle cases where the unboxed entry point requires a method table
argument.

Added two test cases, one from the inspiring issue, and another where
the jit is then able to inline the method.

Closes #16982.
@AndyAyersMS
Copy link
Member Author

@briansull PTAL
cc @dotnet/jit-contrib

No diffs seen in corelib or frameworks. Added a couple new test cases. Removes the allocation from the original example.

For the simple test case, we can also inline since the root method is not shared:

using System;

interface IPrint 
{
    void Print();
}

struct X<T> : IPrint
{
    public X(T t) { _t = t; }
    public void Print() { Console.WriteLine(_t); }
    T _t;
}

class Y
{
    static int Main()
    {
        var s = new X<string>("hello, world!");
        ((IPrint)s).Print();
        return 100;
    }
}
 ; Assembly listing for method Y:Main():int
 ; Emitting BLENDED_CODE for X64 CPU with AVX
 ; optimized code
 ; Final local variable assignments
 ;
 ;* V00 tmp0         [V00    ] (  0,  0   )  struct ( 8) zero-ref   
-;  V01 tmp1         [V01,T00] (  3,  6   )     ref  ->  rdi         class-hnd exact
-;  V02 tmp2         [V02,T01] (  2,  2   )     ref  ->  rsi         V00._t(offs=0x00) P-INDEP
-;  V03 OutArgs      [V03    ] (  1,  1   )  lclBlk (32) [rsp+0x00]  
+;* V01 tmp1         [V01    ] (  0,  0   )  struct ( 8) zero-ref    class-hnd exact
+;  V02 tmp2         [V02,T00] (  2,  2   )     ref  ->  rcx         V00._t(offs=0x00) P-INDEP
+;* V03 tmp3         [V03    ] (  0,  0   )     ref  ->  zero-ref    V01._t(offs=0x00) P-INDEP
+;  V04 OutArgs      [V04    ] (  1,  1   )  lclBlk (32) [rsp+0x00]  
 ;
 ; Lcl frame size = 40
 
-G_M42925_IG01:
-       57                   push     rdi
-       56                   push     rsi
+G_M42926_IG01:
        4883EC28             sub      rsp, 40
 
-G_M42925_IG02:
-       48B968300090BC010000 mov      rcx, 0x1BC90003068
-       488B31               mov      rsi, gword ptr [rcx]
-       48B93858551BFA7F0000 mov      rcx, 0x7FFA1B555838
-       E8DE2B835F           call     CORINFO_HELP_NEWSFAST
-       488BF8               mov      rdi, rax
-       488D4F08             lea      rcx, bword ptr [rdi+8]
-       488BD6               mov      rdx, rsi
-       E86F3B6D5F           call     CORINFO_HELP_ASSIGN_REF
-       488BCF               mov      rcx, rdi
-       E807FCFFFF           call     X`1[__Canon][System.__Canon]:Print():this
+G_M42926_IG02:
+       48B96830616498020000 mov      rcx, 0x29864613068
+       488B09               mov      rcx, gword ptr [rcx]
+       E89AFCFFFF           call     System.Console:WriteLine(ref)
        B864000000           mov      eax, 100
 
-G_M42925_IG03:
+G_M42926_IG03:
        4883C428             add      rsp, 40
-       5E                   pop      rsi
-       5F                   pop      rdi
        C3                   ret      
 
-; Total bytes of code 69, prolog size 6 for method Y:Main():int
+; Total bytes of code 32, prolog size 4 for method Y:Main():int

@stephentoub
Copy link
Member

Thanks, @AndyAyersMS!

@AndyAyersMS
Copy link
Member Author

Don't thank me just yet. Need to figure out what is making x86 so unhappy.

@AndyAyersMS
Copy link
Member Author

A couple of the arm failures look like infrastructure issues; retrying.

@dotnet-bot retest Windows_NT arm64 Cross Checked Innerloop Build and Test
@dotnet-bot retest Windows_NT arm Cross Checked Innerloop Build and Test

The armlb failure is known and tracked via #16961.

Copy link

@briansull briansull left a comment

Choose a reason for hiding this comment

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

Looks Good

@stephentoub
Copy link
Member

Don't thank me just yet.

Can I thank you now? :)

@AndyAyersMS
Copy link
Member Author

Probably? x86 seems happy, and it is the only one using L2R.

@stephentoub
Copy link
Member

Then, thanks 😄

@AndyAyersMS
Copy link
Member Author

armlb failure known (#16961); tizen will almost certainly time out once it runs.

@AndyAyersMS
Copy link
Member Author

Tizen seems to have gotten lost so will retrigger even though it will almost surely fail.

@dotnet-bot retest Tizen armel Cross Checked Innerloop Build and Test

@AndyAyersMS AndyAyersMS merged commit a3e9b7f into dotnet:master Mar 20, 2018
@AndyAyersMS AndyAyersMS deleted the UnboxedEntryNeedsMethodTableArg branch March 20, 2018 15:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants