Skip to content

Conversation

@Robbepop
Copy link
Member

@Robbepop Robbepop commented Feb 14, 2025

Closes #1357.

ToDo

  • memory64
    • Update and adjust MemoryType, MemoryTypeBuilder and Memory types.
    • Implement memory64 support for the following instructions:
      • memory.grow
      • memory.size
      • memory.copy
      • memory.fill
      • memory.init
      • load instructions
        • Translation
        • Execution
        • Translation Tests
          • Fix existing tests.
          • Add new memory64 tests.
        • Execution Tests
      • store instructions
        • Translation
        • Execution
        • Translation Tests
          • Fix existing tests.
          • Add new memory64 tests.
        • Execution Tests
    • Find proper design for Wasmi load instructions to support ptr and offset values of type i64.
    • Find proper design for Wasmi store instructions to support ptr and offset values of type i64.
  • table64
    • Update and adjust TableType and Table types.
    • Implement table64 support for the following instructions:
      • table.grow
      • table.size
      • table.copy
      • table.fill
      • table.init
      • table.get
      • table.set
      • call_indirect
      • return_call_indirect
  • Pass all Wasm spec testsuite tests
  • Evaluate performance and fix potential regressions.

Technical ToDos

  • Create Address32 that has TryFrom<u64> and is infallibly convertible to u64.
    • Used by instructions such as Load32At instead of Const32<u64> type.
  • Create Offset16 that has TryFrom<u64> and is infallibly convertible to Offser64
    • Used by instructions such as Load32Offset16 instead of Const16<u64> type.

New Load Instruction Design

Load32 and Load64

  • Also applicable to:
    • I32Load8s
    • I32Load8u
    • I32Load16s
    • I32Load16u
    • I64Load8s
    • I64Load8u
    • I64Load16s
    • I64Load16u
    • I64Load32s
    • I64Load32u
  • The memory field now is optionally appended to the new encoding scheme.
  • Encodes the lower 32-bit of offset as offset_lo and the upper 32-bit of offset as offset_hi.
  • Upon execution offset_lo and offset_hi are combined as (offset_hi << 32) | offset_lo to form offset.
  • Downsides:
    • Now uses 3 instead of 2 instruction words if a non-default memory index is used.
    • Now always has to compute the (offset_hi << 32) | offset_lo before performing the load operation.

Example for Load32. Load64 is very similar.

/// Load instruction for 32-bit values.
///
/// # Note
///
/// Equivalent to Wasm `{i32,f32}.load` instruction.
///
/// # Encoding
///
/// Followed by
/// 
/// 1. [`Instruction::RegisterAndImm32`]: encoding `ptr` and `offset_hi`
/// 2. Optional [`Instruction::MemoryIndex`]: encoding the used `memory`
/// 
/// Uses the default memory if [`Instruction::MemoryIndex`] is missing.
#[snake_name(load32)]
Load32 {
    @result: Reg,
    /// The linear memory index for which the load instruction is executed.
    offset_lo: u32,
}

Load32At and Load64At

  • No encoding or decoding changes.
  • Now only used if the ptr+offset (address) fits into a u32 value.
  • Downsides:
    • Has to allocate a 0 function local constant if address does not fit in a u32.
      • Note that address can only ever not fit in a u32 if memory64 is enabled.
  • Same applicable to:
    • I32LoadAt8s
    • I32LoadAt8u
    • I32LoadAt16s
    • I32LoadAt16u
    • I64LoadAt8s
    • I64LoadAt8u
    • I64LoadAt16s
    • I64LoadAt16u
    • I64LoadAt32s
    • I64LoadAt32u

Load32Offset16 and Load64Offset16

  • No encoding or decoding changes.
  • Still only used when the default memory is used and the offset can be encoded as u16 value.
  • Same applicable to:
    • I32Load8sOffset16
    • I32Load8uOffset16
    • I32Load16sOffset16
    • I32Load16uOffset16
    • I64Load8sOffset16
    • I64Load8uOffset16
    • I64Load16sOffset16
    • I64Load16uOffset16
    • I64Load32sOffset16
    • I64Load32uOffset16

New Store Instruction Design

Store32 and Store64

  • The memory field now is optionally appended to the new encoding scheme.
  • Encodes the lower 32-bit of offset as offset_lo and the upper 32-bit of offset as offset_hi.
  • Upon execution offset_lo and offset_hi are combined as (offset_hi << 32) | offset_lo to form offset.
  • Downsides:
    • Now uses 3 instead of 2 instruction words if a non-default memory index is used.
    • Now always has to compute the (offset_hi << 32) | offset_lo before performing the store operation.
  • Same applicable to:
    • I32StoreImm16
    • I32Store8
    • I32Store8Imm
    • I32Store16
    • I32Store16Imm
    • I64StoreImm16
    • I64Store8
    • I64Store8Imm
    • I64Store16
    • I64Store16Imm
    • I64Store32
    • I64Store32Imm

Example Store32

/// Store instruction for 32-bit values.
///
/// # Note
///
/// Equivalent to Wasm `{i32,f32}.store` instruction.
///
/// # Encoding
///
/// Followed by
/// 
/// 1. [`Instruction::RegisterAndImm32`]: encoding `value` and `offset_hi`
/// 2. Optional [`Instruction::MemoryIndex`]: encoding the `memory` to operate on
#[snake_name(store32)]
Store32 {
    /// The register storing the pointer of the `store` instruction.
    ptr: Reg,
    /// The lower 32-bits of the offset.
    offset_lo: u32,
}

Store32At and Store64At

  • No encoding or decoding changes.
  • Now only used if the ptr+offset (address) fits into a u32 value.
  • Downsides:
    • Has to allocate a 0 function local constant if address does not fit in a u32.
      • Note that address can only ever not fit in a u32 if memory64 is enabled.
  • Same applicable to:
    • I32StoreAtImm16
    • I32Store8AtImm
    • I32Store16AtImm
    • I64StoreAtImm16
    • I64Store8AtImm
    • I64Store16AtImm
    • I64Store32AtImm16

Store32Offset16 and Store64Offset16

  • No encoding or decoding changes.
  • Still only used if:
    • The default memory is used.
    • The offset fits into a u16 value.
  • Same applicable to:
    • I32Store8Offset16
    • I32Store8Offset16Imm
    • I32Store16Offset16
    • I32Store16Offset16Imm
    • I64Store8Offset16
    • I64Store8Offset16Imm
    • I64Store16Offset16
    • I64Store16Offset16Imm
    • I64Store16Offset32
    • I64Store16Offset32Imm16

This allows to share code and state between MemoryType and MemoryTypeBuilder. The type itself is internal to the memory submodule and not part of the public API of the memory type.
The incorrect behavior was that memory64 was true for all memories if the flag was enabled in the Config. This is nonsense since memory64 allows memories to be of either 32-bit or 64-bit and does not turn all memories into 64-bit. Thus, now the translator queries the particular memory type which is the correct behavior.
This was only done to fix bugs with the old and incorrect memory64 query behavior.
Robbepop added 13 commits March 9, 2025 09:21
The issue was that Wasmi translation handled all 3 parameters to memory.init as i64 type values just like it has to handle all the other memory instructions. However, memory.init is special in that its len and data_src parameters remain i32 types. This is probably because memory64 allows for 64-bit memories and tables but not for 64-bit data and element segments. As of now I do not have an explanation or rational for this but it is what it is.
This is so we can easily re-use it for TableType later.
This is going to be useful later.
@Robbepop
Copy link
Member Author

Robbepop commented Mar 9, 2025

Since the official Wasm spec testsuite was ported over to Wasm 3.0 and Wasmi does not (yet) support Wasm 3.0 we need to have 2 different Wasm spec testsuite Git submodules in our tree: the main one tracking the most up-to-date state so we keep most tests up-to-date, and another one specifically based on b66a09a which is the last commit which had all the necessary memory64 tests neatly in one place.

@codecov
Copy link

codecov bot commented Mar 10, 2025

Codecov Report

Attention: Patch coverage is 80.96591% with 134 lines in your changes missing coverage. Please review.

Project coverage is 70.97%. Comparing base (6944344) to head (a841eb9).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crates/wasmi/src/engine/executor/instrs/memory.rs 60.52% 30 Missing ⚠️
crates/wasmi/src/table/mod.rs 74.75% 26 Missing ⚠️
crates/wasmi/src/engine/executor/instrs/table.rs 53.06% 23 Missing ⚠️
crates/wasmi/src/memory/mod.rs 83.52% 14 Missing ⚠️
crates/wasmi/src/engine/executor/instrs.rs 80.00% 12 Missing ⚠️
crates/wasmi/src/engine/translator/mod.rs 91.02% 7 Missing ⚠️
crates/wasmi/src/table/error.rs 0.00% 5 Missing ⚠️
crates/wasmi/src/engine/translator/utils.rs 86.20% 4 Missing ⚠️
crates/wasmi/src/memory/error.rs 0.00% 3 Missing ⚠️
crates/ir/src/enum.rs 83.33% 2 Missing ⚠️
... and 5 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1371      +/-   ##
==========================================
+ Coverage   70.79%   70.97%   +0.18%     
==========================================
  Files         153      154       +1     
  Lines       14324    14534     +210     
==========================================
+ Hits        10141    10316     +175     
- Misses       4183     4218      +35     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Robbepop
Copy link
Member Author

I just ran the benchmarks to compare main with this PR's branch and saw no performance difference across the entire set of tests. Noticably there were minor improvements but these were all within the margin of error.

@Robbepop Robbepop marked this pull request as ready for review March 10, 2025 11:37
@Robbepop Robbepop merged commit 8b9d192 into main Mar 10, 2025
19 checks passed
@Robbepop Robbepop deleted the rf-implement-memory64 branch March 10, 2025 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement the Wasm memory64 proposal

1 participant