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 compile-time crash related to Pony-specific optimizations. #3831

Merged

Conversation

jemc
Copy link
Member

@jemc jemc commented Aug 20, 2021

Ticket #3784 tracked an issue related the MergeMessageSend optimization pass, which does Pony-specific optimizations within the LLVM optimizer pipeline.

It turned out that this pass was not written to handle the case of being run on a code block that contained sections of code that had already been optimized by this pass, and in such a case it failed an assertion error, crashing the compiler on such a program.

The fix was to disable running the pass on blocks in which we could detect signs of the optimization of that pass already being present.

Programs that were previously compiling okay should not see any difference in behavior or performance - the only programs that could potentially benefit from the skipped optimization were crashing, so there is no regression in their behavior.

Resolves #3784.

Ticket #3784 tracked an issue related the MergeMessageSend optimization pass, which does Pony-specific optimizations within the LLVM optimizer pipeline.

It turned out that this pass was not written to handle the case of being run on a code block that contained sections of code that had already been optimized by this pass, and in such a case it failed an assertion error, crashing the compiler on such a program.

The fix was to disable running the pass on blocks in which we could detect signs of the optimization of that pass already being present.

Programs that were previously compiling okay should not see any difference in behavior or performance - the only programs that could potentially benefit from the skipped optimization were crashing, so there is no regression in their behavior.
@jemc jemc requested a review from a team August 20, 2021 22:40
@jemc jemc self-assigned this Aug 20, 2021
@jemc jemc added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Aug 20, 2021
@jemc
Copy link
Member Author

jemc commented Aug 20, 2021

Note that it was discussed in this comment #3784 (comment) that I was unable to add a compiler test that reproduces the bug, and Sean agreed that in this case we could merge a PR without an automated regression test attached.

Running the sample code in the bug ticket would suffice for manual testing of anyone who wanted to prove this was still working at a later date.

@ergl
Copy link
Member

ergl commented Aug 22, 2021

Verified locally that all examples in #3784 compile correctly with this PR

@SeanTAllen SeanTAllen merged commit dd09bf9 into main Aug 30, 2021
@SeanTAllen SeanTAllen deleted the fix/3784/compile-time-crash-due-to-merge-send-optimization branch August 30, 2021 21:40
github-actions bot pushed a commit that referenced this pull request Aug 30, 2021
github-actions bot pushed a commit that referenced this pull request Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compile-time crash related to Pony-specific optimization passes
3 participants