Push values optimisation

lib/evmone/analysis.cpp

@@ -9,6 +9,8 @@
 
 namespace evmone
 {
+namespace
+{
 inline constexpr evmc_call_kind op2call_kind(uint8_t opcode) noexcept
 {
     switch (opcode)
@@ -28,6 +30,14 @@ inline constexpr evmc_call_kind op2call_kind(uint8_t opcode) noexcept
     }
 }
 
+inline constexpr uint64_t load64be(const unsigned char* data) noexcept
+{
+    return uint64_t{data[7]} | (uint64_t{data[6]} << 8) | (uint64_t{data[5]} << 16) |
+           (uint64_t{data[4]} << 24) | (uint64_t{data[3]} << 32) | (uint64_t{data[2]} << 40) |
+           (uint64_t{data[1]} << 48) | (uint64_t{data[0]} << 56);
+}
+}  // namespace
+
 code_analysis analyze(
     const exec_fn_table& fns, evmc_revision rev, const uint8_t* code, size_t code_size) noexcept
 {
@@ -38,18 +48,19 @@ code_analysis analyze(
 
     // This is 2x more than needed but using (code_size / 2 + 1) increases page-faults 1000x.
     const auto max_args_storage_size = code_size + 1;
-    analysis.args_storage.reserve(max_args_storage_size);
+    analysis.push_values.reserve(max_args_storage_size);
 
     const auto* instr_table = evmc_get_instruction_metrics_table(rev);
 
     block_info* block = nullptr;
 
     int instr_index = 0;
 
-    for (auto code_pos = code; code_pos < code + code_size; ++code_pos, ++instr_index)
+    const auto code_end = code + code_size;
+    for (auto code_pos = code; code_pos < code_end; ++instr_index)
     {
         // TODO: Loop in reverse order for easier GAS analysis.
-        const auto opcode = *code_pos;
+        const auto opcode = *code_pos++;
 
         const bool jumpdest = opcode == OP_JUMPDEST;
 
@@ -65,7 +76,7 @@ code_analysis analyze(
 
             if (jumpdest)  // Add the jumpdest to the map.
             {
-                analysis.jumpdest_offsets.emplace_back(static_cast<int16_t>(code_pos - code));
+                analysis.jumpdest_offsets.emplace_back(static_cast<int16_t>(code_pos - code - 1));
                 analysis.jumpdest_targets.emplace_back(static_cast<int16_t>(instr_index));
             }
             else  // Increase instruction count because additional BEGINBLOCK was injected.
@@ -87,18 +98,44 @@ code_analysis analyze(
 
         switch (opcode)
         {
-        case ANY_PUSH:
+        case ANY_SMALL_PUSH:
+        {
+            const auto push_size = size_t(opcode - OP_PUSH1 + 1);
+            const auto push_end = code_pos + push_size;
+
+            uint8_t value_bytes[8]{};
+            auto insert_pos = &value_bytes[sizeof(value_bytes) - push_size];
+
+            // TODO: Consier the same endianness-specific loop as in ANY_LARGE_PUSH case.
+            while (code_pos < push_end && code_pos < code_end)
+                *insert_pos++ = *code_pos++;
+            instr.arg.small_push_value = load64be(value_bytes);
+            break;
+        }
+
+        case ANY_LARGE_PUSH:
         {
-            // OPT: bswap data here.
             const auto push_size = size_t(opcode - OP_PUSH1 + 1);
-            auto& data = analysis.args_storage.emplace_back();
-
-            const auto leading_zeros = 32 - push_size;
-            const auto i = code_pos - code + 1;
-            for (size_t j = 0; j < push_size && (i + j) < code_size; ++j)
-                data[leading_zeros + j] = code[i + j];
-            instr.arg.data = &data[0];
-            code_pos += push_size;
+            const auto push_end = code_pos + push_size;
+
+            auto& push_value = analysis.push_values.emplace_back();
+            // TODO: Add as_bytes() helper to intx.
+            const auto push_value_bytes = reinterpret_cast<uint8_t*>(intx::as_words(push_value));
+            auto insert_pos = &push_value_bytes[push_size - 1];
+
+            // Copy bytes to the deticated storage in the order to match native endianness.
+            // The condition `code_pos < code_end` is to handle the edge case of PUSH being at
+            // the end of the code with incomplete value bytes.
+            // This condition can be replaced with single `push_end <= code_end` done once before
+            // the loop. Then the push value will stay 0 but the value is not reachable
+            // during the execution anyway.
+            // This seems like a good micro-optimization but we were not able to show
+            // this is faster, at least with GCC 8 (producing the best results at the time).
+            // FIXME: Add support for big endian architectures.
+            while (code_pos < push_end && code_pos < code_end)
+                *insert_pos-- = *code_pos++;
+
+            instr.arg.push_value = &push_value;
             break;
         }
 
@@ -126,7 +163,7 @@ code_analysis analyze(
             break;
 
         case OP_PC:
-            instr.arg.p.number = static_cast<int>(code_pos - code);
+            instr.arg.p.number = static_cast<int>(code_pos - code - 1);
             break;
 
         case OP_LOG0:
@@ -154,8 +191,8 @@ code_analysis analyze(
 
     // FIXME: assert(analysis.instrs.size() <= max_instrs_size);
 
-    // Make sure the args_storage has not been reallocated. Otherwise iterators are invalid.
-    assert(analysis.args_storage.size() <= max_args_storage_size);
+    // Make sure the push_values has not been reallocated. Otherwise iterators are invalid.
+    assert(analysis.push_values.size() <= max_args_storage_size);
 
     return analysis;
 }

lib/evmone/analysis.hpp

@@ -129,6 +129,8 @@ union instr_argument
         evmc_call_kind call_kind;
     } p;
     const uint8_t* data;
+    const intx::uint256* push_value;
+    uint64_t small_push_value;
 };
 
 static_assert(sizeof(instr_argument) == sizeof(void*), "Incorrect size of instr_argument");
@@ -172,11 +174,8 @@ struct code_analysis
     std::vector<instr_info> instrs;
     std::vector<block_info> blocks;
 
-    /// Storage for arguments' extended data.
-    ///
-    /// The deque container is used because pointers to its elements are not
-    /// invalidated when the container grows.
-    std::vector<bytes32> args_storage;
+    /// Storage for large push values.
+    std::vector<intx::uint256> push_values;
 
     /// The offsets of JUMPDESTs in the original code.
     /// These are values that JUMP/JUMPI receives as an argument.

lib/evmone/instructions.cpp

@@ -660,10 +660,14 @@ void op_gaslimit(execution_state& state, instr_argument) noexcept
     state.stack.push(block_gas_limit);
 }
 
+void op_push_small(execution_state& state, instr_argument arg) noexcept
+{
+    state.stack.push(arg.small_push_value);
+}
+
 void op_push_full(execution_state& state, instr_argument arg) noexcept
 {
-    // OPT: For smaller pushes, use pointer data directly.
-    state.stack.push(intx::be::uint256(arg.data));
+    state.stack.push(*arg.push_value);
 }
 
 void op_pop(execution_state& state, instr_argument) noexcept
@@ -1249,7 +1253,9 @@ constexpr exec_fn_table create_op_table_frontier() noexcept
     table[OP_MSIZE] = op_msize;
     table[OP_GAS] = op_gas;
     table[OPX_BEGINBLOCK] = opx_beginblock;  // Replaces JUMPDEST.
-    for (auto op = size_t{OP_PUSH1}; op <= OP_PUSH32; ++op)
+    for (auto op = size_t{OP_PUSH1}; op <= OP_PUSH8; ++op)
+        table[op] = op_push_small;
+    for (auto op = size_t{OP_PUSH9}; op <= OP_PUSH32; ++op)
         table[op] = op_push_full;
     for (auto op = size_t{OP_DUP1}; op <= OP_DUP16; ++op)
         table[op] = op_dup;

lib/evmone/opcodes_helpers.h

@@ -3,38 +3,40 @@
 // Licensed under the Apache License, Version 2.0.
 #pragma once
 
-#define ANY_PUSH    \
-    OP_PUSH1:       \
-    case OP_PUSH2:  \
-    case OP_PUSH3:  \
-    case OP_PUSH4:  \
-    case OP_PUSH5:  \
-    case OP_PUSH6:  \
-    case OP_PUSH7:  \
-    case OP_PUSH8:  \
-    case OP_PUSH9:  \
-    case OP_PUSH10: \
-    case OP_PUSH11: \
-    case OP_PUSH12: \
-    case OP_PUSH13: \
-    case OP_PUSH14: \
-    case OP_PUSH15: \
-    case OP_PUSH16: \
-    case OP_PUSH17: \
-    case OP_PUSH18: \
-    case OP_PUSH19: \
-    case OP_PUSH20: \
-    case OP_PUSH21: \
-    case OP_PUSH22: \
-    case OP_PUSH23: \
-    case OP_PUSH24: \
-    case OP_PUSH25: \
-    case OP_PUSH26: \
-    case OP_PUSH27: \
-    case OP_PUSH28: \
-    case OP_PUSH29: \
-    case OP_PUSH30: \
-    case OP_PUSH31: \
+#define ANY_SMALL_PUSH \
+    OP_PUSH1:          \
+    case OP_PUSH2:     \
+    case OP_PUSH3:     \
+    case OP_PUSH4:     \
+    case OP_PUSH5:     \
+    case OP_PUSH6:     \
+    case OP_PUSH7:     \
+    case OP_PUSH8
+
+#define ANY_LARGE_PUSH \
+    OP_PUSH9:          \
+    case OP_PUSH10:    \
+    case OP_PUSH11:    \
+    case OP_PUSH12:    \
+    case OP_PUSH13:    \
+    case OP_PUSH14:    \
+    case OP_PUSH15:    \
+    case OP_PUSH16:    \
+    case OP_PUSH17:    \
+    case OP_PUSH18:    \
+    case OP_PUSH19:    \
+    case OP_PUSH20:    \
+    case OP_PUSH21:    \
+    case OP_PUSH22:    \
+    case OP_PUSH23:    \
+    case OP_PUSH24:    \
+    case OP_PUSH25:    \
+    case OP_PUSH26:    \
+    case OP_PUSH27:    \
+    case OP_PUSH28:    \
+    case OP_PUSH29:    \
+    case OP_PUSH30:    \
+    case OP_PUSH31:    \
     case OP_PUSH32
 
 #define ANY_DUP    \

test/unittests/analysis_test.cpp

@@ -72,12 +72,11 @@ TEST(analysis, push)
     const auto analysis = analyze(fake_fn_table, rev, &code[0], code.size());
 
     ASSERT_EQ(analysis.instrs.size(), 4);
-    ASSERT_EQ(analysis.args_storage.size(), 2);
+    ASSERT_EQ(analysis.push_values.size(), 1);
     EXPECT_EQ(analysis.instrs[0].fn, fake_fn_table[OPX_BEGINBLOCK]);
-    EXPECT_EQ(analysis.instrs[1].arg.data, &analysis.args_storage[0][0]);
-    EXPECT_EQ(analysis.instrs[2].arg.data, &analysis.args_storage[1][0]);
-    EXPECT_EQ(analysis.args_storage[0][31 - 7], 0x08);
-    EXPECT_EQ(analysis.args_storage[1][1], 0xee);
+    EXPECT_EQ(analysis.instrs[1].arg.small_push_value, 0x0807060504030201);
+    EXPECT_EQ(analysis.instrs[2].arg.push_value, &analysis.push_values[0]);
+    EXPECT_EQ(analysis.push_values[0], intx::uint256{0xee} << 240);
 }
 
 TEST(analysis, jump1)