Add support for NativeAOT testing #62704

MichalStrehovsky · 2021-12-13T07:22:41Z

Adds support for running runtime tests in NativeAOT configuration. It's similar to crossgen2 testing, but there's small differences.

A layout for the AOT compiler is generated into CORE_ROOT. This includes the AOT compiler, NativeAOT corelib, and framework/test dependencies we otherwise dump into CORE_ROOT.
The test execution script is amended to first AOT compile the test using the AOT compiler in CORE_ROOT and then run the AOT compiled test.
The test is compiled by running MSBuild on a generated project file. This is different from crossgen2 testing that directly invokes crossgen2. The extra project file is annoying, but also the NativeAOT compiler has more command line arguments and we also need to invoke the platform linker with another set of command line arguments. Having MSBuild do that for us (using the shipping .target/.props that ship with NativeAOT) saves a some trouble there.
This also includes support for multimodule testing (where each managed .dll is compiled into a single .o/.obj that we link with the native linker). This needs an extra step during test build to precompile the entire framework into a .a/.lib file.

Adds support for running runtime tests in NativeAOT configuration. It's similar to crossgen2 testing, but there's small differences. * A layout for the AOT compiler is generated into `CORE_ROOT`. This includes the AOT compiler, NativeAOT corelib, and framework/test dependencies we otherwise dump into `CORE_ROOT`. * The test execution script is amended to first AOT compile the test using the AOT compiler in `CORE_ROOT` and then run the AOT compiled test. * The test is compiled by running MSBuild on a generated project file. This is different from crossgen2 testing that directly invokes crossgen2. The extra project file is annoying, but also the NativeAOT compiler has more command line arguments and we also need to invoke the platform linker with another set of command line arguments. Having MSBuild do that for us (using the shipping .target/.props that ship with NativeAOT) saves a some trouble there. * This also includes support for multimodule testing (where each managed .dll is compiled into a single .o/.obj that we link with the native linker). This needs an extra step during test build to precompile the entire framework into a .a/.lib file.

ghost · 2021-12-13T07:22:49Z

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

Issue Details

Adds support for running runtime tests in NativeAOT configuration. It's similar to crossgen2 testing, but there's small differences.

A layout for the AOT compiler is generated into CORE_ROOT. This includes the AOT compiler, NativeAOT corelib, and framework/test dependencies we otherwise dump into CORE_ROOT.
The test execution script is amended to first AOT compile the test using the AOT compiler in CORE_ROOT and then run the AOT compiled test.
The test is compiled by running MSBuild on a generated project file. This is different from crossgen2 testing that directly invokes crossgen2. The extra project file is annoying, but also the NativeAOT compiler has more command line arguments and we also need to invoke the platform linker with another set of command line arguments. Having MSBuild do that for us (using the shipping .target/.props that ship with NativeAOT) saves a some trouble there.
This also includes support for multimodule testing (where each managed .dll is compiled into a single .o/.obj that we link with the native linker). This needs an extra step during test build to precompile the entire framework into a .a/.lib file.

Author:	MichalStrehovsky
Assignees:	-
Labels:	`area-Infrastructure-coreclr`
Milestone:	-

trylek

Looks great to me, thanks Michal! I have noticed just a few non-blocking nits, I haven't spotted any serious issue.

trylek · 2021-12-13T08:09:51Z

src/tests/Common/CLRTest.Execute.Batch.targets

@@ -318,6 +321,11 @@ set CLRTestExitCode=!ERRORLEVEL!
 if defined RunCrossGen (
  call :ReleaseLock
 )
+if defined RunNativeAot (
+  if not defined CLRTestNoCleanup (
+    IF EXIST native rmdir /s /q native


Why do we need this just in the Windows version of the script and how does it play together with similar clauses in the execution script snippet generator in CLRTest.NativeAOT.targets?

Ah, good catch. I guess we never run enough tests on Unix systems where not deleting the results would be a problem. I'll fix that.

trylek · 2021-12-13T08:11:47Z

src/tests/Common/CLRTest.NativeAot.targets

+    if errorlevel 1 (
+        ECHO END COMPILATION - FAILED
+        ECHO FAILED
+        rem TODO: Why doesn't "exit /b" end with failure? Crossgen uses that above, but it results in success here!?


If this comment is still valid, we should probably rename Crossgen to NativeAOT, otherwise it looks like an overlooked copy-paste.

The comment refers to these lines in the crossgen2 targets file:

runtime/src/tests/Common/CLRTest.CrossGen.targets

Lines 259 to 262 in 03194f7

IF NOT !CrossGen2Status!==0 (

ECHO Crossgen2 failed with exitcode - !CrossGen2Status!

Exit /b 1

)

When I had exit /b here, AOT compilation failure would result in a test success. It was very puzzling. Without the /b, AOT compilation failure properly fails the test, but also closes the command prompt if ran manually outside the xunit runner so this is rather annoying.

I'll retest if this still works that way. If so, I'll double check what it does in crossgen2's case since there's the /b that I saw as problematic.

Lol, okay, so I spent the whole morning troubleshooting this.

If you have this batch file:

( exit /b 5 ) echo Hello

and run it with

using System; using System.Diagnostics; Process process = new Process(); process.StartInfo.FileName = @"C:\git\runtime\artifacts\tests\coreclr\windows.x64.Debug\nativeaot\SmokeTests\BasicThreading\BasicThreading\aa.cmd"; process.StartInfo.UseShellExecute = false; process.Start(); process.WaitForExit(); int exitCode = process.ExitCode; Console.WriteLine(exitCode);

The Console.WriteLine will print exit code 5. So far so good. Makes sense. This is what I would expect.

Now move the echo Hello inside the block so that you have:

( exit /b 5 echo Hello )

Now it prints 0. Funfunfunfunfun.

trylek · 2021-12-13T08:12:48Z

src/tests/Common/Directory.Build.targets

+      <AotCompilerCopyLocal Include="$(CoreCLRArtifactsPath)build/**/*" TargetDir="nativeaot/build/" />
+      <AotCompilerCopyLocal Include="$(TargetingPackPath)/*" TargetDir="nativeaot/framework/" />
+
+      <!-- Works around https://github.com/dotnet/runtime/issues/62372 -->


The issue was closed about a week ago or so, do we still need this?

The way the issue got resolved is very much not ideal (it placed a similar filter into mono's AOT step, so this filter is also needed).

Correct fix should have been to make it so that the problematic DLL doesn't get into CORE_ROOT in the first place. The fact we still have the file in CORE_ROOT means we're sending a garbage file to all test machines.

Hmm, I see, that's unfortunate indeed, I assume that the right fix would be to adjust the filtering clauses around

runtime/src/tests/Directory.Build.targets

Line 294 in 03194f7

<ItemGroup>

I defer to Jeremy whether we should reopen the issue as not fully resolved or open another issue for the proper solution, link to a closed issue in a code comment is typically interpreted as 'obsolete link to an issue that has since been fixed' and after some time it may become quite a pain to track down what the original problem was and what is its current status.

Yeah we should make sure that DllImportGenerator doesn’t get put into Core_Root in the first place.

I'll look into the DllImportGenerator issue in a separate pull request because I don't want this pull request to be a functional change.

trylek · 2021-12-13T08:15:41Z

src/tests/Common/CLRTest.Execute.targets

@@ -78,6 +78,7 @@ This file contains the logic for providing Execution Script generation.

  <Import Project="CLRTest.Jit.targets" />
  <Import Project="CLRTest.CrossGen.targets" />
+  <Import Project="CLRTest.NativeAot.targets" />


Nit - we may want to check somewhere that RunCrossgen2 and RunNativeAOT aren't set at the same time, otherwise I suspect interesting things may happen.

MichalStrehovsky requested review from jkoritzinsky and trylek December 13, 2021 07:22

ghost added the area-Infrastructure-coreclr label Dec 13, 2021

trylek approved these changes Dec 13, 2021

View reviewed changes

MichalStrehovsky added 2 commits December 14, 2021 13:17

Work around batch oddness

1f114f0

Clean up artifacts on Unix too

8b670a6

MichalStrehovsky merged commit eb0ef76 into dotnet:main Dec 14, 2021

MichalStrehovsky deleted the naottest branch December 14, 2021 08:01

ghost locked as resolved and limited conversation to collaborators Jan 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add support for NativeAOT testing #62704

Add support for NativeAOT testing #62704

Uh oh!

MichalStrehovsky commented Dec 13, 2021

Uh oh!

ghost commented Dec 13, 2021

Uh oh!

trylek left a comment

Uh oh!

trylek Dec 13, 2021

Uh oh!

MichalStrehovsky Dec 13, 2021

Uh oh!

trylek Dec 13, 2021

Uh oh!

MichalStrehovsky Dec 13, 2021

Uh oh!

MichalStrehovsky Dec 14, 2021

Uh oh!

trylek Dec 13, 2021

Uh oh!

MichalStrehovsky Dec 13, 2021

Uh oh!

trylek Dec 13, 2021

Uh oh!

jkoritzinsky Dec 13, 2021

Uh oh!

MichalStrehovsky Dec 14, 2021

Uh oh!

trylek Dec 13, 2021

Uh oh!

Uh oh!

	IF NOT !CrossGen2Status!==0 (
	ECHO Crossgen2 failed with exitcode - !CrossGen2Status!
	Exit /b 1
	)

Add support for NativeAOT testing #62704

Add support for NativeAOT testing #62704

Uh oh!

Conversation

MichalStrehovsky commented Dec 13, 2021

Uh oh!

ghost commented Dec 13, 2021

Uh oh!

trylek left a comment

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!

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!

Uh oh!