Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add OCaml realization for Int.Cast #2315

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

antoniolocascio
Copy link
Contributor

This PR closes #2303.
As explained on the issue, giving an OCaml realization to the Int.Cast module improves its performance significantly.

@antoniolocascio
Copy link
Contributor Author

antoniolocascio commented Jun 17, 2021

For now, I just used the old extracted F# code for FStar.Int.Cast. As a consequence, there's no performance gain in F#, but at least the implementation of FStar_Int_Cast.fs is correct.

(* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! *)
(* THIS FILE WAS EXTRACTED FROM F* *)
(* IT HAS TO BE COPIED HERE BECAUSE THIS MODULE IS NOW REALIZED IN
OCAML *)
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be fine to leave both fst and fsti in ulib, and only add OCaml implementation inside ml subdirectory? I'm not 100% sure, but afair F* was smart enough to use the realized module if it was available and otherwise rely on extraction.

With F#, what is being used is driven with fsproj, but I'm not completely sure how makefiles for OCaml will behave with both fst and ml files present.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mateuszbujalski I didn’t realize I could do that, it's much cleaner that way. Thanks!

@antoniolocascio antoniolocascio changed the title WIP: Add OCaml realization for Int.Cast Add OCaml realization for Int.Cast Jun 18, 2021
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.

Realize FStar.Int.Cast for better perfomance
2 participants