Conversation
spotandjake
left a comment
spotandjake
left a comment
There was a problem hiding this comment.
This looks great, I just had one general question.
spotandjake
added
the
Api
|
Closes: #106 |
spotandjake
left a comment
spotandjake
left a comment
There was a problem hiding this comment.
This looks pretty good to me, it matches the binaryen api.
I just had a few small questions, two related to allocating strings and then a few related to c types.
| BinaryenModuleRef module = BinaryenModuleRef_val(_module); | ||
| BinaryenOp op = BinaryenOp_val(_op); | ||
| BinaryenExpressionRef vec = BinaryenExpressionRef_val(_vec); | ||
| int index = Int_val(_index); |
There was a problem hiding this comment.
Should this type be uint8_t to match the binaryen header rather than int?
| BinaryenModuleRef module = BinaryenModuleRef_val(_module); | ||
| BinaryenOp op = BinaryenOp_val(_op); | ||
| BinaryenExpressionRef vec = BinaryenExpressionRef_val(_vec); | ||
| int index = Int_val(_index); |
| CAMLxparam1(_memoryName); | ||
| BinaryenModuleRef module = BinaryenModuleRef_val(_module); | ||
| BinaryenOp op = BinaryenOp_val(_op); | ||
| int offset = Int_val(_offset); |
There was a problem hiding this comment.
These are uint32_t in the C header, similar question to above
| CAMLxparam3(_ptr, _vec, _memoryName); | ||
| BinaryenModuleRef module = BinaryenModuleRef_val(_module); | ||
| BinaryenOp op = BinaryenOp_val(_op); | ||
| int offset = Int_val(_offset); |
| BinaryenOp op = BinaryenOp_val(_op); | ||
| int offset = Int_val(_offset); | ||
| int align = Int_val(_align); | ||
| int index = Int_val(_index); |
There was a problem hiding this comment.
This is uint8_t in the bindinds similar question to above.
| //Requires: Binaryen | ||
| //Requires: caml_jsstring_of_string, caml_alloc_string_on_heap | ||
| function caml_binaryen_simd_load(wasm_mod, op, offset, align, ptr, memoryName) { | ||
| var memory = caml_alloc_string_on_heap(caml_jsstring_of_string(memoryName)); |
There was a problem hiding this comment.
Are we completely safe to use caml_alloc_string_on_heap.
I know in the js api in binaryen you need to call prserveStack before passing strings and I'm just wondering if we have the same semantics.
| vec, | ||
| memoryName | ||
| ) { | ||
| var memory = caml_alloc_string_on_heap(caml_jsstring_of_string(memoryName)); |
| Bytes.set_int32_le bytes 0 low; | ||
| Bytes.set_int32_le bytes 4 low_mid; | ||
| Bytes.set_int32_le bytes 8 high_mid; | ||
| Bytes.set_int32_le bytes 12 high; |
There was a problem hiding this comment.
It seems weird to me that we don't just construct the array directly with bit shifting and masking, but I guess this is less instructions.
2 participants
No description provided.