-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Preserve IL for CoreCLR interpreter fallback on Apple mobile #130622
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
base: main
Are you sure you want to change the base?
Changes from all commits
eacd9d2
f5f5cce
7462331
bbff0da
7276e8a
6f8d7a0
12d0b93
3b82391
a4ce2cf
7729441
ab2af53
027c501
c099887
8b8c810
3309739
7d4fd8b
5c33a99
4537bbc
9c231ea
c86b9e1
fae3aeb
7e1cc8d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -382,8 +382,11 @@ internal ReadyToRunCodegenCompilation( | |
| CompilationModuleGroup = (ReadyToRunCompilationModuleGroupBase)nodeFactory.CompilationModuleGroup; | ||
|
|
||
| // Generate baseline support specification for InstructionSetSupport. This will prevent usage of the generated | ||
| // code if the runtime environment doesn't support the specified instruction set | ||
| string instructionSetSupportString = ReadyToRunInstructionSetSupportSignature.ToInstructionSetSupportString(instructionSetSupport); | ||
| // code if the runtime environment doesn't support the specified instruction set. Targets that cannot generate | ||
| // code at runtime must not encode "must be absent" assertions, since a failing eager fixup is a fatal startup | ||
| // error with no JIT fallback (see ToInstructionSetSupportString). | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If we have no JIT fallback, we want to make sure that everything both the AOT compiler and the runtime are configured to use the same fixed instruction set so that the instruction support eager fixup would never fail. It is fine to delete the instruction support eager fixup as an optimization. 'Targets that cannot generate code at runtime must not encode "must be absent" assertions' reasoning looks suspect to me. It says "oh, this was failing at runtime so have stopped generating to make the failure go away". Is this just hiding a bug? Is this change still necessary to fix the issues that this PR is fixing?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On Apple arm64 the baseline signature emits "must be absent" assertions for the opportunistic instruction sets that fall outside the fixed baseline. GetFixedInstructionSetSupport widens that absent set to the full complement of the fixed baseline, and the device hardware actually provides many of those sets, so the eager Check_InstructionSetSupport fixup fails at load and disables all ReadyToRun code. With IL stripped and no JIT there is no fallback, so startup crashes. Crossgen2 already fixes the required set through Helpers.GetFixedInstructionSetSupport, and the device has all of it, so the required half already agrees and the check passes. The excluded set is the half that pinning cannot fix. It says the CPU must not have the optional features the code skipped. Apple hardware does have those features, so this check always fails on that hardware. A failed fixup is fatal without a JIT. That is why aligning the fixed set does not prevent the failure. On a JIT target a mismatch rejects the baseline method and the JIT recompiles for the richer hardware. Without a JIT that path does not exist, so their only effect is to reject valid fixed-baseline code and crash. I kept the "must be present" half as an invariant check, since those sets are guaranteed present on these targets. I can drop the whole eager fixup for no-JIT targets instead if you prefer that. |
||
| bool targetAllowsRuntimeCodeGeneration = ((ReadyToRunCompilerContext)nodeFactory.TypeSystemContext).TargetAllowsRuntimeCodeGeneration; | ||
| string instructionSetSupportString = ReadyToRunInstructionSetSupportSignature.ToInstructionSetSupportString(instructionSetSupport, emitExplicitlyUnsupported: targetAllowsRuntimeCodeGeneration); | ||
| ReadyToRunInstructionSetSupportSignature instructionSetSupportSig = new ReadyToRunInstructionSetSupportSignature(instructionSetSupportString); | ||
| _dependencyGraph.AddRoot(new Import(NodeFactory.EagerImports, instructionSetSupportSig), "Baseline instruction set support"); | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.