-
Notifications
You must be signed in to change notification settings - Fork 518
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
[dotnet/msbuild] Add support for using LLVM to build .NET apps. Fixes #11379. #12136
Conversation
…App in .NET from the command line.
…er steps. So that the ComputeAOTArguments can compute the llvm-path value to pass to the AOT compiler (the llvm-path value states where the opt and llc command-line tools are, and they're next to the AOT compiler).
… specify output paths. This deduplicates a little bit code to compute the output path.
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diff✅ API Diff (from PR only) (no change) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results2 tests failed, 119 tests passed.Failed tests
Pipeline on Agent XAMBOT-1096.BigSur' |
@@ -703,7 +705,7 @@ | |||
Condition="'$(_SdkIsSimulator)' != 'true' And '$(_PlatformName)' != 'macOS'" | |||
DependsOnTargets="_ComputeVariables" | |||
Inputs="@(_AssembliesToAOT)" | |||
Outputs="@(_AssembliesToAOT -> '$(_AOTOutputDirectory)%(Arch)\%(Filename)%(Extension).o')"> | |||
Outputs="@(_AssembliesToAOT -> '%(ObjectFile)');@(_AssembliesToAOT -> '%(LLVMFile)');"> |
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.
@rolfbjarne are both ObjectFile
and LLVMFile
ITaskItem outputs of any task? I'm assuming those files will be produced by a task executed on macOS, so we need those to be an output prop of a task so they are copied to Windows to make the Inputs/Outputs check work, otherwise this target will always be executed from Windows.
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.
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.
Yeah, I think that should work!
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.
That does not seems to fulfill the request of having LLVM by default for release (non-debug) builds.
It might be elsewhere... I'm a bit behind on the changes :)
Before we can make it the default, we have to implement it :) Making it the default once it's working is easy enough. |
… make the build work correctly on windows.
hmm... I thought this PR was the (last) missing piece. |
✅ [PR Build] Tests passed on Build. ✅Tests passed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diff✅ API Diff (from PR only) (no change) GitHub pagesResults can be found in the following github pages (it might take some time to publish): 🎉 All 121 tests passed 🎉Pipeline on Agent XAMBOT-1100.BigSur' |
I've created #12147 to track making LLVM the default for release builds. |
/sudo backport release/6.0.1xx-preview7 |
Backport Job to branch release/6.0.1xx-preview7 Created! The magic is happening here |
Hooray! Backport succeeded! Please see https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=5007997 for more details. |
Fixes #11379.