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

Fix a bug in slot_offsets when two separate sets of closures have a slot in common, but not all #2649

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

Ekdohibs
Copy link
Contributor

@Ekdohibs Ekdohibs commented Jun 4, 2024

Extracted from #2642.

@mshinwell mshinwell added the flambda2 Prerequisite for, or part of, flambda2 label Jun 4, 2024
@mshinwell
Copy link
Collaborator

Please add a test for this...

@Ekdohibs
Copy link
Contributor Author

Ekdohibs commented Jun 4, 2024

I'm not sure how to add a test, given it's something that isn't generated by the compiler as of now, and this can't be tested by flexpect either.

@Ekdohibs
Copy link
Contributor Author

Ekdohibs commented Jun 6, 2024

So, this fixes a bug, but there are other remaining. Indeed, we cannot put space between function slots in set of closures, as Gc.compact will scan that area (especially the arity). This will require more changes, the scope of which I am not sure about yet.

@Gbury
Copy link
Contributor

Gbury commented Jun 10, 2024

I'll look at the problem with consecutive slots as soon as I have the time.

@Ekdohibs
Copy link
Contributor Author

The consecutive slots problem has been addressed in #2674.

@Ekdohibs Ekdohibs merged commit 7d944ef into ocaml-flambda:main Jun 19, 2024
17 checks passed
TheNumbat pushed a commit that referenced this pull request Jun 21, 2024
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