Upgrade intx to 0.6.0 #345
Conversation
Codecov Report
@@ Coverage Diff @@
## master #345 +/- ##
=======================================
Coverage 99.78% 99.78%
=======================================
Files 30 30
Lines 4135 4139 +4
=======================================
+ Hits 4126 4130 +4
Misses 9 9
Flags with carried forward coverage won't be shown. Click here to find out more.
|
lib/evmone/instructions.hpp
Outdated
| const auto x_neg = x.hi.hi >> 63; | ||
| const auto y_neg = y.hi.hi >> 63; | ||
| y = ((x_neg ^ y_neg) != 0) ? y_neg : y < x; | ||
| stack[0] = slt(stack[0], x); |
There was a problem hiding this comment.
| stack[0] = slt(stack[0], x); | |
| stack[0] = sgt(stack[0], x); |
Where are your unit tests? 🙈
Edit: Did not notice the swapped operand order. Any reason not going for sgt instead?
There was a problem hiding this comment.
it was done to limit EVM-origin additions to intx. That would be different if intx had signed integers implementation.
Moreover, from the evmone PoW, it is better to use less functions - this is at least virtually less code. The LT/GT are implemented using the same pattern. Also because we know that operator> is implemented in terms of operator< - so we are avoiding some additional indirection.
The proposed change should work performance-wise. The issue is some kind of readability-performance tradeoff.
There was a problem hiding this comment.
Maybe just leave a comment above "Notice the swapped operands".
There was a problem hiding this comment.
intx 0.6.0
Haswell 4.0 GHz, clang-12