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

typeof(T).IsValueType isn't fully optimized for reference types T #32094

Closed
GrabYourPitchforks opened this issue Feb 11, 2020 · 5 comments
Closed
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Comments

@GrabYourPitchforks
Copy link
Member

Related: #1157

It looks like there are still some cases where typeof(T).IsValueType isn't fully optimized by the JIT. Repro:

static void Main(string[] args)
{
    while (true)
    {
        ReturnIsValueType<bool>();
        ReturnIsValueType<bool?>();
        ReturnIsValueType<string>();
    }
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static bool ReturnIsValueType<T>()
{
    return typeof(T).IsValueType;
}

For value types bool and bool? this is optimized as expected.

; for T = bool
00007ffd`51100bf0 b801000000      mov     eax,1
00007ffd`51100bf5 c3              ret

; for T = Nullable<bool>
00007ffd`51100c10 b801000000      mov     eax,1
00007ffd`51100c15 c3              ret

But for string there's still some throwaway work involving the passed type parameter and a stack spill before eventually returning the constant value false.

; for T = string
00007ffd`51100c30 50              push    rax
00007ffd`51100c31 48890c24        mov     qword ptr [rsp],rcx
00007ffd`51100c35 33c0            xor     eax,eax
00007ffd`51100c37 4883c408        add     rsp,8
00007ffd`51100c3b c3              ret
@GrabYourPitchforks GrabYourPitchforks added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 11, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Feb 11, 2020
@EgorBo
Copy link
Member

EgorBo commented Feb 11, 2020

For ReturnIsValueTypeForString

static bool ReturnIsValueTypeForString() => ReturnIsValueType<string>();

static bool ReturnIsValueType<T>() => typeof(T).IsValueType;

(when inlining is involved)
Emits

       xor      eax, eax
       ret      

@GrabYourPitchforks
Copy link
Member Author

Yeah, things are inlined cleanly. The unexpected codegen only occurs when inlining is suppressed.

@AndyAyersMS
Copy link
Member

The codegen for ref type T is by design -- for shared generic methods the jit has to make the generic context (passed in rcx, here) available to the runtime. You can see this in the gc info dump:

Set generic instantiation context stack slot to -16, type is MD.

I suppose one could argue this is unnecessary in a leaf method with no GC safe regions; I don't know the runtime well enough to say for sure.

@AndyAyersMS AndyAyersMS removed the untriaged New issue has not been triaged by the area owner label Feb 13, 2020
@GrabYourPitchforks
Copy link
Member Author

@AndyAyersMS I understand why the type information has to be passed in the rcx register, but why in the method implementation is it being pushed onto the stack if it's never used?

@AndyAyersMS
Copy link
Member

It is implicitly used by the runtime.

The location is conveyed from the jit to the runtime by the GC info, and the only supported reporting location is on the stack.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
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
Projects
None yet
Development

No branches or pull requests

4 participants