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

Remove infinite loop workaround #4068

Merged
merged 1 commit into from
Mar 24, 2022
Merged

Conversation

SeanTAllen
Copy link
Member

Several years ago, Benoit added code to work around LLVM's lack
of support for properly functioning infinite loops.

LLVM added support for infinite loops.

With this commit, we are removing Benoit's temporary fix.

@SeanTAllen SeanTAllen requested a review from a team March 23, 2022 11:51
@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Mar 23, 2022
@SeanTAllen SeanTAllen force-pushed the llvm-12-supports-infinite-loops branch from cab3e1d to 598def3 Compare March 23, 2022 12:07
@ergl
Copy link
Member

ergl commented Mar 23, 2022

The original PR (#2592) also enabled TrapUnreachable (which emits a target-specific trap instruction for unreachable IR instructions). Do we want to revert that, too?

@SeanTAllen
Copy link
Member Author

If I understand correctly @ergl, that you be to remove this line as the rest is from existing code that was moved around:

options.TrapUnreachable = true;

correct?

@ergl
Copy link
Member

ergl commented Mar 23, 2022

@SeanTAllen Yes, I would remove that line, the rest was just moving things around, as you mention.

@SeanTAllen
Copy link
Member Author

@ergl if @jemc agrees then I'll remove it as we would all be in agreement (as we were with what is currently here that we went over at sync).

@jemc
Copy link
Member

jemc commented Mar 23, 2022

I agree about no longer needing the "trap unreachable" setting anymore.

Several years ago, Benoit added code to work around LLVM's lack
of support for properly functioning infinite loops.

LLVM added support for infinite loops.

With this commit, we are removing Benoit's temporary fix.
@SeanTAllen SeanTAllen force-pushed the llvm-12-supports-infinite-loops branch from 598def3 to 6ec4b5a Compare March 23, 2022 20:02
@SeanTAllen
Copy link
Member Author

@jemc @ergl updated.

@SeanTAllen SeanTAllen merged commit 5fa5ad7 into main Mar 24, 2022
@SeanTAllen SeanTAllen deleted the llvm-12-supports-infinite-loops branch March 24, 2022 00:33
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Mar 24, 2022
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.

4 participants