-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Remove getTypeForBoxOnStack from jit interface #114754
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
Conversation
Along with the `StackAllocatedBox` type. The JIT can now represent on-stack boxes directly via class layouts. Closes dotnet#114204
@jakobbotsch PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems: Language not supported
Comments suppressed due to low confidence (1)
src/coreclr/inc/corinfo.h:2481
- Confirm that all implementations of the ICorStaticInfo interface are updated to remove getTypeForBoxOnStack, ensuring interface consistency following its deletion.
virtual CORINFO_CLASS_HANDLE getTypeForBoxOnStack(CORINFO_CLASS_HANDLE cls) = 0;
@@ -2623,7 +2608,7 @@ private static uint _getJitFlags(IntPtr thisHandle, IntPtr* ppException, CORJIT_ | |||
|
|||
private static IntPtr GetUnmanagedCallbacks() | |||
{ | |||
void** callbacks = (void**)Marshal.AllocCoTaskMem(sizeof(IntPtr) * 177); | |||
void** callbacks = (void**)Marshal.AllocCoTaskMem(sizeof(IntPtr) * 176); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the updated callback array size (changed from 177 to 176) accurately reflects the removal of the getTypeForBoxOnStack callback and that all subsequent callback indices have been correctly adjusted.
Copilot uses AI. Check for mistakes.
I am seeing failures in Wondering if #114759 might be related? @MihaZupan FYI Same test is failing in #114785 |
That's #114771, specifically the tests against .NET Framework, cc: @ManickaP. |
Opened #114790 |
Does this support representing a boxed nullable value type as well? |
See #114716, though likely we never change types here as nullable box allocation happens via helpers currently, and once we do that we will need to call the |
Pretty sure none of these failures are related. There seems to be one test failure not covered by build analysis, it's a timeout in remote executor: #108210 (comment) |
Along with the
StackAllocatedBox
type. The JIT can now represent on-stack boxes directly via class layouts.Closes #114204