Skip to content

Add layout type in Lambda#1032

Merged
mshinwell merged 3 commits intooxcaml:mainfrom
Ekdohibs:lambda-layout
Feb 7, 2023
Merged

Add layout type in Lambda#1032
mshinwell merged 3 commits intooxcaml:mainfrom
Ekdohibs:lambda-layout

Conversation

@Ekdohibs
Copy link
Copy Markdown
Contributor

@Ekdohibs Ekdohibs commented Dec 20, 2022 •
edited
Loading

For now, we simply have the type type layout = Pvalue of value_kind, to prepare for the addition of layouts everywhere. Besides, this also adds several pre-defined layouts (often equal to Pvalue Pgenval), so that we know why Pgenval was used, for instance whether it was a true top value or whether it was the kind of a function.

There are quite a number of layout_top instances, which will need to be removed with further PRs, as there is no top layout once we start getting non-value layouts. Hopefully, this PR is the one with the most global effect, and other PRs can start cleaning the code with a smaller scope at a time.

I'd especially like reviews concerning the translmod, translobj and translclass files, where there were a lot of instances of Pgenval, and I was unsure how to categorize a lot of them (so they ended up as layout_top).

@mshinwell
Copy link
Copy Markdown
Collaborator

mshinwell commented Jan 10, 2023

@lthls is going to review this.

lthls
lthls requested changes Jan 20, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@lthls lthls left a comment

Choose a reason for hiding this comment

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

I'm made some comments. I got a bit tired of commenting every single layout_top, and most of them should be fixed in #1084 anyway.

@Ekdohibs
Copy link
Copy Markdown
Contributor Author

Ekdohibs commented Jan 25, 2023

Rebased on current main.

@ccasin
Copy link
Copy Markdown
Collaborator

ccasin commented Jan 28, 2023 •
edited
Loading

I took a quick look at the changes under ocaml, and they look reasonable to me in terms of compatibility with the approach we're taking in the type systems side of this. Some high-level thoughts:

  • This PR adds a layout datatype in Lambda.mli. I think the idea here is that this will mirror the "sorts" in our layout design, but adding more information in the value case. (Sorts are, roughly, the layouts that capture just enough information to know how to compile a type). This seems reasonable.

    In particular, Lambda's view of "value" and its sublayouts, captured by the existing value_kind type, is already more fine-grained that the layouts in the typechecker. An alternative design would be to directly use the type system's sorts and pair them with additional information in the value case, but the design here seems cleaner.

  • A minor difference from what is done in the current type system prototype is that this PR changes Typeopt.value_kind into Typeopt.layout. I think I slightly prefer letting value_kind be just value_kind and adding a second function which dispatches to it in the value case, mostly because value_kind is already quite complicated. Though admittedly that approach does require some care to avoid recursively calling value_kind on non-value types, so I don't feel strongly.

  • This PR changes the value_kinds in the lambda datatype to layouts. This seems right (though in most of these places we'll allow only values in the short term). Obviously there will be some work to do to propagate correct layouts to the right places when we merge this with the front end bits and add more layouts. In particular, we have taken some care to store layout information in the typed tree to avoid the need for the translation to lambda to need to recompute layouts of types. But I don't see any fundamental obstacles here - I already did a lot of the needed work to deal with the (rather silly) way we are compiling void in the current type system prototype.

@ccasin
Copy link
Copy Markdown
Collaborator

ccasin commented Jan 28, 2023 •
edited
Loading

I realized after writing this that these two points are related:

I think I slightly prefer letting value_kind be just value_kind and adding a second function which dispatches to it in the value case, mostly because value_kind is already quite complicated.
...
In particular, we have taken some care to store layout information in the typed tree to avoid the need for the translation to lambda to need to recompute layouts of types

Because our goal is to avoid ever needing to calculate the layout for a type_expr during the translation to lambda by storing enough information in the typed tree, we should have two functions with different types:

val layout : Env.t -> Types.sort -> Types.type_expr -> Lambda.layout

val value_kind  : Env.t -> Types.type_expr -> Lambda.value_kind

The idea being that the former doesn't actually look at the type_expr, it just passes it to value_kind in the case where the sort is value.

Does that make sense?

@Ekdohibs
Copy link
Copy Markdown
Contributor Author

Ekdohibs commented Jan 30, 2023

Ah yes, I see, this would probably be better indeed as it would avoid having to look at the type_expr. This is almost what is currently being done, since the layout function currently just defers to the value_kind function, adding a Pvalue wrapper around it (but value_kind is no longer exposed).

There might be a few cases where we need some more changes to the typed tree to avoid having to recompute the layouts in all cases: these correspond to all uses of function_return_layout in the PR, where the layout will have to be stored in the typed tree explicitly.

@mshinwell mshinwell mentioned this pull request Feb 7, 2023
Closed
@mshinwell mshinwell merged commit 23c793d into oxcaml:main Feb 7, 2023
@mshinwell mshinwell linked an issue Feb 7, 2023 that may be closed by this pull request
Lambda changes for unboxed function parameters and returns #976
Closed
@ccasin ccasin mentioned this pull request Feb 21, 2023
Merged
ccasin added a commit to ccasin/oxcaml that referenced this pull request Mar 12, 2023
@ccasin
Squashed 'ocaml/' changes from c703f5f..77434f0
d93152b 
77434f0 flambda-backend: Bump magic numbers for 4.14.1-5 (oxcaml#1190)
3a78e83 flambda-backend: Revert "Instance compilation units" (oxcaml#1175)
9a683c9 flambda-backend: Move compute_layout to Lambda (oxcaml#1167)
0ea58e9 flambda-backend: Revert "Add a proper top and bottom layout" (oxcaml#1169)
1e5e23a flambda-backend: Add hint for common misplaced [@unboxed] attribute (oxcaml#1164)
10f870a flambda-backend: Add a proper top and bottom layout (oxcaml#1158)
d1be563 flambda-backend: Don't warn about misplaced attributes in -i mode (oxcaml#1163)
feefcaa flambda-backend: Remove immutable arrays from stdlib.ml and stdlib.mli (oxcaml#1154)
ba101be flambda-backend: Shrink the ref_table if it grows large (oxcaml#1156)
777fda7 flambda-backend: Preserve backtraces from failing `Lazy_backtrack` computations (oxcaml#805)
56d014e flambda-backend: Remove remaining `layout_top` after oxcaml#1084 (oxcaml#1138)
dc1c1ce flambda-backend: Layouts for parameters in lambda & remove most layout_top (oxcaml#1084)
49fea78 flambda-backend: Instance compilation units (oxcaml#1113)
1127fd2 flambda-backend: Merge pull request oxcaml#1143 from riaqn/merge-ocaml-jst
f458733 flambda-backend: fix things after merge
b43d385 flambda-backend: File magic updates (oxcaml#306)
9f604aa flambda-backend: Merge ocaml-jst
1f54613 flambda-backend: More assertions for local alloc and letrec (oxcaml#1081)
0c95280 flambda-backend: Remove most layout_top in closure (oxcaml#1127)
8ad48b5 flambda-backend: Use only `.ocamlformat*` to enable or disable ocamlformat (oxcaml#1135)
1aa2885 flambda-backend: Build all boot targets in `make hacking` (oxcaml#1133)
6b6c25a flambda-backend: Hide `Compilation_unit.t`'s definition in a submodule (oxcaml#1134)
529d66b flambda-backend: Compile refactor (oxcaml#1096)
df798c1 flambda-backend: Propagates layouts through Flambda1 (oxcaml#1115)
9bce50b flambda-backend: Add `gc-timings` to collect timing information from the GC (oxcaml#1089)
d431d3b flambda-backend: Use the type of primitive declarations to make their layout (oxcaml#1118)
8da887e flambda-backend: Changes to arity in clambda (oxcaml#1106)
db20e97 flambda-backend: Fix simplify-exits (oxcaml#1108)
6a63906 flambda-backend: Add layout on Lregion (oxcaml#1107)
c562fb3 flambda-backend: caml_{curry,apply,send}* for unboxed types (oxcaml#1104)
50ee311 flambda-backend: Temporary fix for incorrect layout for result of custom and operators (oxcaml#1119)
c89512d flambda-backend: More error checking for natdynlink symbols (oxcaml#1005)
27d68bf flambda-backend: Flambda1 region deletion and locals fixes (oxcaml#1000)
0e3e057 flambda-backend: Add identifiers for instantiated functor units (oxcaml#1092)
4b75b46 flambda-backend: Fix build system under ocaml/runtime (oxcaml#1085)
7c9fc32 flambda-backend: Fix valid character range computation for compilation units (oxcaml#1110)
40c754a flambda-backend: Add result layout in Lapply and Lsend (oxcaml#1102)
cedaea1 flambda-backend: Add layout type in Lambda (oxcaml#1032)
47c0e23 flambda-backend: Configure: Add flag to use legacy library layout (oxcaml#1098)
eed5888 flambda-backend: Restructure LIBDIR: Move Dynlink, Str and Unix to sub-directories (port part of ocaml/PR11198) (oxcaml#1094)
5b98caf flambda-backend: Revert "Restructure LIBDIR" (oxcaml#1093)
46f1b16 flambda-backend: Remove a stray character from dynlink Makefile (oxcaml#1090)
e515442 flambda-backend: Restructure LIBDIR: Move Dynlink, Str and Unix to sub-directories (port PR#11198) (oxcaml#895)
9307e7c flambda-backend: inline fast path of caml_applyN (oxcaml#934)
3244386 flambda-backend: Bump magic numbers (oxcaml#1077)
7c9e15d flambda-backend: Fix bug in arguments to `get_unit_info` (oxcaml#1069)
a2f4c9e flambda-backend: Closure rename static catch (oxcaml#1070)
2ee705a flambda-backend: Backport oxcaml#946 to ocaml/ subfolder (oxcaml#1061)
569703f flambda-backend: Merge ocaml-jst#97 (oxcaml#1063)
7b1779a flambda-backend: Port value_kind changes to testsuite/tools to fix parsecmm.mly (oxcaml#1060)
49505d1 flambda-backend: Bugfix for Ctype.nondep_type (oxcaml#1059)
654c63c flambda-backend: Backport oxcaml#295 kind changes to ocaml/ subfolder (oxcaml#1018)
50a9ce0 flambda-backend: Fix extern_closure_up_to_env (oxcaml#1053)
bc2c78d flambda-backend: Remove obsolete pack-related code (oxcaml#1051)
872ff38 flambda-backend: Fix error-prone syntax resulting from reformatting (oxcaml#1050)
d03b346 flambda-backend: Fix different .cmi files being produced by ocamlopt and ocamlc when using -pack (oxcaml#1049)
90ee37b flambda-backend: Revert "Revert "Use Import_info.t in Cmt_format"" (oxcaml#1045)
ac12d90 flambda-backend: Fix version number (oxcaml#1043)
75e0154 flambda-backend: Revert "Use Import_info.t in Cmt_format" (oxcaml#1042)
ecab74c flambda-backend: Use Import_info.t in Cmt_format (oxcaml#1037)
7661d4d flambda-backend: Bootstrap
1098a56 flambda-backend: Update .depend files
a7292da flambda-backend: Revert ocaml/toplevel/native/dune
7584479 flambda-backend: Remove alloc-check from ocaml/

git-subtree-dir: ocaml
git-subtree-split: 77434f0
ccasin added a commit that referenced this pull request Mar 24, 2023
@ccasin
Squashed 'ocaml/' changes from c703f5f..bcae5ff
2376a0b 
bcae5ff flambda-backend: Bump buffer size to avoid gcc warning (#1247)
d6c170b flambda-backend: Remove `-absname` to improve dune build errors (#1233)
1b2bcb4 flambda-backend: Put frametables in .text with relative offsets for return addresses. (#1227)
999d523 flambda-backend: Small optimisation for caml_modify (#1226)
b16493b flambda-backend: Add timestamps to GC logs (#1229)
95f7e80 flambda-backend: Provide a no-naked-pointers runtime and use it for the compiler (#1224)
ba77581 flambda-backend: Replace assertion with a match statement (#1225)
cfb3cd2 flambda-backend: Memoise last substitution composition (#1209)
1c4db44 flambda-backend: Port PR1202 and PR1205 to the ocaml/ subtree (#1211)
577410e flambda-backend: Correctly stack debuginfo for inlined body in classic mode (#1152)
6733de6 flambda-backend: Fix dune install in otherlibs: missing cmt and typo. (#1194)
4c97d26 flambda-backend: Unboxed numbers (#1165)
1ad7252 flambda-backend: Revert "Revert "Add a proper top and bottom layout" (#1169)" (#1191)
dea4b3e flambda-backend: Don't get the layout of arguments from patterns (#1179)
6d3e85b flambda-backend: Don't copy when resolving aliases in try_modtypes (#1184)
73e52b7 flambda-backend: Clarify the types used for static jump/catch (#1180)
273a40d flambda-backend: Reenable backtrace testsuite folder for flambda2 (#1161)
77434f0 flambda-backend: Bump magic numbers for 4.14.1-5 (#1190)
3a78e83 flambda-backend: Revert "Instance compilation units" (#1175)
9a683c9 flambda-backend: Move compute_layout to Lambda (#1167)
0ea58e9 flambda-backend: Revert "Add a proper top and bottom layout" (#1169)
1e5e23a flambda-backend: Add hint for common misplaced [@unboxed] attribute (#1164)
10f870a flambda-backend: Add a proper top and bottom layout (#1158)
d1be563 flambda-backend: Don't warn about misplaced attributes in -i mode (#1163)
feefcaa flambda-backend: Remove immutable arrays from stdlib.ml and stdlib.mli (#1154)
ba101be flambda-backend: Shrink the ref_table if it grows large (#1156)
777fda7 flambda-backend: Preserve backtraces from failing `Lazy_backtrack` computations (#805)
56d014e flambda-backend: Remove remaining `layout_top` after #1084 (#1138)
dc1c1ce flambda-backend: Layouts for parameters in lambda & remove most layout_top (#1084)
49fea78 flambda-backend: Instance compilation units (#1113)
1127fd2 flambda-backend: Merge pull request #1143 from riaqn/merge-ocaml-jst
f458733 flambda-backend: fix things after merge
b43d385 flambda-backend: File magic updates (#306)
9f604aa flambda-backend: Merge ocaml-jst
1f54613 flambda-backend: More assertions for local alloc and letrec (#1081)
0c95280 flambda-backend: Remove most layout_top in closure (#1127)
8ad48b5 flambda-backend: Use only `.ocamlformat*` to enable or disable ocamlformat (#1135)
1aa2885 flambda-backend: Build all boot targets in `make hacking` (#1133)
6b6c25a flambda-backend: Hide `Compilation_unit.t`'s definition in a submodule (#1134)
529d66b flambda-backend: Compile refactor (#1096)
df798c1 flambda-backend: Propagates layouts through Flambda1 (#1115)
9bce50b flambda-backend: Add `gc-timings` to collect timing information from the GC (#1089)
d431d3b flambda-backend: Use the type of primitive declarations to make their layout (#1118)
8da887e flambda-backend: Changes to arity in clambda (#1106)
db20e97 flambda-backend: Fix simplify-exits (#1108)
6a63906 flambda-backend: Add layout on Lregion (#1107)
c562fb3 flambda-backend: caml_{curry,apply,send}* for unboxed types (#1104)
50ee311 flambda-backend: Temporary fix for incorrect layout for result of custom and operators (#1119)
c89512d flambda-backend: More error checking for natdynlink symbols (#1005)
27d68bf flambda-backend: Flambda1 region deletion and locals fixes (#1000)
0e3e057 flambda-backend: Add identifiers for instantiated functor units (#1092)
4b75b46 flambda-backend: Fix build system under ocaml/runtime (#1085)
7c9fc32 flambda-backend: Fix valid character range computation for compilation units (#1110)
40c754a flambda-backend: Add result layout in Lapply and Lsend (#1102)
cedaea1 flambda-backend: Add layout type in Lambda (#1032)
47c0e23 flambda-backend: Configure: Add flag to use legacy library layout (#1098)
eed5888 flambda-backend: Restructure LIBDIR: Move Dynlink, Str and Unix to sub-directories (port part of ocaml/PR11198) (#1094)
5b98caf flambda-backend: Revert "Restructure LIBDIR" (#1093)
46f1b16 flambda-backend: Remove a stray character from dynlink Makefile (#1090)
e515442 flambda-backend: Restructure LIBDIR: Move Dynlink, Str and Unix to sub-directories (port PR#11198) (#895)
9307e7c flambda-backend: inline fast path of caml_applyN (#934)
3244386 flambda-backend: Bump magic numbers (#1077)
7c9e15d flambda-backend: Fix bug in arguments to `get_unit_info` (#1069)
a2f4c9e flambda-backend: Closure rename static catch (#1070)
2ee705a flambda-backend: Backport #946 to ocaml/ subfolder (#1061)
569703f flambda-backend: Merge ocaml-jst#97 (#1063)
7b1779a flambda-backend: Port value_kind changes to testsuite/tools to fix parsecmm.mly (#1060)
49505d1 flambda-backend: Bugfix for Ctype.nondep_type (#1059)
654c63c flambda-backend: Backport #295 kind changes to ocaml/ subfolder (#1018)
50a9ce0 flambda-backend: Fix extern_closure_up_to_env (#1053)
bc2c78d flambda-backend: Remove obsolete pack-related code (#1051)
872ff38 flambda-backend: Fix error-prone syntax resulting from reformatting (#1050)
d03b346 flambda-backend: Fix different .cmi files being produced by ocamlopt and ocamlc when using -pack (#1049)
90ee37b flambda-backend: Revert "Revert "Use Import_info.t in Cmt_format"" (#1045)
ac12d90 flambda-backend: Fix version number (#1043)
75e0154 flambda-backend: Revert "Use Import_info.t in Cmt_format" (#1042)
ecab74c flambda-backend: Use Import_info.t in Cmt_format (#1037)
7661d4d flambda-backend: Bootstrap
1098a56 flambda-backend: Update .depend files
a7292da flambda-backend: Revert ocaml/toplevel/native/dune
7584479 flambda-backend: Remove alloc-check from ocaml/

git-subtree-dir: ocaml
git-subtree-split: bcae5ff
@ccasin ccasin mentioned this pull request Jun 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mshinwell mshinwell mshinwell approved these changes

@lthls lthls lthls approved these changes

@lpw25 lpw25 Awaiting requested review from lpw25

@antalsz antalsz Awaiting requested review from antalsz

@gretay-js gretay-js Awaiting requested review from gretay-js

@xclerc xclerc Awaiting requested review from xclerc

Assignees

No one assigned

Labels

middle end

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Lambda changes for unboxed function parameters and returns

4 participants

@Ekdohibs @mshinwell @ccasin @lthls