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 does not recognize typeof(X) == o.GetType() checks as intrinsics with R2R #97134

Closed
jkotas opened this issue Jan 18, 2024 · 4 comments · Fixed by #97424
Closed

JIT does not recognize typeof(X) == o.GetType() checks as intrinsics with R2R #97134

jkotas opened this issue Jan 18, 2024 · 4 comments · Fixed by #97424
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Comments

@jkotas
Copy link
Member

jkotas commented Jan 18, 2024

Repro:

using System;
using System.Runtime.CompilerServices;

public sealed class Test96876
{
    public static void Main()
    {
        foo<Test96876>(new DateTime[1]);
    }

    // Validate that the type equality involving shared array types is handled correctly
    // in shared generic code.
    [MethodImpl(MethodImplOptions.NoInlining)]
    static bool foo<T>(DateTime[] list) => typeof(T[]) == list.GetType();
}

Actual result:

foo<__Canon> is optimized to return constant for JIT, but does full type check for R2R.

Expected result

foo<__Canon> is optimized to return constant for both JIT and R2R.

Optimized JIT code:

G_M10478_IG02:  ;; offset=0x0000
       cmp      byte  ptr [rdx], dl
       xor      eax, eax
                                                ;; size=4 bbWeight=1 PerfScore 3.25
G_M10478_IG03:  ;; offset=0x0004
       ret

Optimized R2R code:

G_M000_IG01:                ;; offset=0x0000
       push     rbx
       sub      rsp, 48
       mov      qword ptr [rsp+0x28], rcx
       mov      rbx, rdx

G_M000_IG02:                ;; offset=0x000D
       call     [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
       cmp      qword ptr [rbx], rax
       sete     al
       movzx    rax, al

G_M000_IG03:                ;; offset=0x001C
       add      rsp, 48
       pop      rbx
       ret
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jan 18, 2024
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 18, 2024
@ghost
Copy link

ghost commented Jan 18, 2024

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

Issue Details

Repro:

!!!

Actual result:

foo<__Canon> is optimized to return constant for JIT, but does full type check for R2R.

Expected result

foo<__Canon> is optimized to return constant for both JIT and R2R.

Optimized JIT code:

G_M10478_IG02:  ;; offset=0x0000
       cmp      byte  ptr [rdx], dl
       xor      eax, eax
                                                ;; size=4 bbWeight=1 PerfScore 3.25
G_M10478_IG03:  ;; offset=0x0004
       ret

Optimized R2R code:

G_M000_IG01:                ;; offset=0x0000
       push     rbx
       sub      rsp, 48
       mov      qword ptr [rsp+0x28], rcx
       mov      rbx, rdx

G_M000_IG02:                ;; offset=0x000D
       call     [CORINFO_HELP_READYTORUN_GENERIC_HANDLE]
       cmp      qword ptr [rbx], rax
       sete     al
       movzx    rax, al

G_M000_IG03:                ;; offset=0x001C
       add      rsp, 48
       pop      rbx
       ret
Author: jkotas
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged

Milestone: -

@EgorBo
Copy link
Member

EgorBo commented Jan 18, 2024

Interesting, doesn't repro for me if I set --inputbubble in crossgen.

@EgorBo
Copy link
Member

EgorBo commented Jan 18, 2024

the issue is that impIsClassExact(cls) returns false for DateTime[] class.
looks like getClassAttribs(DateTime[]) returns CORINFO_FLG_VAROBJSIZE | CORINFO_FLG_ARRAY. If I set --inputbubble then it's CORINFO_FLG_FINAL | CORINFO_FLG_ARRAY

that CORINFO_FLG_VAROBJSIZE makes the difference.

@jkotas
Copy link
Member Author

jkotas commented Jan 18, 2024

that CORINFO_FLG_VAROBJSIZE makes the difference.

I assume that you meant CORINFO_FLG_FINAL. We may be able to return CORINFO_FLG_FINAL for R2R. However, the larger problem is that impIsClassExact logic is not right. It is hard to synthetize these type properties on the JIT side. The logic has both false positives (functional bugs) and false negatives (perf bugs). Repro for false positive:

C:\>git clone https://github.com/jkotas/EquivalentTypes
Cloning into 'EquivalentTypes'...
remote: Enumerating objects: 17, done.
remote: Counting objects: 100% (17/17), done.
remote: Compressing objects: 100% (14/14), done.
remote: Total 17 (delta 2), reused 17 (delta 2), pack-reused 0
Receiving objects: 100% (17/17), done.
Resolving deltas: 100% (2/2), done.

C:\>cd EquivalentTypes

C:\EquivalentTypes>dotnet publish
MSBuild version 17.8.3+195e7f5a3 for .NET
  Determining projects to restore...
  Restored C:\EquivalentTypes\test.csproj (in 433 ms).
  Restored C:\EquivalentTypes\pia\pia.csproj (in 408 ms).
  Restored C:\EquivalentTypes\mylib\mylib.csproj (in 432 ms).
  pia -> C:\EquivalentTypes\pia\bin\Release\net8.0\pia.dll
  mylib -> C:\EquivalentTypes\mylib\bin\Release\net8.0\mylib.dll
  test -> C:\EquivalentTypes\bin\Release\net8.0\test.dll
  test -> C:\EquivalentTypes\bin\Release\net8.0\publish\

C:\EquivalentTypes>bin\Release\net8.0\publish\test.exe
True

C:\EquivalentTypes>set DOTNET_TieredCompilation=0

C:\EquivalentTypes>bin\Release\net8.0\publish\test.exe
False

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 23, 2024
jkotas added a commit to jkotas/runtime that referenced this issue Jan 23, 2024
The approximation of isExactType from class flags had false positives (correctness) and false negatives issues. Converting it to JIT/EE interface method fixes them both.

Fixes dotnet#97134
jkotas added a commit that referenced this issue Jan 24, 2024
* Convert isExactType check to JIT/EE interface call

The approximation of isExactType from class flags had false positives (correctness) and false negatives issues. Converting it to JIT/EE interface method fixes them both.

Fixes #97134

* Add test

* Delete canInlineTypeCheck JIT/EE interface method

All implementations return the same constant. Unlikely to be needed again.
@ghost ghost removed in-pr There is an active PR which will close this issue when it is merged untriaged New issue has not been triaged by the area owner labels Jan 24, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Feb 24, 2024
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

Successfully merging a pull request may close this issue.

2 participants