Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable function sorting #2287

Merged
merged 6 commits into from
Feb 14, 2024
Merged

Conversation

gretay-js
Copy link
Contributor

Temporarily disable sorting of function symbols by debug info. Emit function symbols in topological order.

Currently, the layout of function symbols is determined by their debug info, so that functions in the final assembly file appear in the same order as in the source file. This is convenient for debugging and stable layout. However, it can makes some backend passes overly conservative. Currently, this affects only checkmach and polling that rely on "future" function names. For example, zero_alloc check fails on a common code pattern raise_s [%message ...].

let[@zero_alloc] raise_invalid_index ~index ~name =
  if index < 0 then
  raise_s
    [%message
      "Negative" (index : int) (name : string)]

results in the following code after ppx :

let raise_invalid_index ~index ~name =
  if index < 0 then
  raise_s
    (let ppx_sexp_message () =
       Sexp.List
         [Sexp.Atom "Negative";
          Sexp.List
            [Sexp.Atom "index"; Sexp.of_int index];
          Sexp.List
            [Sexp.Atom "name"; Sexp.Atom name]]
     [@@ocaml.inline never][@@ocaml.local never][@@ocaml.specialise never]
     in
     ((ppx_sexp_message ())[@nontail ]))
[@@ocaml.inline never][@@ocaml.local never][@@ocaml.specialise never]

and raise_invalid_index is emitted before ppx_sexp_message even though they are not mutually recursive.

A proper fix is more involved. The backend translates functions in layout order, fully translating each function from Cmm all the way to assembly before moving on to the next function. We can change the backend to translate the functions in topological order and then emit them in source order. This means holding on to Mach.fundecl longer and careful handling any mutable state. Separately, an improvenent to zero_alloc analysis to handle mutually recursive functions will also fix this.

@gretay-js
Copy link
Contributor Author

Sorting was introduced here
ocaml-flambda/ocaml#74

@mshinwell mshinwell merged commit 6c2bb02 into ocaml-flambda:main Feb 14, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants