Skip to content

Commit

Permalink
Squash 'flatten-regions'
Browse files Browse the repository at this point in the history
This squashes 'flatten-regions' into a single commit over
'remove-regions-with-exclaves' and 'cmm-nested-exclaves' (PRs #1524 and #1529,
respectively).

Squashed commit of the following:

commit bd23d8bd75478abafb67f0e791ef77bbd8764b22
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Fri Jun 30 16:03:06 2023 +0100

    Clean up code

commit 0ac81ecfc5b84eb286cbd5f0afc66d7f73b3eb92
Merge: c3a9203f9 9449e65
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Fri Jun 30 14:59:27 2023 +0100

    Merge branch 'cmm-nested-exclaves' into flatten-regions

commit c3a9203f9de900d8acbe77b781debbfb686a3d52
Merge: c1b79d7 d617694
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Fri Jun 30 14:57:23 2023 +0100

    Merge branch 'remove-regions-with-exclaves' into flatten-regions

commit c1b79d7
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Fri Jun 30 14:49:07 2023 +0100

    Add example

commit 033294f
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Thu Jun 8 11:21:22 2023 +0100

    Code review and provisional fixes

    These are fixes to issues that are really second- or third-order effects of
    region lifting and probably need their own PRs, but I'm committing them here for
    testing purposes.

commit 65ad3aa
Merge: e88df82 d65f752
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Thu May 25 17:00:33 2023 +0100

    Merge remote-tracking branch 'upstream/main' into flatten-regions

commit e88df82
Merge: de8cf43 9d3c1c1
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Thu May 11 13:21:45 2023 +0100

    Merge remote-tracking branch 'upstream/main' into flatten-regions

commit de8cf43
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Wed May 10 16:47:44 2023 +0100

    Follow renaming of `Tail` to `Exclave` after merge

commit ceae59a
Merge: 8a717aa 63997c9
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Wed May 10 16:37:23 2023 +0100

    Merge remote-tracking branch 'upstream/main' into flatten-regions

commit 8a717aa
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Wed May 10 16:26:33 2023 +0100

    Don't try to be clever with `let x = M in tail x`

    There's nothing actually wrong with `let x = M in tail x`, so don't try to
    reduce it to just `M`. This works in that narrow case, but the code that was
    doing this transformation didn't notice if the body of the tail is more than
    just `x`. Since the transformation doesn't actually gain anything, better to
    be rid of it than to make things more complicated trying to get it right.

    Also updated a few comments and added an invariant check.

commit 6fa1ce6
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Fri Apr 21 15:58:07 2023 +0100

    Make `map_region_tail` look through regions as well

commit d9043c9
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Fri Apr 21 14:14:19 2023 +0100

    Check for close-on-apply on method calls as well as functions

commit 58bf88a
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Wed Apr 19 17:05:31 2023 +0100

    Fix `Cmmgen`

    `Cmmgen` was (hopefully) the last remaining place where things were broken by
    the assumption that `region (region (tail (tail ...)))` never happens.

commit 66132fb
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Tue Apr 18 18:43:08 2023 +0100

    Don't try to handle regions in `Flambda.fold_lets_option`

    It used to be much more necessary, but no more, and it interferes with the
    handling of regions using `enter_region`.

commit 907627d
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Tue Apr 18 18:41:18 2023 +0100

    Refactor and clean up a bit

commit dccb971
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Tue Apr 18 17:55:02 2023 +0100

    Rewrite `let x = M in tail x` as `M`

    This should get rid of some annoying instances of extra variables introduced
    just to lift, and in particular stop making tail calls into non-tail calls.

commit 6126e9e
Merge: 5b62db1 e138734
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Wed Apr 12 14:31:25 2023 +0100

    Merge remote-tracking branch 'upstream/main' into flatten-regions

commit 5b62db1
Author: Luke Maurer <lmaurer@janestreet.com>
Date:   Wed Nov 30 17:37:40 2022 +0000

    Lift regions along with `let`s

    This transforms expressions of the form

    ```
    let x =
      region (
        let y = E1 in
        E2
      )
    in
    E3
    ```

    into

    ```
    region (
      let y = E1 in
      let x = E2 in
      tail (
        E3
      )
    )
    ```

    so that `y` is available when simplifying `E3` (as is the advantage of lifting
    `let`s to begin with). This requires that `E3` not already have a `tail`
    expression (or tail call) inside an `if` branch or any other construct besides
    the body of a `let`, `let mutable`, `region`, or `tail`.
  • Loading branch information
lukemaurer committed Aug 9, 2023
1 parent 1c6432f commit 9b05a30
Show file tree
Hide file tree
Showing 8 changed files with 427 additions and 44 deletions.
15 changes: 10 additions & 5 deletions middle_end/flambda/flambda_iterators.ml
Original file line number Diff line number Diff line change
Expand Up @@ -731,14 +731,15 @@ let map_sets_of_closures_of_program (program : Flambda.program)
program_body = loop program.program_body;
}

let map_exprs_at_toplevel_of_program (program : Flambda.program)
~(f : Flambda.t -> Flambda.t) =
let map_exprs_at_toplevel_of_program_with_under_lambda
(program : Flambda.program)
~(f : Flambda.t -> under_lambda:bool -> Flambda.t) =
let rec loop (program : Flambda.program_body) : Flambda.program_body =
let map_constant_set_of_closures (set_of_closures:Flambda.set_of_closures) =
let done_something = ref false in
let funs =
Variable.Map.map (fun (function_decl : Flambda.function_declaration) ->
let body = f function_decl.body in
let body = f function_decl.body ~under_lambda:true in
if body == function_decl.body then
function_decl
else begin
Expand Down Expand Up @@ -799,7 +800,7 @@ let map_exprs_at_toplevel_of_program (program : Flambda.program)
let done_something = ref false in
let fields =
List.map (fun field ->
let new_field = f field in
let new_field = f field ~under_lambda:false in
if not (new_field == field) then begin
done_something := true
end;
Expand All @@ -812,7 +813,7 @@ let map_exprs_at_toplevel_of_program (program : Flambda.program)
else
Initialize_symbol (symbol, tag, fields, new_program')
| Effect (expr, program') ->
let new_expr = f expr in
let new_expr = f expr ~under_lambda:false in
let new_program' = loop program' in
if new_expr == expr && new_program' == program' then
program
Expand All @@ -824,6 +825,10 @@ let map_exprs_at_toplevel_of_program (program : Flambda.program)
program_body = loop program.program_body;
}

let map_exprs_at_toplevel_of_program program ~f =
map_exprs_at_toplevel_of_program_with_under_lambda
~f:(fun expr ~under_lambda:_ -> f expr) program

let map_named_of_program (program : Flambda.program)
~(f : Variable.t -> Flambda.named -> Flambda.named) : Flambda.program =
map_exprs_at_toplevel_of_program program
Expand Down
5 changes: 5 additions & 0 deletions middle_end/flambda/flambda_iterators.mli
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,11 @@ val map_project_var_to_named_opt
-> f:(Flambda.project_var -> Flambda.named option)
-> Flambda.t

val map_exprs_at_toplevel_of_program_with_under_lambda
: Flambda.program
-> f:(Flambda.t -> under_lambda:bool -> Flambda.t)
-> Flambda.program

val map_exprs_at_toplevel_of_program
: Flambda.program
-> f:(Flambda.t -> Flambda.t)
Expand Down
4 changes: 3 additions & 1 deletion middle_end/flambda/inline_and_simplify.ml
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,9 @@ and simplify_over_application env r ~args ~args_approxs ~function_decls
inlined = inlined_requested; specialise = specialise_requested;
probe = None})
in
let expr = Lift_code.lift_lets_expr expr ~toplevel:true in
let expr =
Lift_code.lift_lets_expr expr ~toplevel:true ~in_closure:Maybe
in
let expr =
match mode, function_decl.A.region with
| Lambda.Alloc_heap, false -> Flambda.Region expr
Expand Down
Loading

0 comments on commit 9b05a30

Please sign in to comment.