Implement f{32,64}.sub instructions #474
Conversation
chfast
commented
Aug 12, 2020
•
chfast
commented
| EXPECT_THAT(exec(TypeParam{0.0}, q), Result(-q)); | ||
| EXPECT_THAT(exec(-TypeParam{0.0}, -q), Result(q)); | ||
|
|
||
| // Not part of the spec. |
There was a problem hiding this comment.
The fsub(+-0, -q) = +-q is missing.
There was a problem hiding this comment.
This covers fsub(+-0, -q2) and shows that it negates the sign.
There was a problem hiding this comment.
You cannot resolve it like that because
they must be resolved consistently: either the upper sign is chosen for all of them or the lower sign.
There was a problem hiding this comment.
Oh I see. You mean only these are defined:
fsub(+0, +q2) = -q2
fsub(-0, -q2) = +q2
But not these
fsub(-0, +q2) = -q2
fsub(+0, -q2) = +q2
Probably a bug in the spec, because text description doesn't mention any special cases, just "if z1 is zero"
|
How about adding a test that checks this?
|
Added a test for it. |
chfast
force-pushed
the
fp_sub
branch
2 times, most recently
from
3cbf074 to
a01893e
Compare
Codecov Report
@@ Coverage Diff @@
## master #474 +/- ##
========================================
Coverage 99.55% 99.56%
========================================
Files 54 54
Lines 16729 16858 +129
========================================
+ Hits 16655 16784 +129
Misses 74 74 |
|
Please approve. |
Checks the note from the Wasm spec: > Up to the non-determinism regarding NaNs, it always holds that > fsub(z1, z2) = fadd(z1, fneg(z2)).
