Wasmtime GitHub notifications bot (Jul 26 2022 at 18:38): Wasmtime GitHub notifications bot (Jul 26 2022 at 18:38):afonso360 commented on issue #4532:
Oh, I missed that. Sure I'll merge them
Wasmtime GitHub notifications bot (Jul 26 2022 at 18:54):afonso360 commented on issue #4532:
There are a few srem tests in cranelift/filetests/filetests/runtests/div-checks.clif. They aren't nearly as thorough as yours and the last one even invokes the wrong function. Do you want to just delete that?
I missed those :sweat_smile: . I'm going to merge both test files.
Any reason not to run the new i128 tests for aarch64 and x86_64? I see we don't run any i128 tests on s390x yet but the other two are tested for... well, almost all of the i128 runtests.
Yes, they aren't implemented in those backends. And honestly I think its too many ops to just file bugs for them, I'm going to spam the issue stream if I file a separate issue for every instruction that is unimplemented on i128, i16, i8 - AArch64 / x86 / s390x. Although I wish there was a better solution.
As a separate PR, would you like to try turning those on and see if that turns up any easy interpreter fixes?
Yeah, I'll look into this.
Wasmtime GitHub notifications bot (Jul 26 2022 at 20:51):afonso360 commented on issue #4532:
div-checks.clifdoes test some inputs that we can't easily test in the interpreter. NamelyINT_MIN % -1. In the interpreter this always traps, but it looks like it is configurable with theavoid_div_trapsflag.I've moved all inputs that don't trigger this behaviour into the
urem.clifandsrem.clifand we now run all inputs twice, with and without the flag.div-checks.clifremains to test the other inputs.This being said, the tests don't crash if I turn on or off that flag, so I'm not quite sure how well that is being tested. It is also a confusing flag name since we will still trap if we try to divide by zero.
The good news is that we were also not testing these cases (
INT_MIN % -1) in the interpreter but are now!
Last updated: Nov 22 2024 at 17:03 UTC