Skip to content

Remove subkinds from value slots #3981

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

Merged
merged 7 commits into from
May 14, 2025

Conversation

Ekdohibs
Copy link
Contributor

@Ekdohibs Ekdohibs commented May 9, 2025

When the reaper replaces some values by poison, the subkind information in value slots can become incorrect. This fixes the problem by completely removing the subkind information in value slots, which is otherwise only used for putting tagged immediates in the non-scanned area of closures.

@mshinwell mshinwell added the flambda2 Prerequisite for, or part of, flambda2 label May 9, 2025
Copy link
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.

This patch has the following negative consequences:

  • All value slots of kind Value, including immediates, are now stored in the scannable part of the closure
  • All value slots of kind Value, for large closures and static closures, will be initialised using caml_initialize. For immediates, before this PR a direct write could be used.
  • In some very uncommon cases, when simplifying a value slot projection the flambda2 type for the result will be less precise than it was before.

These are expected consequences, and I am in favour of merging this PR now to allow progress on unboxing of free variables, and work on restoring the lost functionality in a later PR (at least the first two points; the third one is likely irrelevant).

@mshinwell
Copy link
Collaborator

I'm not sure actually. How much work is it to restore the old behaviour for immediates here?

@Ekdohibs Ekdohibs merged commit 6e16d56 into oxcaml:main May 14, 2025
27 checks passed
Ekdohibs added a commit to Ekdohibs/flambda-backend that referenced this pull request May 14, 2025
Gbury pushed a commit to Gbury/flambda-backend that referenced this pull request May 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flambda2 Prerequisite for, or part of, flambda2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants