Skip to content
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

[wasm] Fix Vector128 SIMD fmin and fmax #93556

Merged
merged 2 commits into from
Oct 17, 2023

Conversation

radekdoulik
Copy link
Member

@radekdoulik radekdoulik commented Oct 16, 2023

Fix #92885

Min and max now use NaN propagating SIMD instructions, f32x4.min, f64x2.min, f32x4.max and f64x2.max.

Example emitted code:

...
f32x4.min    [SIMD]
v128.store    [SIMD]

for

return Vector128.Min(/* Vector128<float> */ v1, v2);

@radekdoulik
Copy link
Member Author

/azp run runtime-wasm-libtests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@radekdoulik
Copy link
Member Author

radekdoulik commented Oct 16, 2023

I think we can try to unify all the platforms to use llvm.[maximum|minimum].[v4f32|v2f64] intrinsics for v128. Lets keep it simple for this PR so that it is easier to backport to NET8.

@radekdoulik radekdoulik added this to the 8.0.0 milestone Oct 16, 2023
@radekdoulik radekdoulik requested a review from lewing October 16, 2023 16:12
@lewing
Copy link
Member

lewing commented Oct 16, 2023

@radical @ilonatommy are the hybrid globalization tests failing in main?

ah #93354

@radekdoulik
Copy link
Member Author

Failed tests are hybrid globalization, the runtime (Build browser-wasm linux Release AllSubsets_Mono_RuntimeTests monointerpreter) In progress — This check has started... is:

##[error]
,##[warning]Agent  was purged, cancelling the pipeline.
,##[warning]Received request to deprovision: The request was cancelled by the remote provider.
Agent: NetCore-Public 140
Started: Yesterday at 6:11 PM
Duration: 1h 42m 59s

@radekdoulik radekdoulik merged commit 72e6953 into dotnet:main Oct 17, 2023
@radekdoulik
Copy link
Member Author

/backport to release/8.0

@github-actions
Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6545017032

@github-actions
Copy link
Contributor

@radekdoulik an error occurred while backporting to release/8.0, please check the run log for details!

Error: @radekdoulik is not a repo collaborator, backporting is not allowed. If you're a collaborator please make sure your dotnet team membership visibility is set to Public on https://github.com/orgs/dotnet/people?query=radekdoulik

@radekdoulik
Copy link
Member Author

/backport to release/8.0

@github-actions
Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6548822124

@github-actions
Copy link
Contributor

@radekdoulik backporting to release/8.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: [wasm] Fix SIMD fmin and fmax
Applying: Revert "[wasm] Disable `TensorPrimitivesTests.ConvertToHalf_SpecialValues` (#92953)"
Using index info to reconstruct a base tree...
M	src/libraries/System.Numerics.Tensors/tests/TensorPrimitivesTests.netcore.cs
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/System.Numerics.Tensors/tests/TensorPrimitivesTests.netcore.cs
CONFLICT (content): Merge conflict in src/libraries/System.Numerics.Tensors/tests/TensorPrimitivesTests.netcore.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 Revert "[wasm] Disable `TensorPrimitivesTests.ConvertToHalf_SpecialValues` (#92953)"
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@github-actions
Copy link
Contributor

@radekdoulik an error occurred while backporting to release/8.0, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[browser] TensorPrimitivesTests.ConvertToHalf_SpecialValues -Infinity
3 participants