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

CGU cleanups #111899

Merged
merged 7 commits into from
May 26, 2023
Merged

CGU cleanups #111899

merged 7 commits into from
May 26, 2023

Commits on May 24, 2023

  1. Inline and remove numbered_codegen_unit_name.

    It is small and has a single call site, and this change will facilitate
    the next commit.
    nnethercote committed May 24, 2023
    Configuration menu
    Copy the full SHA
    e26c0c9 View commit details
    Browse the repository at this point in the history
  2. Remove the merging module.

    Three of the four methods in `DefaultPartitioning` are defined in
    `default.rs`. But `merge_codegen_units` is defined in a separate module,
    `merging`, even though it's less than 100 lines of code and roughly the
    same size as the other three methods. (Also, the `merging` module
    currently sits alongside `default`, when it should be a submodule of
    `default`, adding to the confusion.)
    
    In rust-lang#74275 this explanation was given:
    
    > I pulled this out into a separate module since it seemed like we might
    > want a few different merge algorithms to choose from.
    
    But in the three years since there have been no additional merging
    algorithms, and there is no mechanism for choosing between different
    merging algorithms. (There is a mechanism,
    `-Zcgu-partitioning-strategy`, for choosing between different
    partitioning strategies, but the merging algorithm is just one piece of
    a partitioning strategy.)
    
    This commit merges `merging` into `default`, making the code easier to
    navigate and read.
    nnethercote committed May 24, 2023
    Configuration menu
    Copy the full SHA
    b39b709 View commit details
    Browse the repository at this point in the history
  3. Remove {Pre,Post}InliningPartitioning.

    I find that these structs obfuscate the code. Removing them and just
    passing the individual fields around makes the `Partition` method
    signatures a little longer, but makes the data flow much clearer. E.g.
    
    - `codegen_units` is mutable all the way through.
    - `codegen_units`'s length is changed by `merge_codegen_units`, but only
      the individual elements are changed by `place_inlined_mono_items` and
      `internalize_symbols`.
    - `roots`, `internalization_candidates`, and `mono_item_placements` are
      all immutable after creation, and all used by just one of the four
      methods.
    nnethercote committed May 24, 2023
    Configuration menu
    Copy the full SHA
    20de2ba View commit details
    Browse the repository at this point in the history
  4. Add a clarifying comment.

    This is something that took me some time to figure out.
    nnethercote committed May 24, 2023
    Configuration menu
    Copy the full SHA
    59c5259 View commit details
    Browse the repository at this point in the history

Commits on May 25, 2023

  1. Configuration menu
    Copy the full SHA
    86754cd View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    ed216e2 View commit details
    Browse the repository at this point in the history
  3. Add struct for the return type of place_root_mono_items.

    As per review request.
    nnethercote committed May 25, 2023
    Configuration menu
    Copy the full SHA
    e6b99a6 View commit details
    Browse the repository at this point in the history