Should copying between 32 and 64-bit memories be allowed?

[alexcrichton]

With the upcoming multi-memory proposal the memory.copy instruction will be allowed to copy between two memories, so I'm curious if the intention with this proposal is to disallow or allow copying between memories of those types?

Disallowing it seems easy to me, but allowing it also seems not too hard (and possibly quite useful!). When allowing though the validation needs to be tweaked somewhat such that if either the source or destination are 64-bit memories then 64-bit offsets and such are required (I think).

[binji]

Good question! I think I'd start with disallowing it, just because it is somewhat complex and we can add it later. But if we do decide to design it now, then I think you'd want to have the instruction change signature based on the memory types (similar to other instructions). Then the size would be limited to the smaller of the two index types, i.e.

given m32 and m64 as 32-bit and 64-bit memories, then we have the following signatures:

memory.copy $m32 $m32: [i32 i32 i32] -> []
memory.copy $m32 $m64: [i32 i64 i32] -> []
memory.copy $m64 $m32: [i64 i32 i32] -> []
memory.copy $m64 $m64: [i64 i64 i64] -> []

[alexcrichton]

Ah right that's a good point, and makes sense to me!

Thinking a bit more, it may be best to basically transform this issue into "add some overview words for the interaction with the bulk memory proposal" now that it's at stage 4. I think memory.fill is pretty straightforward, this issue so far is memory.copy, but for memory.init I initially thought the data segment offset and length would want to match the memory indexing type but since data segments are bounded by i32 it probably makes more sense to only have an i64 offset into memory and the other two indices are i32.

[aardappel]

@alexcrichton I think the bit-ness of memory.init etc is clearly specified now in the overview: https://github.com/WebAssembly/memory64/blob/master/proposals/memory64/Overview.md

I am going to mark the idea of copying between different memories as post-mvp, especially since we don't even have multiple memories yet.

[ghost]

hi guys, I was playing with the experimental support in chromium for wasm64, however, chromium throws on compilation complaining about memory.copy $m64 $m64: [i64 i64 i64] -> [] with message Uncaught CompileError: WebAssembly.instantiateStreaming(): Compiling function #0:"test" failed: memory.copy[2] expected type i32, found i64.const of type i64 @+114.

For reference, the web assembly code I was using is the below (generated by rustc from ptr::copy)

(module
  (table $table0 1 1 funcref)
  (memory $memory (;0;) (export "memory") 16)
  (global $__stack_pointer (;0;) (mut i64) (i64.const 1048576))
  (global $__data_end (;1;) (export "__data_end") i64 (i64.const 1048576))
  (global $__heap_base (;2;) (export "__heap_base") i64 (i64.const 1048576))
  (func $test (;0;) (export "test") (param $var0 i64) (param $var1 i64)
    local.get $var1
    local.get $var0
    i64.const 2400
    memory.copy
  )
)

As per the spec, this would be something that should work?

[lars-t-hansen]

Your memory is not a memory64 though, you need (memory $memory (export "memory") i64 16).

[ghost]

@lars-t-hansen okay the snippet was wat decoded by chrome and shown in developer tools, I tried decoding the same wasm binary with wasm2wat and it generated the below wat

(module
  (type (;0;) (func (param i64 i64)))
  (func $test (type 0) (param i64 i64)
    local.get 1
    local.get 0
    i64.const 2400
    memory.copy)
  (table (;0;) 1 1 funcref)
  (memory (;0;) i64 16)
  (global $__stack_pointer (mut i64) (i64.const 1048576))
  (global (;1;) i64 (i64.const 1048576))
  (global (;2;) i64 (i64.const 1048576))
  (export "memory" (memory 0))
  (export "test" (func $test))
  (export "__data_end" (global 1))
  (export "__heap_base" (global 2)))

[lars-t-hansen]

@jiby-gh, that snippet compiles fine in the Firefox JS shell. If Chrome is giving you trouble it is possible it has not implemented memory.copy for memory64 yet.

[ghost]

@lars-t-hansen right, thanks a lot for confirming, I will file a report with chrome (https://bugs.chromium.org/p/chromium/issues/detail?id=1281995), given that memory64 is experimental, i guess it's most probably not implemented yet as you pointed out.

[sbc100]

Reviving this ancient issue since it came up when adding table64. It looks like #7 added text to the overview that does allow copying between memories with different index types. Sadly we can't really add test for this until multi-memory lands upstream. Do folks think this is necessary or should we instead just disallow this?

[alexcrichton]

Personally I think it would be good to support this. I don't have a use case in mind that would want to use it, but otherwise copying between different-sized memories would have to have a special case of must-be-loads-and-stores where other copies can use memory.copy. Put another way it feels to me more consistent to support it, which is why I'd be in favor of supporting it. Also from an engine/tooling perspective the only moderately difficult part was handling this in fuzzing and generating copies between memories of different sizes, but that's mostly in the complexity of the fuzz-test-case-generator that we have.

[rossberg]

I agree that it would be odd not to support this. The length-type-is-the-min-of-both-index-types part is slightly ugly, but still okay.

[sbc100]

Ok, I'm going to close this issue since the answer seems to be "Yes"