JuliaLabs / MLIR.jl

MIT License
56 stars 9 forks source link

Replace `Base.libllvm_version` with `libmlir_version` preference #70

Closed mofeing closed 6 months ago

mofeing commented 6 months ago

I didn't put this into #69 because this might require more discussion.

This PR lets the user select the MLIR version they want on compile-time.

Wait, why do this?

Well, several projects experimenting with MLIR.jl, like Coil.jl and Reactant.jl, need to import the whole MLIR.jl code just because they use another libMLIR (included in XLA or IREE). They just need a way to overwrite MLIR_jll.mlir_c.

Since we conditionally load code based on Julia's LLVM version (mainly for MLIR.API and MLIR.Dialects), the loaded code won't match the version of the custom MLIR loaded version.

This PR solves it by letting the user explicitly set the correct MLIR version using Preferences.

Upstreaming a patch for adding a mlir_version function in libMLIR-C would remove the need for this, since the users would just overwrite mlir_c in MLIR_jll and we could ask the version to it. Meanwhile, and for supporting older versions, this patch will do the trick.

wsmoses commented 6 months ago

I'm not quite sure this suffices, since someone else in the julia ecosystem would have their MLIR broken so you could not have a package using mlir.jl and the mlir with different lib at the same time.

mofeing commented 6 months ago

I'm not quite sure this suffices, since someone else in the julia ecosystem would have their MLIR broken so you could not have a package using mlir.jl and the mlir with different lib at the same time.

This can be solved by setting mlir_c to be a ScopedValue. But the remaining problem is the conditional code loading depending on MLIR version.

IMO we should remove the conditional code load and do some kind of dynamic dispatch based on some runtime MLIR version.

To my mind comes the following idea: Any user who needs to use an external libMLIR should create an VersionDispatcher (see below) and call the API through it. All the calls would be dynamically dispatched, but I think that's not really a problem or at least, it's a price I'm willing to pay.

struct VersionDispatcher
    version::VersionNumber
end

function getproperty(wrapper::VersionDispatcher, key::Symbol)
    if v"15" <= wrapper.version < v"16"
        getproperty(API.v15, key)
    elseif v"16" <= wrapper.version < v"17"
        getproperty(API.v16, key)
    elseif v"17" <= wrapper.version < v"18"
        getproperty(API.v17, key)
    else
        error("Not a valid MLIR version: $(wrapper.version)")
    end
end

# example
dispatcher = VersionDispatcher(v"16")
dispatcher.mlirBlockCreate(...)

Upstreaming a patch for adding a mlir_version function in libMLIR-C would remove the need for this, since the users would just overwrite mlir_c in MLIR_jll and we could ask the version to it. Meanwhile, and for supporting older versions, this patch will do the trick.

If we get this done, then we could do the VersionDispatcher and the ScopedValue change of scope all in one move.

using ScopedValues

struct VersionDispatcher
    version::VersionNumber
    libmlir::String
end

function VersionDispatcher(libmlir::String)
    version = VersionNumber(@ccall libmlir.version()::Cstring)
    VersionDispatcher(version, libmlir)
end

function getproperty(wrapper::VersionDispatcher, key::Symbol)
    with(mlir_c => wrapper.libmlir) do 
        if v"15" <= wrapper.version < v"16"
            getproperty(API.v15, key)
        elseif ...
            ...
        end
    end
end

EDIT: Revising the code, I now see that we should be careful with some parts. Specifically type definitions in MLIR.IR submodule (e.g. IR.Block has a field of type API.MlirBlock where API contents are versioned). I don't think it's a blocker because most type definitions in API are hidden behind a Ptr{Cvoid}, but we must move the struct definitions out of API and the binding generator.

Pangoraw commented 6 months ago

FYI, another reason for Coil not currently using MLIR.jl is that it uses a custom ccall macro to handle the versioned symbols provided by the upstream libIREECompiler. I need to build a libIREECompiler_jll without versionning.

mofeing commented 6 months ago

Superseeded by #71