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 code related to some recent changes in LLVM #155

Merged
merged 1 commit into from
Jan 3, 2023

Conversation

boomanaiden154
Copy link
Contributor

As of current TOT LLVM (past LLVM 15), the API for LLVM::Optional has changed significantly, and LLVM::None has been deprecated and many pieces have been switched over to use std::optional. This patch fixes the compilation of autoFDO in relation to these new changes.

As of current TOT LLVM (past LLVM 15), the API for LLVM::Optional has
changed significantly, and LLVM::None has been deprecated and many
pieces have been switched over to use std::optional. This patch fixes
the compilation of autoFDO in relation to these new changes.
@boomanaiden154
Copy link
Contributor Author

Looking at some other pieces of the repository, I'm guessing these changes should be feature gated through the preprocessor to enable compatibility with multiple LLVM versions?

@shenhanc78
Copy link
Collaborator

Thanks Aiden. Yes, we have to support ToT as well as older LLVM source trees. However, adding Preprocessors to gate the changes seem to me a little bit cumbersome at least for changes in this PR. If it is just warnings,let's live with it for a while. What do you think?

@boomanaiden154
Copy link
Contributor Author

It isn't just warnings. The APIs for LLVM::Optional and std::optional are not compatible, so there need to be some code changes made between the two versions. LLVM::None has also been completely removed in ToT LLVM.

@shenhanc78
Copy link
Collaborator

Thanks Aiden. I see. I'll merge your PR#155. (And I don't think guarding the changes is sustainable, so let's just make it buildable to ToT llvm.)

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