-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add compiler-generated code tests for NativeAot #77457
Conversation
Also fixes an issue where warnings were not produced for ldftn.
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.
Looks good to me. Nice catch on the Release vs Debug releases. Did you try the checked version too? I'm guessing it behaves like the Debug one
I don't think there is a checked configuration for managed code by default - only for native code in dotnet/runtime. Your comment made me realize I didn't add the release mode tests to this PR yet, thanks! |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/MethodBodyScanner.cs
Outdated
Show resolved
Hide resolved
We do support building the tools directory as checked: runtime/src/coreclr/tools/Directory.Build.props Lines 11 to 15 in 7b5ab35
It may become less relevant if linker tests get moved out of the coreclr test tree in the future (we don't need to support checked outside the coreclr tree). |
Good to know, thanks! For the tests I added, there should be no difference since we compile the testcase in-proc in a way that doesn't depend on the tools config. |
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.
Couple of comments, but nothing blocking.
src/coreclr/tools/aot/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj
Outdated
Show resolved
Hide resolved
...eclr/tools/aot/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs
Outdated
Show resolved
Hide resolved
...eclr/tools/aot/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs
Outdated
Show resolved
Hide resolved
...eclr/tools/aot/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInCompilerGeneratedCode.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/aot/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs
Outdated
Show resolved
Hide resolved
- Remove TODO comments - Use built-in NATIVEAOT define - Use display name helpers
This makes progress towards #68786. It is still hitting issues such as #68688 and #77455, which are referenced in the test comments.
Also fixes an issue where warnings were not produced for ldftn. The test changes are a modified copy of tests from the linker codebase. See dotnet/linker#3085 for the included changes.