JIT: Fold typeof(T).TypeHandle.Value #85804

EgorBo · 2023-05-05T01:02:42Z

It turns out to be an easy fix with all the "frozen" infra we've built previously:

static nint Test()
{
    return typeof(int).TypeHandle.Value;
}

emits:

; Method Test():long
       mov      rax, 0x7FFF9EC7E7E8
       ret      
; Total bytes of code: 11

Addresses #5973 for CoreCLR, might slighly improve GC.Allocate* APIs, see here.
PS: not relevant for NativeAOT since type objects are not frozen

UPD: handles NativeAOT as well:
was:

; Method Program:Test():long
       sub      rsp, 40
       lea      rcx, [(reloc 0x4000000000420450)]
       call     CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE
       mov      rcx, rax
       mov      rax, qword ptr [rax]
       call     [rax+D0H]System.Type:get_TypeHandle():System.RuntimeTypeHandle:this
       nop
       add      rsp, 40
       ret      
; Total bytes of code: 34

now:

; Method Program:Test():long
       lea      rax, [(reloc 0x4000000000421030)]
       ret

ghost · 2023-05-05T01:02:56Z

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

Issue Details

It turns out to be an easy fix with all the "frozen" infra we've built previously:

static nint Test()
{
    return typeof(int).TypeHandle.Value;
}

emits:

; Method Test():long
       mov      rax, 0x7FFF9EC7E7E8
       ret      
; Total bytes of code: 11

Closes #5973, might slighly improve GC.Allocate* APIs, see here.
PS: not relevant for NativeAOT since type objects are not frozen

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

EgorBo · 2023-05-05T01:19:31Z

int[] Foo()
{
    return GC.AllocateUninitializedArray<int>(10000, true);
}

codegen:

; Method Program:Foo():int[]:this
       sub      rsp, 40
       mov      rcx, 0x7FF7A4FAA2A8                                   ;;; type handle
       mov      r8d, 80                                               ;;; gc flags
       mov      edx, 0x2710                                           ;;; 10000 length
       call     System.GC:AllocateNewArray(long,int,int):System.Array ;;; Internal call
       nop
       add      rsp, 40
       ret      
; Total bytes of code: 36

MichalStrehovsky · 2023-05-05T01:22:38Z

Closes #5973

Can we make it not close this?

I think this will be awkward to do like this for NativeAOT even if RuntimeTypes become frozen because there's still a reloc to deal with.

.NET Native did optimize this and it was useful, especially because type handle != RuntimeType for .NET Native or NativeAOT.

We did it at the IL level by just patternmatching and replacing the whole sequence of ldtoken/GetTypeFromHandle/get_Handle with ldtoken.

EgorBo · 2023-05-05T01:23:36Z

Can we make it not close this?

Sure thing! Changed wording

EgorBo · 2023-05-05T01:39:17Z

Current codegen on NativeAOT for return typeof(int).TypeHandle.Value;:

; Method Program:Test():long
       sub      rsp, 40
       lea      rcx, [(reloc 0x4000000000420450)]
       call     CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE
       mov      rcx, rax
       mov      rax, qword ptr [rax]
       call     [rax+D0H]System.Type:get_TypeHandle():System.RuntimeTypeHandle:this
       nop
       add      rsp, 40
       ret      
; Total bytes of code: 34

EgorBo · 2023-05-05T01:43:29Z

@jkotas PTAL, alternatively we can walk all fields in getObjectContent but currently RuntimeType is the only case we handle. We might eventually add delegates there as well though.

src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs

jkotas · 2023-05-05T02:12:42Z

I think it would be better to do this optimization as intrinsic pattern match - to handle the shared generic case, the native AOT case (and the unloadable assemblies case). Is the pattern match difficult to do for some reason?

EgorBo · 2023-05-05T10:42:35Z

I think it would be better to do this optimization as intrinsic pattern match - to handle the shared generic case, the native AOT case (and the unloadable assemblies case). Is the pattern match difficult to do for some reason?

I assume yes? I did this change for delegates and just noticed that it also handles this case - it is effectively 1 line of change + clean up

EgorBo · 2023-05-05T10:44:52Z

for the intrinsic did you mean @GrabYourPitchforks's branch with GrabYourPitchforks@7a6e22e ? Any reason it didn't land as a PR (and is it enough?)

jkotas · 2023-05-05T14:21:42Z

for the intrinsic did you mean @GrabYourPitchforks's branch with GrabYourPitchforks@7a6e22e ?

Yes, something like, but do that for methods on the public surface so that all code out there that wants to convert type to unmanaged pointer can benefit, it is not limited to the internal methods in CoreLib.

EgorBo · 2023-05-05T14:29:51Z

for the intrinsic did you mean @GrabYourPitchforks's branch with GrabYourPitchforks@7a6e22e ?

Yes, something like, but do that for methods on the public surface so that all code out there that wants to convert type to unmanaged pointer can benefit, it is not limited to the internal methods in CoreLib.

It seems like this PR is still needed then. E.g. it can handle

var typ = typeof(T);
...
typ.TypeHandle.Value;

while importer-level intrinsic will give up here

jkotas · 2023-05-05T14:33:40Z

It seems like this PR is still needed then. E.g. it can handle

It does not hurt, but I would be surprised if you can find pattern where it helps in any code out there (after the importer expansion is done).

EgorBo · 2023-05-05T18:12:24Z

@jkotas What we have after import:

STMT00000 ( 0x000[E-] ... 0x00F )
               [000003] I-C-G------                         *  CALL      struct System.RuntimeType:get_TypeHandle():System.RuntimeTypeHandle:this (exactContextHnd=0x00007FF7A7BA11C9)
               [000002] --C-G------ this                    \--*  CALL help ref    CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE
               [000000] H---------- arg0                       \--*  CNS_INT(h) long   0x7ff7a7cfa2a8 class

***** BB01
STMT00001 ( 0x000[E-] ... ??? )
               [000006] -AC--------                         *  ASG       struct (copy)
               [000005] D------N---                         +--*  LCL_VAR   struct<System.RuntimeTypeHandle, 8> V01 loc0         
               [000004] --C--------                         \--*  RET_EXPR  struct(for [000003])

***** BB01
STMT00002 ( 0x010[E-] ... 0x017 )
               [000008] I-C-G------                         *  CALL      long   System.RuntimeTypeHandle:get_Value():long:this (exactContextHnd=0x00007FF7A7D21919)
               [000007] ----------- this                    \--*  LCL_ADDR  byref  V01 loc0         [+0]

***** BB01
STMT00003 ( 0x010[E-] ... ??? )
               [000010] --C--------                         *  RETURN    long  
               [000009] --C--------                         \--*  RET_EXPR  long  (for [000008])

I'm a bit struggling to understand how I can fold this in JIT for CoreCLR without what I have in this PR already - do we want to intrinsify two methods basically (Type.get_TypeHandle to create a struct with gc field inside) and RuntimeTypeHandle:get_Value to read cls handle from that struct (in a late jit phase since we need forward substitution here)? Sounds like a lot of efforts to optimize what we have today:

       mov      rax, 0x2CE80209270
       mov      rax, qword ptr [rax]

to

       mov      rax, 0x7FF7A7CDA2A8

It should be easier for NativeAOT since RuntimeTypeHandle simply a struct with IntPtr and Value just returns that IntPtr, but for CoreCLR it's a lot of efforts for questionable (IMO) reason

EgorBo · 2023-05-05T18:18:51Z

CoreCLR:

public unsafe partial struct RuntimeTypeHandle : IEquatable<RuntimeTypeHandle>, ISerializable
{
    internal RuntimeType m_type; // gc object

    public IntPtr Value => m_type?.m_handle ?? 0;
}

NativeAOT:

public unsafe struct RuntimeTypeHandle : IEquatable<RuntimeTypeHandle>, ISerializable
{
    internal IntPtr RawValue
    {
        get
        {
            return _value;
        }
    }

    private IntPtr _value;
}

So my suggestion is - intrinsify only NativeAOT's Type.get_TypeHandle() and leave CoreCLR with what we have in this PR.

jkotas · 2023-05-05T19:20:50Z

I'm a bit struggling to understand how I can fold this in JIT for CoreCLR without what I have in this PR already - do we want to intrinsify two methods basically.

We sort of have intrinsics on CoreCLR for all this already - RuntimeTypeHandle.GetValueInternal is same/similar to RuntimeTypeHandle.ToIntPtr/RuntimeTypeHandle.Value. We should delete the internal methods for this (RuntimeTypeHandle.GetValueInternal, RuntimeTypeHandle.RawHandle), replace their uses with equivalent public methods and make sure that the public methods (RuntimeTypeHandle.Value, RuntimeTypeHandle.ToIntPtr) are treated as intrinsics where possible.

Ideally, the intrinsic expansion would kick for all cases where we can figure out the raw type.

Shared generic code. static IntPtr Foo<T>() { typeof(T).Handle.Value; } should return the dictionary value, without roundtrip through System.Type.
static IntPtr GetRawRuntimeTypeHandleFromObject(object o) => RuntimeTypeHandle.ToIntPtr(o.GetType().TypeHandle); can compile into mov rax,[rcx] + ret?

Is the spill into local before RuntimeTypeHandle.Value a big problem? If it is a big problem to deal with, I think it would be ok to handle RuntimeTypeHandle.ToIntPtr only and fix all our code to use RuntimeTypeHandle.ToIntPtr instead of RuntimeTypeHandle.Value.

EgorBo · 2023-05-05T20:03:15Z

Pushed a change for NativeAOT, now codegen is:

lea      rax, [(reloc 0x4000000000421030)]

when we know the class handle in JIT, and this for runtime lookup:

mov      rax, qword ptr [rdx]

So from my understanding we're only left with runtime lookup case on CoreCLR and unloadble contexts (ALC). Sadly, in the BCL we have just one use-case (GC.AllocateArray/AllocateUninitializedArray) and, presumably, we're talking about making it around a few % faster only judging by Levi's numbers so it kind of feels that it is not worth the effort?

Is the spill into local before RuntimeTypeHandle.Value a big problem?

We don't have a good liveness analysis in the importer so we'll have to delay this transformation to e.g. morph (after forward sub).

src/coreclr/jit/importercalls.cpp

jkotas · 2023-05-05T20:45:58Z

src/coreclr/jit/importercalls.cpp

+                    lvaSetStruct(structLcl, sig->retTypeClass, false);
+                    GenTree*       realHandle   = op1->AsCall()->gtArgs.GetUserArgByIndex(0)->GetNode();
+                    GenTreeLclFld* handleFld    = gtNewLclFldNode(structLcl, realHandle->TypeGet(), 0);
+                    GenTree*       asgHandleFld = gtNewAssignNode(handleFld, realHandle);


Would it be better to create this as CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPEHANDLE helper call that takes the first arg of op1 as an input?

We have other places in the JIT that look for CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPEHANDLE. I do not think they will be able to recognize this.

It probably makes sense but we'll need to fold it back to a constant somewhere later, I tried to do this and hit no diffs (I'm running ILC for a complex app) so maybe we better teach jit to recognize raw handles if we ever find a use case?

Presumably, there is nothing JIT can do additionally for typeof(T).TypeHandle.Value; if it's compared against a different type handle - it will be folded (if possible) anyway

The canonical shape for creating RuntimeTypeHandle from raw handle used in other places is CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPEHANDLE helper. I think we should stick to canonical shapes where possible, to allow pattern matching optimizations to compose.

Cleaning up CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPEHANDLE in later stages applies to all places that use canonical shape for creating RuntimeTypeHandle from raw handle.

Here is the change you wanted to see EgorBo@55279bc

It seems to be not needed for CoreCLR since we have to wrap RuntimeType object and I've ran SPMI searching for CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPEHANDLE(class handle) and found 0 cases.

I spent some time trying to find a case where this helps and wasn't able to do so, I only see a worse codegen (a stack spill I can't explain) - my understanding that we promote structs right after importer so it's better to expand them there.

I believe that the right way to do this optimization is without any AOT special-casing and without RuntimeType special-casing in getObjectContent. I think you are saying that it is complicated and not worth it, and that you would prefer to either do nothing or do some of the special casing.

@dotnet/jit-contrib Any opinions about this?

Right, I just feel that the energy is better to spent elsewhere since:

Does NativeAOT needs this optimization? It doesn't look so since GC.NativeAOT.cs doesn't use it (although. it's fully implemented in this PR including shared generics)

Is this pattern popular? Only one hit in GC.CoreCLR.cs in our repo + a few hits in OSS judging by grep.app

So we only miss the shared generic case on CoreCLR and unloadable ALC (on CoreCLR). To support both of them we need to mark two methods with [Intrinsic] (Type.get_TypeHandle and RuntimeTypeHandle.get_Value (before it is inlined) - in this case we also will need some hacky way to work with a spilled struct RuntimeTypeHandle before we can detect the shape from two intrinsics.

STMT00000 ( 0x000[E-] ... 0x00F ) [000003] I-C-G------ * CALL struct System.RuntimeType:get_TypeHandle():System.RuntimeTypeHandle:this (exactContextHnd=0x00007FF7A7BA11C9) [000002] --C-G------ this \--* CALL help ref CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE [000000] H---------- arg0 \--* CNS_INT(h) long 0x7ff7a7cfa2a8 class ***** BB01 STMT00001 ( 0x000[E-] ... ??? ) [000006] -AC-------- * ASG struct (copy) [000005] D------N--- +--* LCL_VAR struct<System.RuntimeTypeHandle, 8> V01 loc0 [000004] --C-------- \--* RET_EXPR struct(for [000003]) ***** BB01 STMT00002 ( 0x010[E-] ... 0x017 ) [000008] I-C-G------ * CALL long System.RuntimeTypeHandle:get_Value():long:this (exactContextHnd=0x00007FF7A7D21919) [000007] ----------- this \--* LCL_ADDR byref V01 loc0 [+0] ***** BB01 STMT00003 ( 0x010[E-] ... ??? ) [000010] --C-------- * RETURN long [000009] --C-------- \--* RET_EXPR long (for [000008])

(as a proof that it is not trivial to do anything with this in importer - we need:

make sure the local is single-use

find its def

make sure the def is "before" use (which is possible)
this sort of things we avoid doing in importer

We can sort of do this after importer and ForwardSub but we'll need to disable inlining for them which is not great either.

in this case we also will need some hacky way to work with a spilled struct RuntimeTypeHandle

If it helps, I would be ok with ignoring RuntimeTypeHandle.get_Value and tell people to use RuntimeTypeHandle.ToIntPtr for best results. We have added RuntimeTypeHandle.ToIntPtr/FromIntPtr family of APIs recently, as a managed equivalent of some Mono embedding APIs.

in this case we also will need some hacky way to work with a spilled struct RuntimeTypeHandle

If it helps, I would be ok with ignoring RuntimeTypeHandle.get_Value and tell people to use RuntimeTypeHandle.ToIntPtr for best results. We have added RuntimeTypeHandle.ToIntPtr/FromIntPtr family of APIs recently, as a managed equivalent of some Mono embedding APIs.

Yes, that one is at least do-able in importer, I've just added it

There is some special spill avoidance in the importer to make recognizing these kinds of multi-call patterns easier; you could extend this if needed.

runtime/src/coreclr/jit/importercalls.cpp

Lines 1414 to 1442 in 7d54c05

// For non-candidates we must also spill, since we

// might have locals live on the eval stack that this

// call can modify.

//

// Suppress this for certain well-known call targets

// that we know won't modify locals, eg calls that are

// recognized in gtCanOptimizeTypeEquality. Otherwise

// we may break key fragile pattern matches later on.

bool spillStack = true;

if (call->IsCall())

{

GenTreeCall* callNode = call->AsCall();

if ((callNode->gtCallType == CT_HELPER) && (gtIsTypeHandleToRuntimeTypeHelper(callNode) ||

gtIsTypeHandleToRuntimeTypeHandleHelper(callNode)))

{

spillStack = false;

}

else if ((callNode->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) != 0)

{

spillStack = false;

}

}

if (spillStack)

{

impSpillSideEffects(true, CHECK_SPILL_ALL DEBUGARG("non-inline candidate call"));

}

}

}

EgorBo · 2023-05-05T21:56:21Z

Interesting, there is a test that is written in IL that changes callvirt in typeof(int).TypeHandle.Value; to just call for TypeHandle so it is expected to call Type.TypeHandle that throws an exception 🙂 https://github.com/dotnet/runtime/blob/main/src/tests/Loader/classloader/regressions/dev10_443322/dev10_443322.il#L7

jkotas · 2023-05-06T03:26:10Z

We should delete the internal methods for this (RuntimeTypeHandle.GetValueInternal, RuntimeTypeHandle.RawHandle), replace their uses with equivalent public methods

I have submitted #85861 for this part.

MichalStrehovsky · 2023-05-08T00:21:26Z

We can say "fixes #5973" again. The get_TypeHandle handling looks great. The typeof(Foo).TypeHandle pattern will be common e.g. in the COM interface source generator so I think we'll get a lot of mileage out of it in both R2R and NAOT. Thank you!

jkotas · 2023-05-08T01:54:02Z

Can we also include the simplification of AllocateUninitializedArray in this change?

jkotas · 2023-05-08T14:21:45Z

src/libraries/System.Private.CoreLib/src/System/Type.cs

-        public virtual RuntimeTypeHandle TypeHandle => throw new NotSupportedException();
+        public virtual RuntimeTypeHandle TypeHandle
+        {
+#if NATIVEAOT


Ugh. I thought that the native AOT special casing is not going to be needed. Do we really need it?

It is not needed for RuntimeTypeHandle.ToIntPtr() but it is still needed for old pattern typeof(int).TypeHandle.Value; as @MichalStrehovsky has just mentioned that this pattern is used in COM so I decided to get it back. If we recommend users to always use RuntimeTypeHandle.ToIntPtr I guess we can remove it?

It is not needed for RuntimeTypeHandle.ToIntPtr() but it is still needed for old pattern typeof(int).TypeHandle.Value

RuntimeTypeHandle.ToIntPtr pattern is RuntimeTypeHandle.ToIntPtr(typeof(int).TypeHandle). What makes it work without get_TypeHandle being treated as intrinsic?

In any case, could you please delete the ifdef around the attribute, and centralize the differences in the JIT? Intrinsic annotations are not meant to be 100% precise. We have number of them where the method is treated as Intrinsic on one runtime flavor, but not on the other runtime flavor. We do not encode this detail using ifdefs in .cs files. (In other words, everything should work fine if all CoreLib methods are reported to the JIT as intrinsics.)

RuntimeTypeHandle.ToIntPtr pattern is RuntimeTypeHandle.ToIntPtr(typeof(int).TypeHandle). What makes it work without get_TypeHandle being treated as intrinsic?

For RuntimeTypeHandle.ToIntPtr we don't have to expand get_TypeHandle we only need to know that the arg of ToIntPtr is a "non-expanded get_TypeHandle" call (if it is expanded (only on NAOT) - not a big deal, it will be folded naturally since ToIntPtr is a simple method that returns struct's value so it will be inlined as is.

In any case, could you please delete the ifdef around the attribute, and centralize the differences in the JIT?

Sure

should work fine if all CoreLib methods are reported to the JIT as intrinsics.

I bet it's not the case today as many existing intrinsic expansions just assume (via asserts) that they work with a specific overload

typeof(int).TypeHandle.Value; as @MichalStrehovsky has just mentioned that this pattern is used in COM so I decided to get it back

The pattern in COM generators is typeof(Foo).TypeHandle without the Value part:

runtime/src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/ComClassGenerator.cs

Line 138 in 5eef91d

// details = StrategyBasedComWrappers.DefaultIUnknownInterfaceDetailsStrategy.GetIUnknownDerivedDetails(typeof(<ifaceName>).TypeHandle);

jkotas · 2023-05-08T15:48:09Z

src/coreclr/jit/importercalls.cpp

@@ -2589,7 +2590,7 @@ GenTree* Compiler::impIntrinsic(GenTree*                newobjThis,
            case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant:

            // We need these to be able to fold "typeof(...) == typeof(...)"
-            case NI_System_RuntimeTypeHandle_ToIntPtr:
+            case NI_System_RuntimeType_get_TypeHandle:


NI_System_RuntimeType_get_TypeHandle should not be involved in typeof(...) == typeof(...) folding.

Should NI_System_RuntimeType_get_TypeHandle rather be in the lightweight intrinsics set below?

jkotas

LGTM. Thank you for bearing with me!

Somebody from @dotnet/jit-contrib should signoff on the JIT changes.

src/coreclr/jit/importercalls.cpp

AndyAyersMS · 2023-05-09T02:50:54Z

src/coreclr/jit/importercalls.cpp

+                    lvaSetStruct(structLcl, sig->retTypeClass, false);
+                    GenTree*       realHandle   = op1->AsCall()->gtArgs.GetUserArgByIndex(0)->GetNode();
+                    GenTreeLclFld* handleFld    = gtNewLclFldNode(structLcl, realHandle->TypeGet(), 0);
+                    GenTree*       asgHandleFld = gtNewAssignNode(handleFld, realHandle);


There is some special spill avoidance in the importer to make recognizing these kinds of multi-call patterns easier; you could extend this if needed.

runtime/src/coreclr/jit/importercalls.cpp

Lines 1414 to 1442 in 7d54c05

// For non-candidates we must also spill, since we

// might have locals live on the eval stack that this

// call can modify.

//

// Suppress this for certain well-known call targets

// that we know won't modify locals, eg calls that are

// recognized in gtCanOptimizeTypeEquality. Otherwise

// we may break key fragile pattern matches later on.

bool spillStack = true;

if (call->IsCall())

{

GenTreeCall* callNode = call->AsCall();

if ((callNode->gtCallType == CT_HELPER) && (gtIsTypeHandleToRuntimeTypeHelper(callNode) ||

gtIsTypeHandleToRuntimeTypeHandleHelper(callNode)))

{

spillStack = false;

}

else if ((callNode->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) != 0)

{

spillStack = false;

}

}

if (spillStack)

{

impSpillSideEffects(true, CHECK_SPILL_ALL DEBUGARG("non-inline candidate call"));

}

}

}

src/coreclr/jit/importercalls.cpp

Co-authored-by: Andy Ayers <andya@microsoft.com>

EgorBo · 2023-05-09T22:53:22Z

There is some special spill avoidance in the importer to make recognizing these kinds of multi-call patterns easier; you could extend this if needed.

For some reason I can't reply directly on your comment on github 😐
Thanks, good to know, I see that it already contains the helpers I need

…-value

JIT: Fold typeof(T).TypeHandle.Value

52065e1

dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 5, 2023

ghost assigned EgorBo May 5, 2023

jkotas reviewed May 5, 2023

View reviewed changes

src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs Show resolved Hide resolved

EgorBo force-pushed the fold-typehandle-value branch from 274b8e9 to 52065e1 Compare May 5, 2023 14:38

Handle NativeAOT

04210ec

Clean up

3c81b79

jkotas reviewed May 5, 2023

View reviewed changes

src/coreclr/jit/importercalls.cpp Outdated Show resolved Hide resolved

jkotas reviewed May 5, 2023

View reviewed changes

MihuBot mentioned this pull request May 5, 2023

[EgorBo] JIT: Fold typeof(T).TypeHandle.Value MihuBot/runtime-utils#41

Closed

EgorBo added 2 commits May 5, 2023 23:58

fix test

e4d391b

check callvirt explicitly

375df02

EgorBo added 3 commits May 7, 2023 17:18

Address feedback

7716934

formatting

726a42f

formatting

5b313af

jkotas closed this May 8, 2023

jkotas reopened this May 8, 2023

Address feedback

5bdfc2c

jkotas reviewed May 8, 2023

View reviewed changes

EgorBo added 2 commits May 8, 2023 17:40

address feedback

7821c85

oops, reverted too much

7ab11e5

jkotas reviewed May 8, 2023

View reviewed changes

jkotas approved these changes May 8, 2023

View reviewed changes

Addressed feedback

ec522f2

EgorBo requested review from jakobbotsch and AndyAyersMS May 8, 2023 15:53

build-analysis bot mentioned this pull request May 8, 2023

Could not load file or assembly 'Microsoft.CodeAnalysis.NetAnalyzers #84995

Closed

AndyAyersMS approved these changes May 9, 2023

View reviewed changes

jakobbotsch reviewed May 9, 2023

View reviewed changes

src/coreclr/jit/importercalls.cpp Outdated Show resolved Hide resolved

EgorBo and others added 2 commits May 10, 2023 00:20

Merge branch 'main' into fold-typehandle-value

83a6fae

Update src/coreclr/jit/importercalls.cpp

473a361

Co-authored-by: Andy Ayers <andya@microsoft.com>

EgorBo added 3 commits May 10, 2023 17:16

Merge branch 'main' of github.com:dotnet/runtime into fold-typehandle…

eaeaef5

…-value

formatting

0442988

address feedback

0e4b625

build-analysis bot mentioned this pull request May 10, 2023

Checkout failure: "Git fetch failed with exit code 128" dotnet/arcade#9009

Open

2 tasks

EgorBo merged commit 59893de into dotnet:main May 11, 2023
189 checks passed

EgorBo deleted the fold-typehandle-value branch May 11, 2023 12:35

EgorBo mentioned this pull request May 19, 2023

NativeAOT codegen optimization opportunities #64242

Closed

15 tasks

dotnet locked as resolved and limited conversation to collaborators Jun 10, 2023

	// For non-candidates we must also spill, since we
	// might have locals live on the eval stack that this
	// call can modify.
	//
	// Suppress this for certain well-known call targets
	// that we know won't modify locals, eg calls that are
	// recognized in gtCanOptimizeTypeEquality. Otherwise
	// we may break key fragile pattern matches later on.
	bool spillStack = true;
	if (call->IsCall())
	{
	GenTreeCall* callNode = call->AsCall();
	if ((callNode->gtCallType == CT_HELPER) && (gtIsTypeHandleToRuntimeTypeHelper(callNode) \|\|
	gtIsTypeHandleToRuntimeTypeHandleHelper(callNode)))
	{
	spillStack = false;
	}
	else if ((callNode->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) != 0)
	{
	spillStack = false;
	}
	}

	if (spillStack)
	{
	impSpillSideEffects(true, CHECK_SPILL_ALL DEBUGARG("non-inline candidate call"));
	}
	}
	}

JIT: Fold typeof(T).TypeHandle.Value #85804

JIT: Fold typeof(T).TypeHandle.Value #85804

Conversation

EgorBo commented May 5, 2023 • edited

ghost commented May 5, 2023

EgorBo commented May 5, 2023 • edited

MichalStrehovsky commented May 5, 2023

EgorBo commented May 5, 2023

EgorBo commented May 5, 2023 • edited

EgorBo commented May 5, 2023

jkotas commented May 5, 2023

EgorBo commented May 5, 2023 • edited

EgorBo commented May 5, 2023

jkotas commented May 5, 2023

EgorBo commented May 5, 2023

jkotas commented May 5, 2023

EgorBo commented May 5, 2023 • edited

EgorBo commented May 5, 2023 • edited

jkotas commented May 5, 2023

EgorBo commented May 5, 2023 • edited

Choose a reason for hiding this comment

EgorBo May 5, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EgorBo May 7, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EgorBo May 7, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EgorBo commented May 5, 2023

jkotas commented May 6, 2023

MichalStrehovsky commented May 8, 2023

jkotas commented May 8, 2023

Choose a reason for hiding this comment

EgorBo May 8, 2023 • edited

Choose a reason for hiding this comment

jkotas May 8, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkotas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EgorBo commented May 9, 2023

EgorBo commented May 5, 2023 •

edited

EgorBo commented May 5, 2023 •

edited

EgorBo commented May 5, 2023 •

edited

EgorBo commented May 5, 2023 •

edited

EgorBo commented May 5, 2023 •

edited

EgorBo commented May 5, 2023 •

edited

EgorBo commented May 5, 2023 •

edited

EgorBo May 5, 2023 •

edited

EgorBo May 7, 2023 •

edited

EgorBo May 7, 2023 •

edited

EgorBo May 8, 2023 •

edited

jkotas May 8, 2023 •

edited