Skip to content

Add stub marker to lambda code#957

Merged
lpw25 merged 1 commit intoocaml:trunkfrom
lpw25:lambda-stubs
Jan 10, 2017
Merged

Add stub marker to lambda code#957
lpw25 merged 1 commit intoocaml:trunkfrom
lpw25:lambda-stubs

Conversation

@lpw25
Copy link
Contributor

@lpw25 lpw25 commented Dec 8, 2016

When Flambda has to create small functions for things like partial applications, it marks these functions as "stub" functions. This changes the heuristics for inlining them, and can produce much better code.

However, lambda also creates such functions and they are not currently marked as "stub" since lambda code has no such notion. This PR fixes that deficiency. It adds a stub field to the function_attribute type in lambda.ml and uses it to mark the small wrapper functions produced by lambda.

closure.ml is also updated to always inline such functions much as flambda does -- although since they're all tiny this change is basically a no-op.

The addition of "stub" to lambda also allows to remove a horrible hack around the treatment of "default argument wrappers" in closure_conversion.ml.

The following program demonstrates the benefit of this PR. It produces much better code with this change, as without it the partial applications of labelled functions get in the way of the inlining heuristics:

type 'a t =
    F :
      { init : unit -> 's
      ; cond : 's -> bool
      ; step : 's -> ('a -> unit) -> unit
      ; freeze : 's -> 'a t } -> 'a t

let[@inline] fold  ~init ~f (F t)=
  let s = t.init () in
  let acc = ref init in
  while t.cond s do
    t.step s (fun a -> acc := f !acc a)
  done;
  !acc

let[@inline] rec of_array a ~i =
  let[@inline] init () = ref i in
  let[@inline] cond s = !s < Array.length a in
  let[@inline] step s f = f a.(!s); incr s in
  let[@inline] freeze s = of_array a ~i:!s in
  F { init; cond; step; freeze }

let of_array = of_array ~i:0

let[@inline] rec map ~f t =
  match t with
  | F t ->
    let[@inline] step s g = t.step s ((fun a -> g (f a)) [@inline]) in
    F { step;
        init = t.init;
        cond = t.cond;
        freeze = fun s -> (map[@inlined never]) ~f (t.freeze s)
      }

let[@inline] filter t ~f =
  match t with
  | F t ->
    let[@inline] step s g = t.step s ((fun a -> if f a then g a) [@inline]) in
    F { t with step }

let main a =
  of_array a
  |> map ~f:((+) 1)
  |> filter ~f:(fun x -> x mod 2 = 0)
  |> fold ~init:0 ~f:(+)

[(fun_id, Lfunction{kind; params; body = create_wrapper_body wrapper_body;
attr; loc}); inner]
let body, inner = aux [] body in
let attr = default_stub_attribute in
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible that in the future there may be attributes that we don't want to discard when marking something as a stub? I wonder if it would be better if there was a function set_stub_attribute or something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not really discarding them, it's just not duplicating them onto the wrapper. Attributes which should appear on both the wrapper and the function seem less likely to me (there are none at the moment), so I think starting from default_stub_attribute and explicitly copying any that it should share would be more natural (which is what the current code does except that at the moment there are none to explicitly share).

Copy link
Contributor

Choose a reason for hiding this comment

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

OK

@chambart
Copy link
Contributor

I reviewed it too, seems ok and get rid of an ugly hack. I approve too.

@lpw25 lpw25 merged commit 7b1ce6d into ocaml:trunk Jan 10, 2017
chambart added a commit to chambart/ocaml-1 that referenced this pull request Feb 6, 2017
camlspotter pushed a commit to camlspotter/ocaml that referenced this pull request Oct 17, 2017
Add stub marker to lambda code
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
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.

3 participants