Skip to content

Merge Types.* and Subst.Lazy.* types#1312

Merged
lpw25 merged 5 commits intooxcaml:mainfrom
rleshchinskiy:merge-lazy-types
Apr 18, 2023
Merged

Merge Types.* and Subst.Lazy.* types#1312
lpw25 merged 5 commits intooxcaml:mainfrom
rleshchinskiy:merge-lazy-types

Conversation

@rleshchinskiy
Copy link
Copy Markdown
Contributor

This merges the type definitions in Types and Subst.Lazy by abstracting over the bits of the AST that can be lazy in the new functor in types.mli (cg. Types.Pod), accompanied by a generic scheme for converting between different kinds of ASTs in Types.Map_pods. The conversions to and from lazy ASTs in Subst are replaced by this.

The laziness in Subst is slightly more generic than what we had before. This is in anticipation of a future PR which will allow us to lazily deserialise .cmi files by piggy-backing on the same mechanism.

There is also a bit of refactoring in Env which removes some code duplication and addresses comments in #1228.

I have observed no changes to compile times in my benchmarks. There is a regression when building the compiler with flambda2 without optimisations but we shouldn't do that (cf. #1291).

Roman Leshchinskiy added 2 commits April 13, 2023 15:26
We introduce a functor which generates the types instead of duplicating
them. The main benefit (in addition to removing code duplication) is
that we abstract over things which can be lazy via the Pod mechanism.
Going forward, this will allow us to make more thing lazy (esp. the
`type_expr` in `value_description` in the next set of changes) but also
to reuse the same mechanism to deserialise cmi files lazily.
This removes duplicated code introduced oxcaml#1228 and also gets rid of some
temporary tuples as per the comments on that PR.
@goldfirere
Copy link
Copy Markdown
Collaborator

I don't feel I have the right expertise to comment on the merits, but this looks like a patch we should aim to upstream sooner than later, so that we can avoid merge conflicts.

Copy link
Copy Markdown
Collaborator

@lpw25 lpw25 left a comment

Choose a reason for hiding this comment

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

Some small suggestions, but this is fine to merge.

@rleshchinskiy
Copy link
Copy Markdown
Contributor Author

I don't feel I have the right expertise to comment on the merits, but this looks like a patch we should aim to upstream sooner than later, so that we can avoid merge conflicts.

This depends on #1228 so if we want to upstream things then that one needs to be included. I'm happy to do whatever is necessary if we decide to upstream.

@lpw25 lpw25 merged commit e5d94d0 into oxcaml:main Apr 18, 2023
@rleshchinskiy rleshchinskiy deleted the merge-lazy-types branch April 18, 2023 10:06
mshinwell added a commit to mshinwell/oxcaml that referenced this pull request Apr 28, 2023
a7d005a flambda-backend: Lazy deserialization of cmi files (oxcaml#1322)
aa83fa3 flambda-backend: Reinstate previous API for Env.lookup_value (oxcaml#1323)
e4007a4 flambda-backend: Lazy substitution into value_declaration (oxcaml#1320)
634b607 flambda-backend: Merge Types.* and Subst.Lazy.* types (oxcaml#1312)
cf82708 flambda-backend: Bump magic numbers for 4.14.1-7 (oxcaml#1317)
6470400 flambda-backend: zero_alloc attribute payload "assert all" and "ignore" (oxcaml#1296)
bba5248 flambda-backend: Teach `ocamldep` about all the language extensions (oxcaml#1303)
33e97b0 flambda-backend: Change Includemod to work on lazy modtypes (oxcaml#1228)
16e5002 flambda-backend: zero_alloc new warning for unchecked functions (oxcaml#1302)
36b4626 flambda-backend: Attribute [@@@zero_alloc check] to turn the check on (oxcaml#1294)
3b524c6 flambda-backend: Cmm.value_kind cleanup (oxcaml#1091)
ec99505 flambda-backend: Fix failure of `check_all_arches` on RISCV (oxcaml#1300)
450bc58 flambda-backend: Backend changes for multiple returns (oxcaml#1268)
84a7a26 flambda-backend: Static check for zero_alloc: ignore allocation that lead to exceptional return (oxcaml#1157)
1723728 flambda-backend: Re-enable parallel build of the runtime (oxcaml#1287)
26ea7f3 flambda-backend: Fix closure marshalling when not in NNP mode (oxcaml#1286)
9b91f2e flambda-backend: Reduce number of caml_apply functions taking/returning "I" and "V" (oxcaml#1272)
1686928 flambda-backend: Restore Cmm unboxing behaviour inside regions (oxcaml#1285)
cf9be42 flambda-backend: Fix all the no-naked-pointers problems (oxcaml#1282)
8fe089e flambda-backend: Unrevert oxcaml#1131 and fix a Cmm unboxing bug (oxcaml#1284)
c4143c3 flambda-backend: Revert "Make Selectgen treat region boundaries more precisely" (oxcaml#1283)
2078dce flambda-backend: Add some -dtimings output for the typechecker (oxcaml#1245)
273a7f9 flambda-backend: Make Selectgen treat region boundaries more precisely (oxcaml#1131)
47610e6 flambda-backend: Bump magic numbers for 4.14.1-6 (oxcaml#1274)
fd53d38 flambda-backend: Generate *.cms files for merlin (oxcaml#1232)
853f95f flambda-backend: Add tail_mod_const to builtin_attrs (oxcaml#1265)
f9ef051 flambda-backend: Fix issue caused by effects of gadt expansion in mode cross check (oxcaml#1263)
e9ffcf8 flambda-backend: Fix dependencies for regenerating Flambda2 parser, tests (oxcaml#1255)
6f1cd1f flambda-backend: Restore a lost location, needed for merlin (oxcaml#1242)
009332b flambda-backend: Fix merge from ocaml-jst

git-subtree-dir: ocaml
git-subtree-split: a7d005a
ccasin added a commit to ccasin/oxcaml that referenced this pull request Apr 29, 2023
a7d005a flambda-backend: Lazy deserialization of cmi files (oxcaml#1322)
aa83fa3 flambda-backend: Reinstate previous API for Env.lookup_value (oxcaml#1323)
e4007a4 flambda-backend: Lazy substitution into value_declaration (oxcaml#1320)
634b607 flambda-backend: Merge Types.* and Subst.Lazy.* types (oxcaml#1312)
cf82708 flambda-backend: Bump magic numbers for 4.14.1-7 (oxcaml#1317)
6470400 flambda-backend: zero_alloc attribute payload "assert all" and "ignore" (oxcaml#1296)
bba5248 flambda-backend: Teach `ocamldep` about all the language extensions (oxcaml#1303)
33e97b0 flambda-backend: Change Includemod to work on lazy modtypes (oxcaml#1228)
16e5002 flambda-backend: zero_alloc new warning for unchecked functions (oxcaml#1302)
36b4626 flambda-backend: Attribute [@@@zero_alloc check] to turn the check on (oxcaml#1294)
3b524c6 flambda-backend: Cmm.value_kind cleanup (oxcaml#1091)
ec99505 flambda-backend: Fix failure of `check_all_arches` on RISCV (oxcaml#1300)
450bc58 flambda-backend: Backend changes for multiple returns (oxcaml#1268)
84a7a26 flambda-backend: Static check for zero_alloc: ignore allocation that lead to exceptional return (oxcaml#1157)
1723728 flambda-backend: Re-enable parallel build of the runtime (oxcaml#1287)
26ea7f3 flambda-backend: Fix closure marshalling when not in NNP mode (oxcaml#1286)
9b91f2e flambda-backend: Reduce number of caml_apply functions taking/returning "I" and "V" (oxcaml#1272)
1686928 flambda-backend: Restore Cmm unboxing behaviour inside regions (oxcaml#1285)
cf9be42 flambda-backend: Fix all the no-naked-pointers problems (oxcaml#1282)
8fe089e flambda-backend: Unrevert oxcaml#1131 and fix a Cmm unboxing bug (oxcaml#1284)
c4143c3 flambda-backend: Revert "Make Selectgen treat region boundaries more precisely" (oxcaml#1283)
2078dce flambda-backend: Add some -dtimings output for the typechecker (oxcaml#1245)
273a7f9 flambda-backend: Make Selectgen treat region boundaries more precisely (oxcaml#1131)
47610e6 flambda-backend: Bump magic numbers for 4.14.1-6 (oxcaml#1274)
fd53d38 flambda-backend: Generate *.cms files for merlin (oxcaml#1232)
853f95f flambda-backend: Add tail_mod_const to builtin_attrs (oxcaml#1265)
f9ef051 flambda-backend: Fix issue caused by effects of gadt expansion in mode cross check (oxcaml#1263)
e9ffcf8 flambda-backend: Fix dependencies for regenerating Flambda2 parser, tests (oxcaml#1255)
6f1cd1f flambda-backend: Restore a lost location, needed for merlin (oxcaml#1242)
009332b flambda-backend: Fix merge from ocaml-jst

git-subtree-dir: ocaml
git-subtree-split: a7d005a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants