Conversation
lib/fizzy/execute.cpp
Outdated
| inline DstT extend(SrcT in) noexcept | ||
| { | ||
| if constexpr (std::is_signed<SrcT>::value) | ||
| if constexpr (std::is_floating_point_v<SrcT>) |
There was a problem hiding this comment.
I don't think I like additional levels of generic helpers. But in this case I propose simpler implementation.
if constexpr (std::is_same_v<SrcT, DstT>)
return in;
// Handle else...
lib/fizzy/execute.cpp
Outdated
| } | ||
|
|
||
| template <typename T> | ||
| inline T pop_store_memory_arg(OperandStack& stack) noexcept |
There was a problem hiding this comment.
I don't see reason to pass OperandStack in. Better to take Value as argument and make it similar to extend. Maybe call it shrink or trunc then?
Codecov Report
@@ Coverage Diff @@
## master #456 +/- ##
========================================
Coverage 99.52% 99.53%
========================================
Files 54 54
Lines 15507 15821 +314
========================================
+ Hits 15433 15747 +314
Misses 74 74 |
gumb0
force-pushed
the
memory-float
branch
3 times, most recently
from
cce59e4 to
1b1f73e
Compare
chfast
force-pushed
the
fp_utils
branch
3 times, most recently
from
707f100 to
167c41b
Compare
|
Please rebase |
gumb0
force-pushed
the
memory-float
branch
9 times, most recently
from
c982cc0 to
cf6d852
Compare
gumb0
force-pushed
the
memory-float
branch
2 times, most recently
from
c75b2e0 to
f58e876
Compare
| (data (i32.const 12) "\00\00\c0\7f") ;; canonical NaN | ||
| (data (i32.const 16) "\01\00\c0\7f") ;; arithmetic NaN | ||
| (data (i32.const 20) "\01\00\80\7f") ;; signaling NaN | ||
| (func (param i32) (result f32) |
There was a problem hiding this comment.
Can you add all the representations from floating_point_utils, i.e. include infinity and -0?
There was a problem hiding this comment.
Added -0 and infinities, also to store tests
There was a problem hiding this comment.
Don't want to stretch it too much, but would it make sense having every case form floating_point_utils:double_as_uint, etc?
There was a problem hiding this comment.
More cases would ask for a general table approach... I'm inclined to say it's not worth it
There was a problem hiding this comment.
Don't want to stretch it too much, but would it make sense having every case form
floating_point_utils:double_as_uint, etc?
These values are not very specific. As it is is fine to me.
There was a problem hiding this comment.
I think they are nice examples though, and shows if any part (beginning or end) is cut from loading/storing.
| (data (i32.const 24) "\00\00\00\00\00\00\f8\7f") ;; canonical NaN | ||
| (data (i32.const 32) "\01\00\00\00\00\00\f8\7f") ;; arithmetic NaN | ||
| (data (i32.const 40) "\01\00\00\00\00\00\f0\7f") ;; signaling NaN | ||
| (func (param i32) (result f64) |
gumb0
force-pushed
the
memory-float
branch
2 times, most recently
from
07555c1 to
85ee1c1
Compare
| return value.as<DstT>(); | ||
| else | ||
| { | ||
| // Could use `.as<DstT>()` would we have overloads for uint8_t/uint16_t, |
There was a problem hiding this comment.
Perhaps add a static assert for is_integral? Not sure it is important.
3 participants
No description provided.