Wasmtime GitHub notifications bot (Jul 14 2021 at 17:37): Wasmtime GitHub notifications bot (Jul 14 2021 at 17:37):akirilov-arm opened PR #3087 from aarch64_tests to main:
The tests for the SIMD floating-point maximum and minimum operations require particular care because the handling of the NaN values is non-deterministic and may vary between platforms. There is no way to match several NaN values in a test, so the solution is to extract the non-deterministic test cases into a separate file that is subsequently replicated for every backend under test, with adjustments made to the expected results.
Wasmtime GitHub notifications bot (Jul 14 2021 at 17:47):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Jul 14 2021 at 17:47):bjorn3 created PR review comment:
Is this the minimum necessary or would
nehalemwork too? Last time I checked the macOS builders on GHA were still atnehalem.
Wasmtime GitHub notifications bot (Jul 14 2021 at 17:56):akirilov-arm created PR review comment:
Honestly, I don't know; I just copied the value from
simd-arithmetic.clif.
Wasmtime GitHub notifications bot (Jul 14 2021 at 17:56):akirilov-arm submitted PR review.
Wasmtime GitHub notifications bot (Jul 14 2021 at 19:41):abrown created PR review comment:
This pair of tests should probably go in https://github.com/bytecodealliance/wasmtime/tree/main/cranelift/filetests/filetests/isa?
Wasmtime GitHub notifications bot (Jul 14 2021 at 19:41):abrown submitted PR review.
Wasmtime GitHub notifications bot (Jul 14 2021 at 19:42):abrown edited PR review comment.
Wasmtime GitHub notifications bot (Jul 14 2021 at 21:04):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Jul 14 2021 at 21:04):bjorn3 created PR review comment:
More than one target may have the same result for this test. It would be nice to not duplicate it in that case.
Wasmtime GitHub notifications bot (Jul 15 2021 at 00:00):abrown submitted PR review.
Wasmtime GitHub notifications bot (Jul 15 2021 at 00:00):abrown created PR review comment:
But that isn't the case here, right?
Wasmtime GitHub notifications bot (Jul 15 2021 at 06:12):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Jul 15 2021 at 06:12):bjorn3 created PR review comment:
Not yet, but it may in the future as new targets get added. Eg s390x may gove the same result as either x86_64 or aarch64.
Wasmtime GitHub notifications bot (Jul 15 2021 at 16:48):akirilov-arm submitted PR review.
Wasmtime GitHub notifications bot (Jul 15 2021 at 16:48):akirilov-arm created PR review comment:
What @bjorn3 has said is one reason I have kept the files in their present location; there is also an incomplete backend targeting the 32-bit Arm architecture, in which case the behaviour may very well match the AArch64 one (but not necessarily - there are differences between the floating-point handling in AArch32 and AArch64). The other reason is to preserve the current organization of having all runtests in one place, and some of the latter are already meant for a specific backend - the ones with
legacyin their name (e.g.cranelift/filetests/filetests/runtests/i128-arithmetic-legacy.clif), though admittedly they should be removed eventually.
Wasmtime GitHub notifications bot (Jul 26 2021 at 21:19):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Jul 26 2021 at 21:19):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Jul 26 2021 at 21:19):cfallin created PR review comment:
It's probably not too important, as tests such as this are OS-independent and will get coverage on the (more up-to-date-CPU) Linux runners.
Wasmtime GitHub notifications bot (Jul 26 2021 at 21:19):cfallin created PR review comment:
Agreed that these tests could be shared. That said, would it be possible to name the file in a way that denotes the behavior a little more specifically?
-nondeterminsticis good (it indicates that the specified behavior isn't the only valid behavior) but maybe something like-nondeterministic-armlikeor somesuch (and we can rename if we later add e.g.s390x)?
Wasmtime GitHub notifications bot (Aug 17 2021 at 12:26):akirilov-arm updated PR #3087 from aarch64_tests to main.
Wasmtime GitHub notifications bot (Aug 17 2021 at 12:28):akirilov-arm updated PR #3087 from aarch64_tests to main.
Wasmtime GitHub notifications bot (Aug 17 2021 at 16:21):cfallin merged PR #3087.
Last updated: Oct 23 2024 at 20:03 UTC