Conversation
Codecov Report
@@ Coverage Diff @@
## master #5800 +/- ##
==========================================
- Coverage 64.02% 64.02% -0.01%
==========================================
Files 359 359
Lines 30796 30772 -24
Branches 3417 3416 -1
==========================================
- Hits 19718 19702 -16
+ Misses 9855 9845 -10
- Partials 1223 1225 +2 |
| uint64_t s = (uint64_t)m_SP[1]; | ||
| owning_bytes_ref output{std::move(m_mem), b, s}; | ||
| throwRevertInstruction(std::move(output)); | ||
| throwRevertInstruction((uint64_t)m_SP[0], (uint64_t)m_SP[1]); |
There was a problem hiding this comment.
Should we static_cast here?
There was a problem hiding this comment.
Would be better, yes. But I wanted to minimize number of changes.
| if (size < _required) | ||
| BOOST_THROW_EXCEPTION(StackUnderflow() << RequirementError((bigint)_required, size)); | ||
| if (m_stackEnd - m_SPP < _required) | ||
| throw EVMC_STACK_UNDERFLOW; |
There was a problem hiding this comment.
Why don't we include the stack information in the exception anymore?
There was a problem hiding this comment.
Everything is converted into a single evmc_status_code anyway.
| void throwRevertInstruction(owning_bytes_ref&& _output); | ||
| void throwDisallowedStateChange(); | ||
| void throwBufferOverrun(intx::uint512 const& _enfOfAccess); | ||
| static void throwOutOfGas(); |
There was a problem hiding this comment.
Maybe better to make them inline or/and free functions
There was a problem hiding this comment.
Or maybe there's not much sense in these helpers at all, and we can just put throws inline
There was a problem hiding this comment.
This was done previously for performance reasons. I plan to rework this in separate PR with benchmarks.
There was a problem hiding this comment.
I did inlining. From quick benchmarks, the inlining commit gives ~3% improvement. In total the PR gives up to 13% improvement.
| static void throwBadInstruction(); | ||
| static void throwBadJumpDestination(); | ||
| void throwBadStack(int _removed); | ||
| void throwRevertInstruction(uint64_t _offset, uint64_t _size); |
There was a problem hiding this comment.
I'd fill m_output outside of the function and make it static as others
There was a problem hiding this comment.
m_output can then stay private, too
There was a problem hiding this comment.
This is accessed also in EVMC's execute() method. More refactoring is possible to clean up how EVMC result is created and reported.
Instead of using complex exception types from Aleth, throw EVMC status code in simple cases.
chfast
force-pushed
the
interpreter
branch
2 times, most recently
from
6d81224 to
32b30d7
Compare
gumb0
left a comment
gumb0
left a comment
There was a problem hiding this comment.
many unnecessary whitespace changes
| uint64_t dest = decodeJumpDest(_code, pc); | ||
|
|
||
| _pc += 1 + n * 2; // adust inout _pc to opcode after table | ||
| _pc += 1 + n * 2; // adust inout _pc to opcode after table |
There was a problem hiding this comment.
| _pc += 1 + n * 2; // adust inout _pc to opcode after table | |
| _pc += 1 + n * 2; // adjust inout _pc to opcode after table |
| ON_OP(); | ||
| if (m_rev < EVMC_CONSTANTINOPLE) | ||
| throwBadInstruction(); | ||
| throw EVMC_BAD_JUMP_DESTINATION; |
There was a problem hiding this comment.
| throw EVMC_BAD_JUMP_DESTINATION; | |
| throw EVMC_UNDEFINED_INSTRUCTION; |
| NEXT | ||
|
|
||
| CASE(MSTORE) | ||
| CASE(MSTORE) |
There was a problem hiding this comment.
Indentation here... Maybe let's put semicolon after NEXT to work around this?
| CASE(BEGINDATA) | ||
| CASE(GETLOCAL) | ||
| CASE(PUTLOCAL) | ||
| CASE(JUMPTO) CASE(JUMPIF) CASE(JUMPV) CASE(JUMPSUB) CASE(JUMPSUBV) CASE(RETURNSUB) |
There was a problem hiding this comment.
😱 here semicolon won't help, so I don't know what to do other that manually overriding it
There was a problem hiding this comment.
I accidentally formatted whole file. I will have to revert the formatting changes somehow, but I didn't have time to do so so far.
4 participants
No description provided.