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

Constraint LLVM_full_jll version based on Julia version #27

Closed
wants to merge 2 commits into from
Closed

Constraint LLVM_full_jll version based on Julia version #27

wants to merge 2 commits into from

Conversation

mofeing
Copy link
Collaborator

@mofeing mofeing commented Jan 5, 2024

We should only provide dialect bindings for the LLVM version that is running, which depends on the Julia version (i.e. LLVM 14 for Julia 1.9, LLVM 15 for Julia 1.10...).

Constraining a dependency based on the Julia version is impossible unless we create an intermediate auxiliary package in which we tune the version and the julia_compat settings for the configuration we seek.

Luckily for us, we don't need this since LLVM_full_jll has the correct lower-bound for the Julia versions; i.e. if we just let the package manager install it, it will fetch the correct version. The only exception is for LLVM versions that are too new for any Julia version (e.g. LLVM 16 and LLVM 17), which are mapped to the latest Julia release and fixed after the proper Julia release. In order to solve it, we just need an upper bound for the LLVM_full_jll.

Summarizing, we're just missing adding the v14 in compat to let Julia v1.9 work.

@jumerckx
Copy link
Collaborator

jumerckx commented Jan 5, 2024

Aaah, CI uncovers an annoying problem. I used getAllDerivedDefinitionsIfDefined which apparently isn't available in LLVM14...
Where should this be fixed, change the generator script to be compatible with LLVM14 and assume that will be backwards compatible for a while or compile the build script with Clang from llvm15, independent of the rest? Or any other options?

@mofeing
Copy link
Collaborator Author

mofeing commented Jan 5, 2024

I'm no expert in LLVM but since there is no stable release of MLIR yet, maybe we can set the minimum supported Julia version to v1.10 (which corresponds to LLVM 15).

Anyway, I would expect more breaking changes in future LLVM versions.

@mofeing
Copy link
Collaborator Author

mofeing commented Jan 8, 2024

@vchuravy @Pangoraw it is ok to set the minimum supported Julia version to 1.10? or do we have to find a fix for mlir-jl-tblgen on LLVM 14?

@Pangoraw Pangoraw mentioned this pull request Jan 11, 2024
@vchuravy
Copy link
Member

I would prefer a design that takes into account multiple versions of Julia from the get go. So having 1.9 is useful for now, but it it is to much work we can drop it.

Why are we depending on LLVM_full_jll?

@mofeing
Copy link
Collaborator Author

mofeing commented Jan 13, 2024

I would prefer a design that takes into account multiple versions of Julia from the get go. So having 1.9 is useful for now, but it it is to much work we can drop it.

Ahh. So you would rather generate the bindings offline and select them on precompilation? My main motivation for generating them on build was that we only need the bindings for the running Julia version and considered that committing files that can be modified in the future by the generator was a lil fragile.

I have no problem to move it back to a offline generation script, but would love to have the tblgen generator as an artifact to be used by other packages for external dialects. @jumerckx Maybe the tblgen generator can be moved to another package? or a subpackage?

Why are we depending on LLVM_full_jll?

It is required because it provides the llvm-config binary to get the required compiler flags. clang and MLIR are also provided, but we could use Clang_jll and MLIR_jll instead.

@vchuravy
Copy link
Member

I think we should:

  1. Do offline generation for the default dialects
  2. Provide the tblgen generator as a jll.

@mofeing mofeing mentioned this pull request Jan 15, 2024
2 tasks
@mofeing
Copy link
Collaborator Author

mofeing commented Jan 15, 2024

Superseded by #37

@mofeing mofeing closed this Jan 15, 2024
@mofeing mofeing deleted the constraint-llvm-to-julia branch January 15, 2024 02:36
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