Skip to content

Refactor otherlibs build#2477

Merged
dkalinichenko-js merged 9 commits intof32-litfrom
f32-lib
Apr 29, 2024
Merged

Refactor otherlibs build#2477
dkalinichenko-js merged 9 commits intof32-litfrom
f32-lib

Conversation

@TheNumbat
Copy link
Copy Markdown
Member

@TheNumbat TheNumbat commented Apr 19, 2024

This refactors how the alpha/beta/stable otherlibs are built.

  • They are no longer included in the upstream build, since they will use extensions that the upstream compiler doesn't support. Hence they do not need a Makefile-based build.
  • They are now compiled with their corresponding extension universe enabled.
  • The jst makefile now copies all ml/mli files from the otherlibs dirs, not just alpha*, etc.
  • The tests for exported modules have been moved from ocaml/testsuite/ to testsuite/, so will only run in flambda-backend builds. The upstream compatible versions (only modules.ml and upstream_compatible.ml) are still under ocaml/testsuite.
  • Added tests for alpha/beta/upstream_compatible as well as stable.
  • Enabled ocamlformat for these libs

@TheNumbat TheNumbat changed the title float32 otherlib Refactor otherlibs build Apr 24, 2024
@TheNumbat TheNumbat marked this pull request as ready for review April 24, 2024 21:31
@dkalinichenko-js
Copy link
Copy Markdown
Collaborator

  • They are no longer included in the upstream build, since they will use extensions that the upstream compiler doesn't support. Hence they do not need a Makefile-based build.
  • The tests for exported modules have been moved from ocaml/testsuite/ to testsuite/, so will only run in flambda-backend builds. The upstream compatible versions (only modules.ml and upstream_compatible.ml) are still under ocaml/testsuite.

As far as I know, the makefile-based build inside this repository is used not by the upstream compiler, but by our compiler. ocaml/testsuite/ also contains numerous tests that only work inside our compiler, such as layouts tests. So I'm confused why would we move the otherlibs tests to testsuite/.

Co-authored-by: dkalinichenko-js <118547217+dkalinichenko-js@users.noreply.github.com>
@TheNumbat
Copy link
Copy Markdown
Member Author

Some time ago we switched flambda-backend builds to use dune whenever available - I confirmed the lib-extensions tests still run under make test here, so it seems fine.

The layouts tests actually work in the upstream compiler build because they only require frontend features present in ocaml/. As of this PR it's not yet necessary to move the otherlibs tests, but it will be when I add the Float32 module, because it requires backend features that are not present in ocaml/.

@dkalinichenko-js
Copy link
Copy Markdown
Collaborator

I think you should move upstream_compatible to be flambda-only then too. upstream_compatible does not actually mean "builds as is with the upstream compiler", instead it's "something we provide an upstream-compatible substitute shim for, even if it degrades performance". So, for example, Float_u can be flambda-only, yet still be in upstream_compatible if we have a module with the same interface (modulo extension erasure stuff) and an upstream-compatible implementation, even if that implementation just uses normal float.

@TheNumbat
Copy link
Copy Markdown
Member Author

Sounds good, upstream_compatible is now treated the same as the others

@dkalinichenko-js
Copy link
Copy Markdown
Collaborator

Should we merge this?

@dkalinichenko-js dkalinichenko-js merged commit 736b9b4 into f32-lit Apr 29, 2024
@dkalinichenko-js dkalinichenko-js deleted the f32-lib branch April 29, 2024 19:21
dkalinichenko-js added a commit that referenced this pull request Apr 29, 2024
@dkalinichenko-js dkalinichenko-js restored the f32-lib branch April 29, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants