Skip to content

Compat changes for wasm 2.0.0#267

Merged
tahina-pro merged 5 commits intomasterfrom
wasm-2.0.0
Jun 8, 2022
Merged

Compat changes for wasm 2.0.0#267
tahina-pro merged 5 commits intomasterfrom
wasm-2.0.0

Conversation

@msprotz
Copy link
Copy Markdown
Contributor

@msprotz msprotz commented May 31, 2022

To be merged after ocaml/opam-repository#21446 gets merged.

@msprotz
Copy link
Copy Markdown
Contributor Author

msprotz commented Jun 1, 2022

I added a commit that now allows returning I64's across the boundary as a multi-value made up of two I32s. This is required to expose Hacl_Bignum64_add for @danwallach

@msprotz msprotz requested review from W95Psp and denismerigoux June 1, 2022 23:53
Comment thread src/AstToCFlat.ml

(* See digression for [dup32] in CFlatToWasm *)
let scratch_locals =
[ I64; I64; I32; I32 ]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the top-of-the-stack pointer is now always saved in the fifth local

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(as opposed to being left on the stack, which used to require contortions to swap operands at the top of the stack)

Comment thread src/AstToCFlat.ml
WASM."

let mk_decl env (d: decl): env * CF.decl option =
let mk_wrapper orig_name n_args locals =
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the strategy for returning 64-bit integers to javascript is as follows:

  • if the function f is part of the public API
  • and returns a uint64
    then we generate a _packed variant that calls f, then decomposes the uint64 return value as a pair of uint32s

since krml does not take advantage of multi-value returns (it should! but these weren't supported when I wrote the backend), a special instruction here allows converting a 64-bit integer into two 32-bit integers...

in the future, it'd be good to add general support for multi-values, and then simply call WasmSupport.u64tou32s of type uint64 -> uint32 & uint32

Comment thread src/CFlatToWasm.ml
in
(if name <> "WasmSupport_align_64" then Debug.mk env debug_enter else []) @
read_highwater @
[ dummy_phrase (W.Ast.LocalSet (mk_var (env.n_args + 4))) ] @
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here, instead of leaving the top-of-the-(memory)-stack pointer on top of the (operand) stack, we save it in the reserved local

@msprotz msprotz marked this pull request as ready for review June 1, 2022 23:58
Copy link
Copy Markdown
Contributor

@denismerigoux denismerigoux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, even though I didn't have a good knowledge of that part of the codebase.

Copy link
Copy Markdown
Collaborator

@W95Psp W95Psp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know much about Karamel's extraction to WASM, but this seems good to me :)

@tahina-pro tahina-pro merged commit 28a9d4a into master Jun 8, 2022
@msprotz msprotz deleted the wasm-2.0.0 branch June 9, 2022 16:33
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.

4 participants