[release/7.0] [mono][llvm] Disable simd intrinsics when we might be interoping between jit and llvmaot #75261
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://github.com/avatars/in/15368?s=60&v=4){kind=small}
…en jit and llvmaot Revert previous partial disabling of simd intrinsics. If we have a method that checkes if some hardware functionality `IsSupported` and then calls another method making use of this functionality if so, we have a problem if the method using the actual intrinsic is not also aot compiled and the intrisic is not supported by the jit. This issue is exposed inside bcl code, where this pattern is very common, by use of profiled aot on android. This change goes for the most conservative approach, by disabling all simd if we are not fullaot-ing and we use llvm.
github-actions
bot
requested review from
vargaz,
lambdageek and
SamMonoRT
as code owners
|
There is a failing lane that I don't see it running on my main PR. This is fishy. |
I"ll temporarily add the "no merge" label while you investigate it. Please ping me when ready to get this merged (and also please remove the label). @marek-safar we need an approval, please. |
carlossanlop
added
the
NO-MERGE
marek-safar
approved these changes
Sep 8, 2022
|
@vargaz or @lambdageek or @SamMonoRT can you please give a code review sign off? |
Some of these methods are recursively calling themselves so we need to intrinsify them.
Mixing llvm jit with intrinsics enabled with aot code that has intrinsics disabled also is buggy. Seems like it can lead to simd opcodes being applied to non aligned addresses. At this point we would only have simd instrinsics enabled with fullaotllvm.
vargaz
approved these changes
Sep 9, 2022
|
Seems like just disabling SIMD wasn't really tested and it keeps triggering failures here and there even with fix attempts. This PR is not really suitable for backport. |
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #74797 to release/7.0
/cc @SamMonoRT @BrzVlad
Customer Impact
Testing
Risk
IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.