Skip to content

Tweak logging code of register allocators #3822

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 3 commits into from
Apr 8, 2025

Conversation

xclerc
Copy link
Contributor

@xclerc xclerc commented Apr 8, 2025

This pull request is essentially a copy of #3775
(not sure what went wrong), where the discussion
will still happen in order to not lose the state
of the conversation.

@xclerc xclerc force-pushed the regalloc-logging-indent branch from a005e1c to f7ff5bd Compare April 8, 2025 08:21
@xclerc xclerc added the backend label Apr 8, 2025
Copy link
Contributor

@spiessimon spiessimon left a comment

Choose a reason for hiding this comment

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

I think this looks good! Thank you for addressing my comments from the other PR. I did not carefully read over all of the changes here again, given the other PR (and in particular did not do indent/dedent counting).

I talked to @mshinwell, and flambda2 with 03 enabled (but perhaps already as is) should be able to optimize away the with_ constructs that we discussed the other day. If we want to force O3, it can be enabled at the beginning of the file with [@@@ocaml.flambda_o3]. (But feel free to keep the indent/dedent if refactoring it would be too much of a hassle.)

@xclerc
Copy link
Contributor Author

xclerc commented Apr 8, 2025

(I have made a note to revisit the logging
during the next sprint.)

@xclerc xclerc merged commit 0908448 into ocaml-flambda:main Apr 8, 2025
26 checks passed
gretay-js pushed a commit to gretay-js/flambda-backend that referenced this pull request Apr 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants