Implement f32/f64 comparison operators #449
Conversation
axic
force-pushed
the
float-comparison
branch
2 times, most recently
from
0b749f0 to
6942142
Compare
axic
force-pushed
the
float-comparison
branch
3 times, most recently
from
3d213ef to
c2acfb3
Compare
chfast
force-pushed
the
float-comparison
branch
2 times, most recently
from
0cf4a4e to
e4b50c2
Compare
| std::nextafter(-Limits::denorm_min(), -Limits::infinity()), | ||
| -Limits::denorm_min(), | ||
| TypeParam{0}, // FIXME. |
There was a problem hiding this comment.
It was to add "negative zero" comparison.
| } | ||
|
|
||
| // Negative zero. |
There was a problem hiding this comment.
Why are these needed if we have -0 and 0 in ordered_values?
There was a problem hiding this comment.
Ah, its -1 and 0. Can we just have -0 there too?
There was a problem hiding this comment.
The values in the array must be strictly ordered, otherwise we don't know what result to expect. But -0 == 0.
There was a problem hiding this comment.
I don't see why they need to be strictly ordered, to get the expected result you always calculate it as i op j anyway. Also as at some iterations i < j, at others j < i (and at some i == j)
There was a problem hiding this comment.
Please commit a change doing that because I don't see how this suppose to work.
There was a problem hiding this comment.
Oh I see, I misunderstood how it works first. But I pushed the way I thought it works first, it doesn't rely on strict ordering, what do you think?
chfast
force-pushed
the
float-comparison
branch
from
e4b50c2 to
cc8dc5b
Compare
Codecov Report
@@ Coverage Diff @@
## master #449 +/- ##
========================================
Coverage 99.51% 99.52%
========================================
Files 54 54
Lines 15349 15507 +158
========================================
+ Hits 15275 15433 +158
Misses 74 74 |
chfast
force-pushed
the
float-comparison
branch
2 times, most recently
from
4935d40 to
d15f8b8
Compare
chfast
force-pushed
the
float-comparison
branch
from
d15f8b8 to
8b963e4
Compare
gumb0
force-pushed
the
float-comparison
branch
from
68ecdc9 to
3524d8e
Compare
| EXPECT_THAT(execute(*inst, gt, {a, b}), Result(uint32_t{i > j})) << a << ">" << b; | ||
| EXPECT_THAT(execute(*inst, le, {a, b}), Result(uint32_t{i <= j})) << a << "<=" << b; | ||
| EXPECT_THAT(execute(*inst, ge, {a, b}), Result(uint32_t{i >= j})) << a << ">=" << b; | ||
| EXPECT_THAT(execute(*inst, eq, {a, b}), Result(uint32_t{a == b})) << a << "==" << b; |
There was a problem hiding this comment.
I want to avoid using floating point comparison on the result side, because then the test is effectively EXPECT(a == b, a == b). This may be enough to detect a typo in the implementation, but if the C++ compiler is wrong, it is likely to be wrong in both places.
There was a problem hiding this comment.
That is a good point, I'd rather document these requirements/decision as comments in the test.
There was a problem hiding this comment.
Ok, feel free to delete my commit
There was a problem hiding this comment.
Added comments and dropped the commit.
Co-authored-by: Alex Beregszaszi <alex@rtfs.hu>
chfast
force-pushed
the
float-comparison
branch
from
3524d8e to
abc3358
Compare
Depends on #448.
Need to be cleaned up and unit tests.