Skip to content

Conversation

vouillon
Copy link
Member

No description provided.

@vouillon vouillon force-pushed the ref-unboxing branch 2 times, most recently from ad356b3 to 417dc29 Compare April 30, 2025 14:07
@hhugo
Copy link
Member

hhugo commented May 9, 2025

rebased

@vouillon vouillon force-pushed the ref-unboxing branch 2 times, most recently from 55e92ea to e3a93a3 Compare May 16, 2025 08:40
@vouillon vouillon mentioned this pull request Jul 10, 2025
8 tasks
@vouillon vouillon force-pushed the ref-unboxing branch 4 times, most recently from 42c5f9e to fe4b44c Compare September 1, 2025 10:21
@vouillon vouillon marked this pull request as ready for review September 5, 2025 08:22
@vouillon vouillon requested a review from hhugo September 5, 2025 08:22
Freevars.iter_last_free_var discard block.branch;
match block.branch with
| Pushtrap ((pc', _), _, (pc'', _)) ->
traverse (depth + 1) start_pc pc';
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment explaining why we can't optimize across try-catch ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have updated the comment at the beginning of the file

Comment on lines +50 to +54
let refs, _ = Int.Hashtbl.find relevant_vars pc' in
let vars = Var.Map.filter (fun x _ -> Var.Set.mem x refs) vars in
Copy link
Member

Choose a reason for hiding this comment

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

Is the filtering necessary ? isn't vars always a subset of refs ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not when you jump outside the scope of a reference. refs is the set of references which are in scope a the beginning of block pc'; vars maps references in scope in the current block to the variables which contain their current value.

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