Enable "Allocate on stack" for NAOT/R2R by EgorBo · Pull Request #104411 · dotnet/runtime

EgorBo · 2024-07-04T00:41:35Z

Closes#104350
Fixes#104337
This PR adds NAOT/R2R support for #103361

Example:

staticintTest1(){MyClassobj=newMyClass(42);// MyClass allocation never escapesreturnobj.X;}staticintTest2(){objecto=42;// boxing (allocation)return(int)o;// unboxing}publicclassMyClass(intx){publicintX=>x;}

Old codegen for Test() on NAOT (and R2R):

; Assembly listing for method Proga:Test1():int (FullOpts); NativeAOT compilationsubrsp,40learcx,[(reloc 0x40000000004211a8)] ; MyClasscall CORINFO_HELP_NEWSFASTmov dword ptr [rax+0x08],42moveax, dword ptr [rax+0x08]addrsp,40ret; Assembly listing for method Proga:Test2():int (FullOpts); NativeAOT compilationsubrsp,40learcx,[(reloc 0x4000000000421008)] ; Int32call CORINFO_HELP_NEWSFASTmov dword ptr [rax+0x08],42moveax, dword ptr [rax+0x08]addrsp,40ret

New codegen:

; Assembly listing for method Proga:Test1():int (FullOpts); NativeAOT compilationmoveax,42ret; Assembly listing for method Proga:Test2():int (FullOpts); NativeAOT compilationmoveax,42ret

ShreyasJejurkar · 2024-07-04T14:25:15Z

Is the .NET main branch broken on CE somehow? Checking the codegen for the example, yields older code.

https://godbolt.org/z/aveWzrfvv

EgorBo · 2024-07-04T15:11:51Z

Is the .NET main branch broken on CE somehow? Checking the codegen for the example, yields older code.

#104442

AndyAyersMS · 2024-07-04T18:13:19Z

I'm surprised that you don't need to do anything on the runtime side to implement CORINFO_HELP_UNBOX_TYPETEST.

EgorBo · 2024-07-04T19:13:35Z

I'm surprised that you don't need to do anything on the runtime side to implement CORINFO_HELP_UNBOX_TYPETEST.

CI is currently failing with:

EXEC : error : One or more errors occurred. (Code generation failed for method '[Microsoft.Extensions.Logging.Configuration]Microsoft.Extensions.Configuration.Binder.SourceGeneration.<BindingExtensions_g>F7FD99DEF4D1E9898A126F6C62635605FA12A7F83D698AD1B212E573AF4334BC7__BindingExtensions.GetValue<bool>(IConfiguration,string,bool)') [D:\a\_work\1\s\artifacts\bin\aotTests\projects\Microsoft.Extensions.Logging.Console.TrimmingTests\JsonFormattingTests\win-x64\project.csproj]
##[error]EXEC(0,0): error : (NETCORE_ENGINEERING_TELEMETRY=Build) One or more errors occurred. (Code generation failed for method '[Microsoft.Extensions.Logging.Configuration]Microsoft.Extensions.Configuration.Binder.SourceGeneration.<BindingExtensions_g>F7FD99DEF4D1E9898A126F6C62635605FA12A7F83D698AD1B212E573AF4334BC7__BindingExtensions.GetValue<bool>(IConfiguration,string,bool)')
System.AggregateException: One or more errors occurred. (Code generation failed for method '[Microsoft.Extensions.Logging.Configuration]Microsoft.Extensions.Configuration.Binder.SourceGeneration.<BindingExtensions_g>F7FD99DEF4D1E9898A126F6C62635605FA12A7F83D698AD1B212E573AF4334BC7__BindingExtensions.GetValue<bool>(IConfiguration,string,bool)')
---> ILCompiler.CodeGenerationFailedException: Code generation failed for method '[Microsoft.Extensions.Logging.Configuration]Microsoft.Extensions.Configuration.Binder.SourceGeneration.<BindingExtensions_g>F7FD99DEF4D1E9898A126F6C62635605FA12A7F83D698AD1B212E573AF4334BC7__BindingExtensions.GetValue<bool>(IConfiguration,string,bool)'
---> System.NotImplementedException: CORINFO_HELP_UNBOX_TYPETEST
at Internal.JitInterface.CorInfoImpl.GetHelperFtnUncached(CorInfoHelpFunc) + 0xa60
at Internal.JitInterface.CorInfoImpl.getHelperFtn(CorInfoHelpFunc, Void*&) + 0x34

🙂

EgorBo · 2024-07-05T10:40:06Z

/azp run runtime-nativeaot-outerloop

azure-pipelines · 2024-07-05T10:40:15Z

Azure Pipelines successfully started running 1 pipeline(s).

EgorBo · 2024-07-05T12:28:31Z

I think this is ready, PTAL @jkotas @MichalStrehovsky @AndyAyersMS

EgorBo · 2024-07-05T13:39:16Z

- for (int i = 0; i < ptrsCount; i++)
- gcPtrs[i] = (byte)CorInfoGCType.TYPE_GC_NONE;
+ uint size = type.IsValueType ? getClassSize(cls) : getHeapClassSize(cls);
+ new Span<byte>(gcPtrs, (int)((size + PointerSize - 1) / PointerSize)).Clear();


This was copied from CoreCLR impl, it does a similar memset (see this)
Previously, getClassGClayout used InstanceFieldSize.AsInt which for ref types always return just pointer-size while we need the real size. Since it has never been used for ref-types before, it wasn't an issue.

EgorBo · 2024-07-12T13:46:00Z

Outerloops:

runtime-nativeaot-outerloop: mostly Condition DurationHistogram_HasBucketSizeHints on RemoteExecutor #104736, also a weird AVE in System.Net.Sockets.Tests but it's not my PR specific, I'll file a known-build-failure for it. Also, a small issue in the StackAllocation test - going to fix it now.
gcstress0x3-gcstress0xc: no new issues
runtime-coreclr crossgen2: no new issues

MichalStrehovsky · 2024-07-12T14:00:03Z

also a weird AVE in System.Net.Sockets.Tests but it's not my PR specific

This is #104500.

MichalStrehovsky · 2024-07-12T14:08:47Z

+ if (attribs.Length == 0)
+ return true; // Assume corelib is optimized in this case


Adding <DebuggerSupport>true</DebuggerSupport> to the project file would make the attribute visible, but ILC has its own optimization switch so CoreLib having this attribute still doesn't necessarily imply how optimized it is.

@MichalStrehovsky do we run runtime tests with NativeAOT in full debug mode?

So can you tell me how to properly check it here?

@MichalStrehovsky do we run runtime tests with NativeAOT in full debug mode?
We do have such legs, I'm not sure if they include this test too. It would be in the runtime or runtime-nativeaot-outerloop if it exists.
So can you tell me how to properly check it here?
Native AOT will use the same settings for optimizing both CoreLib and user code so enabling the attribute and looking for the attribute on Assembly.GetEntryAssembly would give the answer.
I guess I don't understand why this is looking at whether corelib is optimized. Why does that affect stack allocations in this test?

I guess I don't understand why this is looking at whether corelib is optimized. Why does that affect stack allocations in this test?
Maybe the way we build the tests, optimization of CoreLib always matches optimization of the test?
If that's the case, all that's needed is to add <DebuggerSupport>true</DebuggerSupport> to the test project and this will work the same. It will also fail the same if there's ever mismatch between test optimization and corelib optimization.

I guess I don't understand why this is looking at whether corelib is optimized. Why does that affect stack allocations in this test?
I am not the original author (I presume the authors no longer work in the team), but looks like the test tries to be agile and validate that nothing is stack-allocated under debug maybe? 🤷

I am not the original author (I presume the authors no longer work in the team), but looks like the test tries to be agile and validate that nothing is stack-allocated under debug maybe?
Right, but for that one would check whether the test is optimized, not corelib. One can build the test with <Optimize>true</Optimize> and have corelib from a debug build. I assume the test would fail in that case.

(I mean in the JIT case. It would also fail in the AOT case because I believe the test is looking at the wrong assembly.)

I ended up removing these bits and the test is still working fine (it's always optimized due to <Optimize>true</Optimize>)

EgorBo · 2024-07-12T14:48:08Z

/azp run runtime-nativeaot-outerloop

azure-pipelines · 2024-07-12T14:48:19Z

Azure Pipelines successfully started running 1 pipeline(s).

…lloc-stack-naot

EgorBo · 2024-07-13T17:45:11Z

/azp run runtime-coreclr runtime-nativeaot-outerloop

azure-pipelines · 2024-07-13T17:45:17Z

No pipelines are associated with this pull request.

EgorBo · 2024-07-13T19:21:59Z

/azp run runtime-nativeaot-outerloop

azure-pipelines · 2024-07-13T19:22:15Z

Azure Pipelines successfully started running 1 pipeline(s).

EgorBo · 2024-07-14T11:52:29Z

@jkotas @MichalStrehovsky can this be merged then? All nativeaot-outerloop failures are either #104862 or #104500

jkotas

LGTM otherwise. Thank you!

Enable "Allocate on stack" for NAOT/R2R
000b528

ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 4, 2024

dotnet-policy-serviceBot assigned EgorBo Jul 4, 2024

jkotas reviewed Jul 4, 2024
View reviewed changes

Comment threadsrc/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/StackAllocatedBox.cs

Fix getClassGClayout on NAOT
67b495d

EgorBo added 2 commits July 4, 2024 22:30

Handle CORINFO_HELP_UNBOX_TYPETEST on NAOT/R2R
8e0e9eb

fix build
2a62eeb

EgorBo added area-NativeAOT-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jul 4, 2024

build-analysisBot mentioned this pull request Jul 5, 2024
System.IO.Net5Compat.Tests and System.IO.Tests suddenly exiting with error 137 #100558
Closed

EgorBo added 3 commits July 5, 2024 11:24

implement RhUnboxTypeTest
741e4fa

Update ILCompiler.RyuJit.csproj
396677b

Update StackAllocatedBox.cs
19936e7

EgorBo commented Jul 5, 2024
View reviewed changes

Comment threadsrc/coreclr/inc/corinfoinstructionset.h

EgorBo marked this pull request as ready for review July 5, 2024 11:25

EgorBo requested a review from MichalStrehovsky as a code owner July 5, 2024 11:25

github-actionsBot mentioned this pull request Jul 5, 2024
104411 MichalStrehovsky/rt-sz#44
Closed

EgorBo commented Jul 5, 2024
View reviewed changes

Comment threadsrc/coreclr/nativeaot/Runtime.Base/src/System/Runtime/RuntimeExports.cs Outdated

clean up
b07c1a0

EgorBo commented Jul 5, 2024
View reviewed changes

EgorBo added 2 commits July 12, 2024 15:47

Update ObjectStackAllocationTests.cs
4f9ff46

Update ObjectStackAllocationTests.cs
ac65ff6

MichalStrehovsky reviewed Jul 12, 2024
View reviewed changes

Merge branch 'main' into alloc-stack-naot
203593c

EgorBo added 3 commits July 13, 2024 18:01

Merge branch 'alloc-stack-naot' of github.com:EgorBo/runtime-1 into a…
001b01c
…lloc-stack-naot

Merge branch 'main' of github.com:dotnet/runtime into alloc-stack-naot
ac0e6e8

Clean up in the test
ed2568c

jkotas reviewed Jul 14, 2024
View reviewed changes

Comment threadsrc/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.csproj Outdated

jkotas reviewed Jul 14, 2024
View reviewed changes

Comment threadsrc/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.csproj Outdated

jkotas reviewed Jul 14, 2024
View reviewed changes

Comment threadsrc/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs

jkotas approved these changes Jul 14, 2024
View reviewed changes

EgorBo added 2 commits July 14, 2024 19:03

Update ObjectStackAllocationTests.csproj
d6a32b3

Update ObjectStackAllocationTests.cs
85340ae

build-analysisBot mentioned this pull request Jul 14, 2024
TimeProviderTests.TestProviderTimer failed in CI #103459
Closed

EgorBo merged commit af5e715 into dotnet:mainJul 14, 2024

EgorBo deleted the alloc-stack-naot branch July 14, 2024 23:12

EgorBo mentioned this pull request Jul 18, 2024
Test failure: JIT/opt/Devirtualization/Comparer_get_Default/Comparer_get_Default.dll #104927
Closed

github-actionsBot locked and limited conversation to collaborators Aug 14, 2024

		if (attribs.Length == 0)
		return true; // Assume corelib is optimized in this case

Uh oh!

Conversation

EgorBo commented Jul 4, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ShreyasJejurkar commented Jul 4, 2024

Uh oh!

EgorBo commented Jul 4, 2024

Uh oh!

AndyAyersMS commented Jul 4, 2024

Uh oh!

EgorBo commented Jul 4, 2024

Uh oh!

Uh oh!

EgorBo commented Jul 5, 2024

Uh oh!

azure-pipelinesBot commented Jul 5, 2024

Uh oh!

EgorBo commented Jul 5, 2024

Uh oh!

Uh oh!

EgorBoJul 5, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EgorBo commented Jul 12, 2024

Uh oh!

MichalStrehovsky commented Jul 12, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EgorBoJul 13, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EgorBo commented Jul 12, 2024

Uh oh!

azure-pipelinesBot commented Jul 12, 2024

Uh oh!

EgorBo commented Jul 13, 2024

Uh oh!

azure-pipelinesBot commented Jul 13, 2024

Uh oh!

EgorBo commented Jul 13, 2024

Uh oh!

azure-pipelinesBot commented Jul 13, 2024

Uh oh!

EgorBo commented Jul 14, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkotas left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

EgorBo commented Jul 4, 2024•
edited
Loading

EgorBoJul 5, 2024•
edited
Loading

EgorBoJul 13, 2024•
edited
Loading