JIT: Update class for Unsafe.As<>()

[EgorBo]

We used to import T Unsafe.As<T>(object o) where T : class as a no-op and didn't benefit from a more exact T hint.

Closes#84377 (cc @stephentoub)
Closes#85906 (cc @MichalStrehovsky) - checked that it also devirtualizes the interface call

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

[ghost]

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

Issue Details

---

We used to import T Unsafe.As<T>(object o) where T : class as no-op basically and ignored T.

Closes #84377 (cc @stephentoub)
Closes #85906 (cc @MichalStrehovsky)

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

Author:	EgorBo
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[jkotas]

I think you want to check for CORINFO_FLG_SHAREDINST flag here.

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

[EgorBo]

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

Thanks, I was wondering about that too. I though that since I only need it here I can request that on demand, will change the original sig.

[EgorBo]

@jkotas it seems that we're losing the exact sig due to this line of getCallInfo:

runtime/src/coreclr/vm/jitinterface.cpp

Line 5232 in 231314d

pTargetMD = pTargetMD->GetWrappedMethodDesc();

this wrapped pMD is then used for getMethodSigInternal does it look correct to you?

[EgorBo]

Myabe it should be here if (pTargetMD->IsWrapperStub()) ?

[jkotas]

Does it work with the last update, or do you still need help with this?

[EgorBo]

Does it work with the last update, or do you still need help with this?

Still need help, the current impl doesn't work since we always have System._Canon as methodInst[0], I just prepared the code to pick up a fix in the getCallInfo

[jkotas]

getCallInfo implementation is correct. getCallInfo returns information about how the method should be called. Unsafe.As<object> should be called as Unsafe.As<Canon> with hidden argument, so it is that's what getCallInfo returns, including signature for Unsafe.As<Canon>. "Fixing" getCallInfo to return some kind of different signature would require reviewing all places that use the signature returned by getCallInfo and making sure that they behave correctly.

I think you want to take the method handle from CORINFO_RESOLVED_TOKEN and call getMethodSig on that. It should return what you are looking for.

[EgorBo]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

[jkotas]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

Yes - to fix the issues that you have linked.

A similar approach can be probably used to improve the type precision in other places, but that can be left for future.

[jkotas]

This is going to make us skip e.g. Unsafe.As<List<Canon>>. We do not want that.

Delete this early out and just depend on isMoreSpecificType to see whether the type is more exact.

[EgorBo]

Thanks, addressed

[EgorBo]

Failure is #35066

[EgorBo]

We used to import T Unsafe.As<T>(object o) where T : class as a no-op and didn't benefit from a more exact T hint.

Closes#84377 (cc @stephentoub)
Closes#85906 (cc @MichalStrehovsky) - checked that it also devirtualizes the interface call

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

[ghost]

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

Issue Details

---

We used to import T Unsafe.As<T>(object o) where T : class as no-op basically and ignored T.

Closes #84377 (cc @stephentoub)
Closes #85906 (cc @MichalStrehovsky)

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

Author:	EgorBo
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[jkotas]

I think you want to check for CORINFO_FLG_SHAREDINST flag here.

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

[EgorBo]

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

Thanks, I was wondering about that too. I though that since I only need it here I can request that on demand, will change the original sig.

[EgorBo]

@jkotas it seems that we're losing the exact sig due to this line of getCallInfo:

runtime/src/coreclr/vm/jitinterface.cpp

Line 5232 in 231314d

pTargetMD = pTargetMD->GetWrappedMethodDesc();

this wrapped pMD is then used for getMethodSigInternal does it look correct to you?

[EgorBo]

Myabe it should be here if (pTargetMD->IsWrapperStub()) ?

[jkotas]

Does it work with the last update, or do you still need help with this?

[EgorBo]

Does it work with the last update, or do you still need help with this?

Still need help, the current impl doesn't work since we always have System._Canon as methodInst[0], I just prepared the code to pick up a fix in the getCallInfo

[jkotas]

getCallInfo implementation is correct. getCallInfo returns information about how the method should be called. Unsafe.As<object> should be called as Unsafe.As<Canon> with hidden argument, so it is that's what getCallInfo returns, including signature for Unsafe.As<Canon>. "Fixing" getCallInfo to return some kind of different signature would require reviewing all places that use the signature returned by getCallInfo and making sure that they behave correctly.

I think you want to take the method handle from CORINFO_RESOLVED_TOKEN and call getMethodSig on that. It should return what you are looking for.

[EgorBo]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

[jkotas]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

Yes - to fix the issues that you have linked.

A similar approach can be probably used to improve the type precision in other places, but that can be left for future.

[jkotas]

This is going to make us skip e.g. Unsafe.As<List<Canon>>. We do not want that.

Delete this early out and just depend on isMoreSpecificType to see whether the type is more exact.

[EgorBo]

Thanks, addressed

[EgorBo]

Failure is #35066

[EgorBo]

We used to import T Unsafe.As<T>(object o) where T : class as a no-op and didn't benefit from a more exact T hint.

Closes#84377 (cc @stephentoub)
Closes#85906 (cc @MichalStrehovsky) - checked that it also devirtualizes the interface call

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

[ghost]

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

Issue Details

---

We used to import T Unsafe.As<T>(object o) where T : class as no-op basically and ignored T.

Closes #84377 (cc @stephentoub)
Closes #85906 (cc @MichalStrehovsky)

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

Author:	EgorBo
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[jkotas]

I think you want to check for CORINFO_FLG_SHAREDINST flag here.

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

[EgorBo]

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

Thanks, I was wondering about that too. I though that since I only need it here I can request that on demand, will change the original sig.

[EgorBo]

@jkotas it seems that we're losing the exact sig due to this line of getCallInfo:

runtime/src/coreclr/vm/jitinterface.cpp

Line 5232 in 231314d

pTargetMD = pTargetMD->GetWrappedMethodDesc();

this wrapped pMD is then used for getMethodSigInternal does it look correct to you?

[EgorBo]

Myabe it should be here if (pTargetMD->IsWrapperStub()) ?

[jkotas]

Does it work with the last update, or do you still need help with this?

[EgorBo]

Does it work with the last update, or do you still need help with this?

Still need help, the current impl doesn't work since we always have System._Canon as methodInst[0], I just prepared the code to pick up a fix in the getCallInfo

[jkotas]

getCallInfo implementation is correct. getCallInfo returns information about how the method should be called. Unsafe.As<object> should be called as Unsafe.As<Canon> with hidden argument, so it is that's what getCallInfo returns, including signature for Unsafe.As<Canon>. "Fixing" getCallInfo to return some kind of different signature would require reviewing all places that use the signature returned by getCallInfo and making sure that they behave correctly.

I think you want to take the method handle from CORINFO_RESOLVED_TOKEN and call getMethodSig on that. It should return what you are looking for.

[EgorBo]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

[jkotas]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

Yes - to fix the issues that you have linked.

A similar approach can be probably used to improve the type precision in other places, but that can be left for future.

[jkotas]

This is going to make us skip e.g. Unsafe.As<List<Canon>>. We do not want that.

Delete this early out and just depend on isMoreSpecificType to see whether the type is more exact.

[EgorBo]

Thanks, addressed

[EgorBo]

Failure is #35066

[EgorBo]

We used to import T Unsafe.As<T>(object o) where T : class as a no-op and didn't benefit from a more exact T hint.

Closes#84377 (cc @stephentoub)
Closes#85906 (cc @MichalStrehovsky) - checked that it also devirtualizes the interface call

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

[ghost]

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

Issue Details

---

We used to import T Unsafe.As<T>(object o) where T : class as no-op basically and ignored T.

Closes #84377 (cc @stephentoub)
Closes #85906 (cc @MichalStrehovsky)

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

Author:	EgorBo
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[jkotas]

I think you want to check for CORINFO_FLG_SHAREDINST flag here.

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

[EgorBo]

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

Thanks, I was wondering about that too. I though that since I only need it here I can request that on demand, will change the original sig.

[EgorBo]

@jkotas it seems that we're losing the exact sig due to this line of getCallInfo:

runtime/src/coreclr/vm/jitinterface.cpp

Line 5232 in 231314d

pTargetMD = pTargetMD->GetWrappedMethodDesc();

this wrapped pMD is then used for getMethodSigInternal does it look correct to you?

[EgorBo]

Myabe it should be here if (pTargetMD->IsWrapperStub()) ?

[jkotas]

Does it work with the last update, or do you still need help with this?

[EgorBo]

Does it work with the last update, or do you still need help with this?

Still need help, the current impl doesn't work since we always have System._Canon as methodInst[0], I just prepared the code to pick up a fix in the getCallInfo

[jkotas]

getCallInfo implementation is correct. getCallInfo returns information about how the method should be called. Unsafe.As<object> should be called as Unsafe.As<Canon> with hidden argument, so it is that's what getCallInfo returns, including signature for Unsafe.As<Canon>. "Fixing" getCallInfo to return some kind of different signature would require reviewing all places that use the signature returned by getCallInfo and making sure that they behave correctly.

I think you want to take the method handle from CORINFO_RESOLVED_TOKEN and call getMethodSig on that. It should return what you are looking for.

[EgorBo]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

[jkotas]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

Yes - to fix the issues that you have linked.

A similar approach can be probably used to improve the type precision in other places, but that can be left for future.

[jkotas]

This is going to make us skip e.g. Unsafe.As<List<Canon>>. We do not want that.

Delete this early out and just depend on isMoreSpecificType to see whether the type is more exact.

[EgorBo]

Thanks, addressed

[EgorBo]

Failure is #35066

[EgorBo]

We used to import T Unsafe.As<T>(object o) where T : class as a no-op and didn't benefit from a more exact T hint.

Closes#84377 (cc @stephentoub)
Closes#85906 (cc @MichalStrehovsky) - checked that it also devirtualizes the interface call

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

[ghost]

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

Issue Details

---

We used to import T Unsafe.As<T>(object o) where T : class as no-op basically and ignored T.

Closes #84377 (cc @stephentoub)
Closes #85906 (cc @MichalStrehovsky)

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

Author:	EgorBo
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[jkotas]

I think you want to check for CORINFO_FLG_SHAREDINST flag here.

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

[EgorBo]

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

Thanks, I was wondering about that too. I though that since I only need it here I can request that on demand, will change the original sig.

[EgorBo]

@jkotas it seems that we're losing the exact sig due to this line of getCallInfo:

runtime/src/coreclr/vm/jitinterface.cpp

Line 5232 in 231314d

pTargetMD = pTargetMD->GetWrappedMethodDesc();

this wrapped pMD is then used for getMethodSigInternal does it look correct to you?

[EgorBo]

Myabe it should be here if (pTargetMD->IsWrapperStub()) ?

[jkotas]

Does it work with the last update, or do you still need help with this?

[EgorBo]

Does it work with the last update, or do you still need help with this?

Still need help, the current impl doesn't work since we always have System._Canon as methodInst[0], I just prepared the code to pick up a fix in the getCallInfo

[jkotas]

getCallInfo implementation is correct. getCallInfo returns information about how the method should be called. Unsafe.As<object> should be called as Unsafe.As<Canon> with hidden argument, so it is that's what getCallInfo returns, including signature for Unsafe.As<Canon>. "Fixing" getCallInfo to return some kind of different signature would require reviewing all places that use the signature returned by getCallInfo and making sure that they behave correctly.

I think you want to take the method handle from CORINFO_RESOLVED_TOKEN and call getMethodSig on that. It should return what you are looking for.

[EgorBo]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

[jkotas]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

Yes - to fix the issues that you have linked.

A similar approach can be probably used to improve the type precision in other places, but that can be left for future.

[jkotas]

This is going to make us skip e.g. Unsafe.As<List<Canon>>. We do not want that.

Delete this early out and just depend on isMoreSpecificType to see whether the type is more exact.

[EgorBo]

Thanks, addressed

[EgorBo]

Failure is #35066

[EgorBo]

We used to import T Unsafe.As<T>(object o) where T : class as a no-op and didn't benefit from a more exact T hint.

Closes#84377 (cc @stephentoub)
Closes#85906 (cc @MichalStrehovsky) - checked that it also devirtualizes the interface call

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

[ghost]

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

Issue Details

---

We used to import T Unsafe.As<T>(object o) where T : class as no-op basically and ignored T.

Closes #84377 (cc @stephentoub)
Closes #85906 (cc @MichalStrehovsky)

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

Author:	EgorBo
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[jkotas]

I think you want to check for CORINFO_FLG_SHAREDINST flag here.

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

[EgorBo]

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

Thanks, I was wondering about that too. I though that since I only need it here I can request that on demand, will change the original sig.

[EgorBo]

@jkotas it seems that we're losing the exact sig due to this line of getCallInfo:

runtime/src/coreclr/vm/jitinterface.cpp

Line 5232 in 231314d

pTargetMD = pTargetMD->GetWrappedMethodDesc();

this wrapped pMD is then used for getMethodSigInternal does it look correct to you?

[EgorBo]

Myabe it should be here if (pTargetMD->IsWrapperStub()) ?

[jkotas]

Does it work with the last update, or do you still need help with this?

[EgorBo]

Does it work with the last update, or do you still need help with this?

Still need help, the current impl doesn't work since we always have System._Canon as methodInst[0], I just prepared the code to pick up a fix in the getCallInfo

[jkotas]

getCallInfo implementation is correct. getCallInfo returns information about how the method should be called. Unsafe.As<object> should be called as Unsafe.As<Canon> with hidden argument, so it is that's what getCallInfo returns, including signature for Unsafe.As<Canon>. "Fixing" getCallInfo to return some kind of different signature would require reviewing all places that use the signature returned by getCallInfo and making sure that they behave correctly.

I think you want to take the method handle from CORINFO_RESOLVED_TOKEN and call getMethodSig on that. It should return what you are looking for.

[EgorBo]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

[jkotas]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

Yes - to fix the issues that you have linked.

A similar approach can be probably used to improve the type precision in other places, but that can be left for future.

[jkotas]

This is going to make us skip e.g. Unsafe.As<List<Canon>>. We do not want that.

Delete this early out and just depend on isMoreSpecificType to see whether the type is more exact.

[EgorBo]

Thanks, addressed

[EgorBo]

Failure is #35066

[EgorBo]

We used to import T Unsafe.As<T>(object o) where T : class as a no-op and didn't benefit from a more exact T hint.

Closes#84377 (cc @stephentoub)
Closes#85906 (cc @MichalStrehovsky) - checked that it also devirtualizes the interface call

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

[ghost]

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

Issue Details

---

We used to import T Unsafe.As<T>(object o) where T : class as no-op basically and ignored T.

Closes #84377 (cc @stephentoub)
Closes #85906 (cc @MichalStrehovsky)

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

Author:	EgorBo
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[jkotas]

I think you want to check for CORINFO_FLG_SHAREDINST flag here.

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

[EgorBo]

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

Thanks, I was wondering about that too. I though that since I only need it here I can request that on demand, will change the original sig.

[EgorBo]

@jkotas it seems that we're losing the exact sig due to this line of getCallInfo:

runtime/src/coreclr/vm/jitinterface.cpp

Line 5232 in 231314d

pTargetMD = pTargetMD->GetWrappedMethodDesc();

this wrapped pMD is then used for getMethodSigInternal does it look correct to you?

[EgorBo]

Myabe it should be here if (pTargetMD->IsWrapperStub()) ?

[jkotas]

Does it work with the last update, or do you still need help with this?

[EgorBo]

Does it work with the last update, or do you still need help with this?

Still need help, the current impl doesn't work since we always have System._Canon as methodInst[0], I just prepared the code to pick up a fix in the getCallInfo

[jkotas]

getCallInfo implementation is correct. getCallInfo returns information about how the method should be called. Unsafe.As<object> should be called as Unsafe.As<Canon> with hidden argument, so it is that's what getCallInfo returns, including signature for Unsafe.As<Canon>. "Fixing" getCallInfo to return some kind of different signature would require reviewing all places that use the signature returned by getCallInfo and making sure that they behave correctly.

I think you want to take the method handle from CORINFO_RESOLVED_TOKEN and call getMethodSig on that. It should return what you are looking for.

[EgorBo]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

[jkotas]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

Yes - to fix the issues that you have linked.

A similar approach can be probably used to improve the type precision in other places, but that can be left for future.

[jkotas]

This is going to make us skip e.g. Unsafe.As<List<Canon>>. We do not want that.

Delete this early out and just depend on isMoreSpecificType to see whether the type is more exact.

[EgorBo]

Thanks, addressed

[EgorBo]

Failure is #35066

[EgorBo]

We used to import T Unsafe.As<T>(object o) where T : class as a no-op and didn't benefit from a more exact T hint.

Closes#84377 (cc @stephentoub)
Closes#85906 (cc @MichalStrehovsky) - checked that it also devirtualizes the interface call

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

[ghost]

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

Issue Details

---

We used to import T Unsafe.As<T>(object o) where T : class as no-op basically and ignored T.

Closes #84377 (cc @stephentoub)
Closes #85906 (cc @MichalStrehovsky)

publicclassClassA{publicvirtualintGetVal()=>1;}publicsealedclassClassB:ClassA{publicoverrideintGetVal()=>42;}voidGetValUnsafe(ClassAtc)=>Unsafe.As<ClassB>(tc).GetVal();

Old codegen:

; Method GetValUnsafe(ClassA):thissubrsp,40movrcx,rdxmovrax, qword ptr [rdx]movrax, qword ptr [rax+48H]call[rax+20H]ClassA:GetVal():int:thisnopaddrsp,40ret; Total bytes of code: 23

New codegen:

; Method GetValUnsafe(ClassB):thiscmp byte ptr [rdx],dlret; Total bytes of code: 3

Author:	EgorBo
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[jkotas]

I think you want to check for CORINFO_FLG_SHAREDINST flag here.

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

[EgorBo]

I do not think you need to re-resolve the signature when it is not shared. sig->sigInst.methInst[0] should have the exact type for the non-shared case.

Thanks, I was wondering about that too. I though that since I only need it here I can request that on demand, will change the original sig.

[EgorBo]

@jkotas it seems that we're losing the exact sig due to this line of getCallInfo:

runtime/src/coreclr/vm/jitinterface.cpp

Line 5232 in 231314d

pTargetMD = pTargetMD->GetWrappedMethodDesc();

this wrapped pMD is then used for getMethodSigInternal does it look correct to you?

[EgorBo]

Myabe it should be here if (pTargetMD->IsWrapperStub()) ?

[jkotas]

Does it work with the last update, or do you still need help with this?

[EgorBo]

Does it work with the last update, or do you still need help with this?

Still need help, the current impl doesn't work since we always have System._Canon as methodInst[0], I just prepared the code to pick up a fix in the getCallInfo

[jkotas]

getCallInfo implementation is correct. getCallInfo returns information about how the method should be called. Unsafe.As<object> should be called as Unsafe.As<Canon> with hidden argument, so it is that's what getCallInfo returns, including signature for Unsafe.As<Canon>. "Fixing" getCallInfo to return some kind of different signature would require reviewing all places that use the signature returned by getCallInfo and making sure that they behave correctly.

I think you want to take the method handle from CORINFO_RESOLVED_TOKEN and call getMethodSig on that. It should return what you are looking for.

[EgorBo]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

[jkotas]

Ah I see, thanks! I assume we only need to do that for Unsafe.As?

Yes - to fix the issues that you have linked.

A similar approach can be probably used to improve the type precision in other places, but that can be left for future.

[jkotas]

This is going to make us skip e.g. Unsafe.As<List<Canon>>. We do not want that.

Delete this early out and just depend on isMoreSpecificType to see whether the type is more exact.

[EgorBo]

Thanks, addressed

[EgorBo]

Failure is #35066