NaN canonization

[MaksymZavershynskyi]

This issue should be resolved by Wasmer. Once it is resolved we should enable non-x86 architectures.

• [ ] Implement NaN canonization on Singlepass compiler of Wasmer;
• [ ] Enable NaN canonization feature;
• [ ] Remove x86 check

[MaksymZavershynskyi]

Assigning low priority since most of the work is done by Wasmer.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[bowenwang1996]

@nearmax what is the status of this issue?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[bowenwang1996]

@matklad could you check whether this is still relevant?

[matklad]

@olonho am I correct that wasmer2 canonicalizes nans in an arch-independent way, and that we can close this after #4910 ?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[nagisa]

Wasmer2's singlepass backend canonicalizes NaNs as necessary by default (though I cannot vouch for the canonicalization being exhaustive, or bug-free), making the first two points in the checklist above solved.

In response to:

Once it is resolved we should enable non-x86 architectures

singlepass supports x86_64 only, right now, so NaN canonicalization is probably the least of the blockers for this kind of feature request and probably should be tracked as a separate issue. The check in question that we have in nearcore can go away either way though, because wasmer now checks that a supported architecture is being targetted by itself.

[matklad]

Makes sense!

Do we have tests for nan canonicalization? This is something that might differ between runtimes (as that's not specced by WASM), so we should have tests for this on our side.

I think we have one test here:

https://github.com/near/nearcore/blob/960c4112111bc4a2f214af65c37bfa55a1552062/runtime/near-vm-runner/src/tests/runtime_errors.rs#L794-L843

But I don't think we have reinterpret based tests.

So: let's add test some time and then close this.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity in the last 2 months. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[matklad]

cc @akhi3030 , you might want to take a look at this.