-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[main] Update dependencies from 9 repositories #84635
[main] Update dependencies from 9 repositories #84635
Conversation
…otnet-optimization build 20230406.7 optimization.linux-arm64.MIBC.Runtime , optimization.linux-x64.MIBC.Runtime , optimization.windows_nt-arm64.MIBC.Runtime , optimization.windows_nt-x64.MIBC.Runtime , optimization.windows_nt-x86.MIBC.Runtime , optimization.PGO.CoreCLR From Version 1.0.0-prerelease.23175.4 -> To Version 1.0.0-prerelease.23206.7
…ild 20230412.6 Microsoft.DotNet.HotReload.Utils.Generator.BuildTool From Version 1.1.0-alpha.0.23179.3 -> To Version 8.0.0-alpha.0.23212.6
…otnet-optimization build 20230411.4 optimization.linux-arm64.MIBC.Runtime , optimization.linux-x64.MIBC.Runtime , optimization.windows_nt-arm64.MIBC.Runtime , optimization.windows_nt-x64.MIBC.Runtime , optimization.windows_nt-x86.MIBC.Runtime , optimization.PGO.CoreCLR From Version 1.0.0-prerelease.23175.4 -> To Version 1.0.0-prerelease.23211.4
Microsoft.NETCore.Runtime.ICU.Transport From Version 8.0.0-preview.4.23203.1 -> To Version 8.0.0-preview.4.23212.1
…30412.1 Microsoft.DotNet.XHarness.CLI , Microsoft.DotNet.XHarness.TestRunners.Common , Microsoft.DotNet.XHarness.TestRunners.Xunit From Version 1.0.0-prerelease.23178.2 -> To Version 1.0.0-prerelease.23212.1
…otnet-optimization build 20230412.3 optimization.linux-arm64.MIBC.Runtime , optimization.linux-x64.MIBC.Runtime , optimization.windows_nt-arm64.MIBC.Runtime , optimization.windows_nt-x64.MIBC.Runtime , optimization.windows_nt-x86.MIBC.Runtime , optimization.PGO.CoreCLR From Version 1.0.0-prerelease.23175.4 -> To Version 1.0.0-prerelease.23212.3
|
|
/azp run runtime-ioslike |
Azure Pipelines successfully started running 1 pipeline(s). |
@steveisok should we split the icu flow out and let the rest complete? |
should we roll back the dotnet-optimization update here to unblock the other flow? |
Taking a look now... Tests seem to be calling FailFast from managed code, still tracking down why. Looks like we hit an assert in one of these two methods: runtime/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Packed.cs Lines 787 to 807 in 281c462
and are unable to handle an assert happening quite this early (the code gets reinvoked as part of assert handling). So possibly we're calling a version we should not be calling? Will keep digging, but progress is slow; WinDBG keeps disconnecting unexpectedly. |
… 20230421.2 runtime.linux-arm64.Microsoft.NETCore.Runtime.JIT.Tools , runtime.linux-arm64.Microsoft.NETCore.Runtime.Mono.LLVM.Sdk , runtime.linux-arm64.Microsoft.NETCore.Runtime.Mono.LLVM.Tools , runtime.linux-arm64.Microsoft.NETCore.Runtime.ObjWriter , runtime.linux-musl-arm64.Microsoft.NETCore.Runtime.JIT.Tools , runtime.linux-musl-arm64.Microsoft.NETCore.Runtime.ObjWriter , runtime.linux-musl-x64.Microsoft.NETCore.Runtime.JIT.Tools , runtime.linux-musl-x64.Microsoft.NETCore.Runtime.ObjWriter , runtime.linux-x64.Microsoft.NETCore.Runtime.JIT.Tools , runtime.linux-x64.Microsoft.NETCore.Runtime.Mono.LLVM.Sdk , runtime.linux-x64.Microsoft.NETCore.Runtime.Mono.LLVM.Tools , runtime.linux-x64.Microsoft.NETCore.Runtime.ObjWriter , runtime.osx-arm64.Microsoft.NETCore.Runtime.Mono.LLVM.Sdk , runtime.osx-arm64.Microsoft.NETCore.Runtime.Mono.LLVM.Tools , runtime.osx-x64.Microsoft.NETCore.Runtime.Mono.LLVM.Sdk , runtime.osx-x64.Microsoft.NETCore.Runtime.Mono.LLVM.Tools , runtime.win-arm64.Microsoft.NETCore.Runtime.JIT.Tools , runtime.win-arm64.Microsoft.NETCore.Runtime.ObjWriter , runtime.win-x64.Microsoft.NETCore.Runtime.JIT.Tools , runtime.win-x64.Microsoft.NETCore.Runtime.Mono.LLVM.Sdk , runtime.win-x64.Microsoft.NETCore.Runtime.Mono.LLVM.Tools , runtime.win-x64.Microsoft.NETCore.Runtime.ObjWriter From Version 14.0.0-alpha.1.23179.1 -> To Version 14.0.0-alpha.1.23221.2
FWIW I can repro running on a checked build, with no DOTNET env vars set. Same setup is fine with a latest main build. So has nothing to do directly with PGO or OSR. (using the Runtime_77968 test case, which is very simple). Just a guess at this point -- I see the number of methods being jitted drop considerably, so perhaps:
|
Yes, those functions appear to be violating the special rules for System.Private.CoreLib. The rule for writing code in there is that each and every function MUST behave identically whether or not the particular intrinsics are enabled. This means that you can't safely write a function which assumes that Avx2 is enabled in System.Private.CoreLib. See the special rules in https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/botr/vectors-and-intrinsics.md |
@AndyAyersMS The problem is that failing to follow the rules will only cause problems if tiering occurs in a problematic order. |
@davidwrighton, the guidance needs to be updated a bit, as for crossgen 2: https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/botr/vectors-and-intrinsics.md#code-review-rules-for-use-of-platform-intrinsics-1
Edit Nevermind, I missed the subsequent:
|
@tannergooding Note that that comment applies to usage of intrinsics OUTSIDE of System.Private.CoreLib. System.Private.CoreLib also needs to follow the rules in https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/botr/vectors-and-intrinsics.md#crossgen1-model-of-hardware-intrinsic-usage |
@tannergooding Yes, the wording is unfortunately confusing. I'd happily approve a PR which removed comments about crossgen1 and simply talked about special rules for System.Private.CoreLib. |
It would be beneficial if the doc explicitly reiterated that corelib needs to follow the rules of crossgen1. Right now its somewhat inferred, but its very easy to miss or misinterpret things. That being said, I wonder in general if the rules for corelib are the right thing here overall. We have the need and desire to factor out things into helpers that are only for one platform. This overall helps with maintainability and often JIT throughput/perf. Having these different rules for corelib makes it a lot easier to accidentally break things and much harder to write "clean code". |
So wrt to this pr, it sounds like the correct thing to do is remove the dotnet-optimization flow from it to unblock the other flow? If that is the case @sbomer already has a separate pr with the optimization flow that is triggering the existing problem I'll also mark the dotnet-optimization darc subscription as non-batchable until the underlying issue is worked out. |
Yes, let's do that. We can sort those issues out separately. |
👍, I imagine we have several of these that have snuck in. |
@BruceForstall and were wondering if there could be some special test mode that might uncover these issues more readily, especially if there is no simple way to check for them directly. Say we have a crossgen2 of SPC that crossgens everything it can, and then some runtime mode that randomly decides to use prejitted code or not -- running that we'd get a much more varied mixtures of prejitted and jitted code. |
I've disabled dontet-optimization batching and the new flow is #85193 |
@AndyAyersMS, @BruceForstall, @tannergooding Sigh. Yes the rules in System.Private.CoreLib are a problem. Unfortunately, we derive significant startup performance wins from these special rules, but I'll spend some time this week thinking about if we can make writing code .... sensible again in S.P.C When we only had a small amount of intrinsics usage this was reasonable, but we've been increasing their usage, and testing for this is impractical. |
Ok, it looks like we can build an analyzer based off of the logic of the PlatformCompatibilityAnalyzer, to find all the problematic code, and we could build a pattern with a slightly modified variant of the BypassReadyToRun attribute that would allow a bail-out for helper methods so that we can follow good engineering practices. Given the impact on the build today, I'm taking this as my current high priority item. |
For a quick solution, we may not even need to write a new analyzer - it looks like you can make the existing platform analyzer do roughly what we want with a few tweaks: https://gist.github.com/sbomer/4bfb41750259d60810aeb0b92bcc0279. |
This pull request updates the following dependencies
From https://dev.azure.com/dnceng/internal/_git/dotnet-optimization
From https://github.com/dotnet/hotreload-utils
From https://github.com/dotnet/xharness
From https://github.com/dotnet/emsdk
From https://github.com/dotnet/runtime
From https://github.com/dotnet/llvm-project
From https://github.com/dotnet/runtime-assets
From https://github.com/dotnet/cecil