Skip to content

Conversation

@lthls
Copy link
Contributor

@lthls lthls commented Apr 29, 2025

This PR restores support for generating phantom bindings.

I wrote it by enabling phantom lets, and fixing errors as I find them. Given the number of bugs I had to fix, it is likely that some remain in some less-tested corner cases.

Here is a summary of the changes:

  • The Name_mode lattice is now a straight line. The typing env requires all the variables in it to have a name mode at least In_types, so I made Phantom greater than In_types instead of not comparable. It would also be possible to prevent the typing env from handling variables in Phantom mode, and that should make the environments smaller, but I think it should be a separate PR.
  • The logic in Expr_builder to decide whether to keep a binding, delete it or phantomise it has been moved to Simplify_let_expr.rebuild_let. This makes decisions more consistent (now even bindings removed by mutable unboxing get phantomised if needed)
  • Slot offset computations now ignore slots not in normal mode, not only in projections. Phantom closures also get Deleted code IDs to avoid keeping old code around.
  • No CSE equations are created for phantom bindings

It is possible that this PR still contains changes that were made redundant by a more general fix. Do not hesitate to point these out during review.

@mshinwell mshinwell added flambda2 Prerequisite for, or part of, flambda2 dwarf and debugging labels Apr 29, 2025
@lthls lthls force-pushed the flambda2-phantom-let branch 2 times, most recently from 1188c79 to 03acdfe Compare June 3, 2025 14:53
@mshinwell mshinwell force-pushed the flambda2-phantom-let branch from 03acdfe to 2e47768 Compare August 26, 2025 14:39
Copy link
Contributor

@chambart chambart left a comment

Choose a reason for hiding this comment

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

Ok this looks sound.
It's hard to tell whether anything is missing out of the changes that would lose any bindings. But the changes themselves seems ok.
There are a few places where It's quite hard to know if the name modes are actually the right one, but I don't know how to verify that without too much effort. I suppose that trying it in the wild is quite safe, as if anything is wrong on that part, this is going to fail quite loudly with all the information we need.

@lthls lthls force-pushed the flambda2-phantom-let branch from 01c7443 to de64a85 Compare September 29, 2025 09:33
@lthls lthls force-pushed the flambda2-phantom-let branch from de64a85 to 88b9b9b Compare October 20, 2025 15:32
@lthls lthls merged commit aaf1d13 into oxcaml:main Oct 21, 2025
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dwarf and debugging flambda2 Prerequisite for, or part of, flambda2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants