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

Allowing multiple LLVM versions to be used. #119

Merged
merged 17 commits into from
Jun 1, 2022

Conversation

idavis
Copy link
Collaborator

@idavis idavis commented May 16, 2022

This PR set up the ability to compile against multiple versions of LLVM. Setting PYQIR_LLVM_FEATURE_VERSION to one of the following values llvm11-0, llvm12-0, and llvm13-0 will change the LLVM version for all components. LLVM-IR doesn't support LLVM 14, so 14 was not added to the build. The default value if the environment variable isn't set is llvm11-0.

I have not added and build configurations for CI. The default LLVM version for qirlib is not specified, but the build will fail if the features isn't set.

This PR is based off of the work @nilslice did for #114.

@idavis idavis force-pushed the iadavis/multiple-llvm-version branch from fc552ba to 3702251 Compare May 17, 2022 14:20
@idavis idavis self-assigned this May 17, 2022
@idavis idavis force-pushed the iadavis/multiple-llvm-version branch 2 times, most recently from 876c5c1 to 2c66ef4 Compare May 17, 2022 16:58
@idavis idavis force-pushed the iadavis/multiple-llvm-version branch from a8d2f18 to 682d7cd Compare May 19, 2022 19:13
@idavis
Copy link
Collaborator Author

idavis commented May 26, 2022

This does not fully address #35, but makes the change very simple.

@nilslice
Copy link

Hey @idavis, thanks for this! I've tested building with qirlib and pyqir_parser, and am able to compile it from this branch as a rust lib as originally intended -- however, I still see py03 and related binding crates downloaded. This is with default-features = false set on the dependency in Cargo.toml.

Let me know there is any other specific surface area I can help test.

@idavis
Copy link
Collaborator Author

idavis commented May 31, 2022

Thanks @nilslice ! I think the issue is that pyo3 isn't marked as an optional dependency. I'll give that a try and push.

@idavis
Copy link
Collaborator Author

idavis commented May 31, 2022

@nilslice Can you please pull and try again?

@nilslice
Copy link

@idavis That was it indeed - fixed.

Copy link
Collaborator

@swernli swernli left a comment

Choose a reason for hiding this comment

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

Just a couple of questions, but overall looks ready to go!

.github/workflows/ci.yml Show resolved Hide resolved
eng/utils.ps1 Outdated Show resolved Hide resolved
@idavis idavis merged commit a4d8176 into main Jun 1, 2022
@idavis idavis deleted the iadavis/multiple-llvm-version branch June 1, 2022 01:30
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.

3 participants