initial bulk array op implementations #363

conrad-watt · 2023-03-27T19:02:25Z

See #313, and more specifically this comment.

This patch only updates the interpreter, the overview is not updated.

Note - this patch currently using the following opcode encodings in the hope of provoking more permanent suggestions:

| ArrayCopy (x, y)     -> op 0xfb; op 0x18; var x; var y
| ArrayFill x          -> op 0xfb; op 0x0f; var x
| ArrayInitData (x, y) -> op 0xfb; op 0x52; var x; var y
| ArrayInitElem (x, y) -> op 0xfb; op 0x53; var x; var y

tlively · 2023-03-27T19:29:56Z

The opcodes here seem fine to me. We can figure out permanent opcodes alongside everything else just before we go to phase 4.

jakobkummerow · 2023-03-30T09:56:04Z

For the record, 0xfb52 and 0xfb53 are still used by the old ref.is_i31 and ref.is_data (which have already disappeared from this repository, but are still supported by the prototype implementations for incremental backwards compatibility, and we have received a specific request to continue supporting them for a little while longer).
So for implementing array.init_from_{data,elem}, we'll use a different binary encoding, e.g. 0xfb54 and 0xfb55 are free. As always, we'll keep https://bit.ly/3cWcm6Q updated. Feel free to switch the encoding that this PR uses, or feel free to keep what you have for now; it'll all get resolved in the big final reshuffling anyway.

conrad-watt · 2023-03-30T11:07:13Z

I'm happy to switch this PR to match the reservations listed in that doc (IIUC, 0xfb54 and 0xfb55).

tlively · 2023-04-04T17:53:14Z

What needs to happen to get this landed? Do we need an update to MVP.md to accompany the work here, or can that come later?

conrad-watt · 2023-05-02T16:07:59Z

@rossberg PTAL!

interpreter/valid/valid.ml

test/core/gc/array_bulk.wast

interpreter/exec/eval.ml

rossberg · 2023-05-09T07:47:22Z

interpreter/exec/eval.ml

+          let seg = data c.frame.inst y in
+          let bs = Data.bytes seg in
+          let j = I32.to_int_u y_off in
+          (* TODO: unify with ArrayNewData *)


Can you extend the Data module with a load_num function analogous to Memory.load_num and use that here and for ArrayNewData?

opening Type/Value in Data seems to cause a build circularity. For now I'm factoring this out into a shared helper in eval.ml.

Hm, that is very odd. What's the circularity? I cannot reproduce it on main.

interpreter/exec/eval.ml

conrad-watt · 2023-05-15T17:50:28Z

@rossberg PTAL

interpreter/exec/eval.ml

interpreter/valid/valid.ml

interpreter/exec/eval.ml

rossberg · 2023-05-16T10:13:01Z

interpreter/exec/eval.ml

  match v with
  | Num n -> Plain (Const (n @@ at))
  | Vec v -> Plain (VecConst (v @@ at))
  | Ref r -> Refer r

+let value_of_data_ind (seg : Data.data) (st : storage_type) (j : int) at =


Can we somehow make this mirror the load_num/load_vec functions in Memory.ml?

I'm not sure what you have in mind here. Now that I've at least pulled this out as an auxiliary definition, would you be willing to take on refactoring it as a small follow-up PR?

rossberg · 2023-05-16T10:15:09Z

interpreter/exec/eval.ml

+          let seg = data c.frame.inst y in
+          let bs = Data.bytes seg in
+          let j = I32.to_int_u y_off in
+          (* TODO: unify with ArrayNewData *)


Hm, that is very odd. What's the circularity? I cannot reproduce it on main.

test/core/gc/array_bulk.wast

Co-authored-by: Andreas Rossberg <rossberg@dfinity.org>

conrad-watt · 2023-05-16T18:06:49Z

@rossberg PTAL, everything should be resolved except the issue with value_of_data_ind (see comment).

interpreter/valid/valid.ml

interpreter/exec/eval.ml

interpreter/valid/valid.ml

rossberg · 2023-05-16T18:18:01Z

interpreter/exec/eval.ml

  match v with
  | Num n -> Plain (Const (n @@ at))
  | Vec v -> Plain (VecConst (v @@ at))
  | Ref r -> Refer r

+let value_of_data_ind (seg : Data.data) (st : storage_type) (j : int) at =


conrad-watt · 2023-05-16T18:27:04Z

merging now 🥳

initial bulk array op implementations

6a4de1e

conrad-watt mentioned this pull request Mar 27, 2023

MVP array.copy #313

Closed

add segment drop tests

953b0a1

tests -add non-trivial subtyping to ref cases

537116e

align with prototype encoding

131871e

conrad-watt requested a review from rossberg May 2, 2023 16:06

rossberg reviewed May 9, 2023

View reviewed changes

jakobkummerow mentioned this pull request May 9, 2023

Final opcodes #372

Merged

conrad-watt added 2 commits May 15, 2023 18:38

comments

c299690

line length?

3e4650b

rossberg reviewed May 16, 2023

View reviewed changes

conrad-watt and others added 13 commits May 16, 2023 11:32

Update interpreter/exec/eval.ml

8ad8552

Co-authored-by: Andreas Rossberg <rossberg@dfinity.org>

Update interpreter/exec/eval.ml

6789b72

Co-authored-by: Andreas Rossberg <rossberg@dfinity.org>

Update interpreter/exec/eval.ml

86adef7

Co-authored-by: Andreas Rossberg <rossberg@dfinity.org>

Update interpreter/exec/eval.ml

7a8470b

Co-authored-by: Andreas Rossberg <rossberg@dfinity.org>

Update interpreter/exec/eval.ml

1f5027c

Co-authored-by: Andreas Rossberg <rossberg@dfinity.org>

Update interpreter/exec/eval.ml

d695271

Co-authored-by: Andreas Rossberg <rossberg@dfinity.org>

Update interpreter/valid/valid.ml

738c08d

Co-authored-by: Andreas Rossberg <rossberg@dfinity.org>

Update interpreter/exec/eval.ml

bd52917

Co-authored-by: Andreas Rossberg <rossberg@dfinity.org>

Update interpreter/exec/eval.ml

fe79afa

Co-authored-by: Andreas Rossberg <rossberg@dfinity.org>

Update interpreter/exec/eval.ml

b7e9081

Co-authored-by: Andreas Rossberg <rossberg@dfinity.org>

Update interpreter/exec/eval.ml

206d643

Co-authored-by: Andreas Rossberg <rossberg@dfinity.org>

Update interpreter/exec/eval.ml

07a375d

Co-authored-by: Andreas Rossberg <rossberg@dfinity.org>

split tests, fix var naming issues

b2d5a9d

conrad-watt added 3 commits May 16, 2023 18:11

Merge remote-tracking branch 'origin/main' into array_bulk_1

3d2f2e7

rebase fallout

759fba4

add array bounds check helper

f45a33d

conrad-watt commented May 16, 2023

View reviewed changes

interpreter/valid/valid.ml Show resolved Hide resolved

rossberg approved these changes May 16, 2023

View reviewed changes

nit

8091000

conrad-watt merged commit afa3e66 into main May 16, 2023
1 check passed

conrad-watt deleted the array_bulk_1 branch May 16, 2023 18:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

initial bulk array op implementations #363

initial bulk array op implementations #363

conrad-watt commented Mar 27, 2023

tlively commented Mar 27, 2023

jakobkummerow commented Mar 30, 2023

conrad-watt commented Mar 30, 2023

tlively commented Apr 4, 2023

conrad-watt commented May 2, 2023

rossberg May 9, 2023

conrad-watt May 15, 2023

rossberg May 16, 2023

conrad-watt commented May 15, 2023

rossberg May 16, 2023

conrad-watt May 16, 2023

rossberg May 16, 2023

rossberg May 16, 2023

conrad-watt commented May 16, 2023

rossberg May 16, 2023

conrad-watt commented May 16, 2023

initial bulk array op implementations #363

initial bulk array op implementations #363

Conversation

conrad-watt commented Mar 27, 2023

tlively commented Mar 27, 2023

jakobkummerow commented Mar 30, 2023

conrad-watt commented Mar 30, 2023

tlively commented Apr 4, 2023

conrad-watt commented May 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conrad-watt commented May 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conrad-watt commented May 16, 2023

Choose a reason for hiding this comment

conrad-watt commented May 16, 2023