Added floor and ceiling intrinsics for System.Runtime.Intrinsics.Vector128 and System.Numerics.Vector
#83592
Conversation
Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-4.local>
This reverts commit 7a86e8d6ef1d963328026f82c8c51673a27bffc4.
Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-4.local>
Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-4.local>
Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-4.local>
Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-4.local>
LeVladIonescu
requested review from
vargaz,
lambdageek and
SamMonoRT
as code owners
src/mono/mono/mini/simd-intrinsics.c
Outdated
| @@ -1325,8 +1327,14 @@ emit_sri_vector (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSignature *fsi | |||
| if (!type_enum_is_float (arg0_type)) | |||
| return NULL; | |||
| #ifdef TARGET_ARM64 | |||
| int ceil_or_floor = id == SN_Ceiling ? INTRINS_AARCH64_ADV_SIMD_FRINTP : INTRINS_AARCH64_ADV_SIMD_FRINTM; | |||
There was a problem hiding this comment.
This could be implemented by handling OP_XOP_OVR_X_X in mini-arm64.c ?
There was a problem hiding this comment.
I tried like that firstly but INTRINS_AARCH64_ADV_SIMD_FRINTP and INTRINS_AARCH64_ADV_SIMD_FRINTM were not defined anywhere…I assume they are LLVM specific.
There was a problem hiding this comment.
This is okay for now. We probably need to clean it up later, such as unify the label names.
There was a problem hiding this comment.
They are defined in llvm-intrinsics-types.h.
There was a problem hiding this comment.
Yeah, together with llvm-intrinsics.h. And the design only considered LLVM scenarios. Now we have llvm and non-llvm. We need to somehow make the labels/keys fit both backends.
There was a problem hiding this comment.
A possible approach would be to use the llvm intrinsics in mini-arm64.c as well, they are just constants. If additional constants are needed to encode instructions that llvm doesn't have, they could be added after them, i.e. after INTRINS_NUM.
There was a problem hiding this comment.
We want to do this now or after we enable all intrinsics?
There was a problem hiding this comment.
I think it's better to do it now. I see we will need that included for other things, too.
|
It is worth mentioning in the description box that this PR contributes to a certain issue and providing the issue number. |
Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-4.local>
Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-4.local>
|
/azp run runtime-extra-platforms |
|
Pull request contains merge conflicts. |
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Some of your CI failures should be solved by #83770 |
src/mono/mono/mini/mini-arm64.c
Outdated
| @@ -21,6 +21,7 @@ | |||
| #include "ir-emit.h" | |||
| #include "aot-runtime.h" | |||
| #include "mini-runtime.h" | |||
| #include "llvm-intrinsics-types.h" | |||
There was a problem hiding this comment.
This is not needed anymore, since line 31 has it already added from Jan's PR.
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
CI failures not related to this PR |
src/mono/mono/mini/simd-intrinsics.c
Outdated
| return NULL; | ||
| #endif | ||
| // #ifdef TARGET_ARM64 | ||
| // if (!(cfg->compile_aot && cfg->full_aot && !cfg->interp)) |
There was a problem hiding this comment.
This needs to be removed before merge.
Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-4.local>
This reverts commit 9c75322.
Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-4.local>
ghost
locked as resolved and limited conversation to collaborators
Contributes to #80566