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

Unsafe.As doesn't update type information in JIT #84377

Closed
EgorBo opened this issue Apr 5, 2023 · 1 comment · Fixed by #85954
Closed

Unsafe.As doesn't update type information in JIT #84377

EgorBo opened this issue Apr 5, 2023 · 1 comment · Fixed by #85954
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@EgorBo
Copy link
Member

EgorBo commented Apr 5, 2023

Noticed in #84370 (comment)

public class ClassA
{
    public virtual int GetVal() => 1;
}

public sealed class ClassB : ClassA
{
    public override int GetVal() => 42;
}


void GetValUnsafe(ClassA tc) => Unsafe.As<ClassB>(tc).GetVal();

Current codegen:

; Method GetValUnsafe(ClassA):this
       sub      rsp, 40
       mov      rcx, rdx
       mov      rax, qword ptr [rdx]
       mov      rax, qword ptr [rax+48H]
       call     [rax+20H]ClassA:GetVal():int:this
       nop      
       add      rsp, 40
       ret      
; Total bytes of code: 23

Expected codegen:

; Method GetValUnsafe(ClassB):this
       cmp      byte  ptr [rdx], dl
       ret      
; Total bytes of code: 3

The idea that Unsafe.As JIT intrinsic should spill its input to a local and set class for that local based on signature.

case NI_SRCS_UNSAFE_As:
{
assert((sig->sigInst.methInstCount == 1) || (sig->sigInst.methInstCount == 2));
// ldarg.0
// ret
return impPopStack().val;
}

@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 Apr 5, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Apr 5, 2023
@ghost
Copy link

ghost commented Apr 5, 2023

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

Issue Details

Noticed in #84370 (comment)

public class ClassA
{
    public virtual int GetVal() => 1;
}

public sealed class ClassB : ClassA
{
    public override int GetVal() => 42;
}


void GetValUnsafe(ClassA tc) => Unsafe.As<ClassB>(tc).GetVal();

Current codegen:

; Method GetValUnsafe(ClassA):this
       sub      rsp, 40
       mov      rcx, rdx
       mov      rax, qword ptr [rdx]
       mov      rax, qword ptr [rax+48H]
       call     [rax+20H]ClassA:GetVal():int:this
       nop      
       add      rsp, 40
       ret      
; Total bytes of code: 23

Expected codegen:

; Method GetValUnsafe(ClassB):this
       cmp      byte  ptr [rdx], dl
       ret      
; Total bytes of code: 3

The idea that Unsafe.As JIT intrinsic should spill its input to a local and set class for that local based on signature.

Author: EgorBo
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged

Milestone: -

@EgorBo EgorBo removed the untriaged New issue has not been triaged by the area owner label Apr 5, 2023
@EgorBo EgorBo added this to the 8.0.0 milestone Apr 5, 2023
@EgorBo EgorBo self-assigned this Apr 5, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 8, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 9, 2023
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.

1 participant