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

Add back support for optimizing a single function #435

Closed
wsmoses opened this issue Aug 4, 2024 · 13 comments · Fixed by #441
Closed

Add back support for optimizing a single function #435

wsmoses opened this issue Aug 4, 2024 · 13 comments · Fixed by #441

Comments

@wsmoses
Copy link
Contributor

wsmoses commented Aug 4, 2024

Since the NewPM api's were changed, there is no longer a way to run function passes in the new PM.

This is breaking Enzyme's ability to work in 1.11 at the moment.

I started working around this here: EnzymeAD/Enzyme.jl#1699

but now finally I'm hitting a lack of NewPM related functions which were stripped out of LibLLVMExtra

deps/LLVMExtra/include/NewPM.h:LLVMModuleAnalysisManagerRef LLVMCreateNewPMModuleAnalysisManager(void);
deps/LLVMExtra/lib/NewPM.cpp:LLVMModuleAnalysisManagerRef LLVMCreateNewPMModuleAnalysisManager(void) {

Can this functionality [at least in jll] be restored?

@maleadt
Copy link
Collaborator

maleadt commented Aug 6, 2024

Since the NewPM api's were changed, there is no longer a way to run function passes in the new PM.

add!(mpm, NewPMFunctionPassManager()) do fpm
    add!(fpm, "function-pass")
end

You'll have to clarify what exactly it is you're missing.


I'm hitting a lack of NewPM related functions which were stripped out of LibLLVMExtra

deps/LLVMExtra/include/NewPM.h:LLVMModuleAnalysisManagerRef LLVMCreateNewPMModuleAnalysisManager(void);
deps/LLVMExtra/lib/NewPM.cpp:LLVMModuleAnalysisManagerRef LLVMCreateNewPMModuleAnalysisManager(void) {

Upstream has decided that the string interface is the way to go, without exposing the guts of the pass manager, so I'm not inclined to do this.

@wsmoses
Copy link
Contributor Author

wsmoses commented Aug 6, 2024

at minimum I can't presently run the Julia pass pipeline in

@ccall jl_build_newpm_pipeline(mpm.ref::Ptr{Cvoid}, pb.ref::Ptr{Cvoid}, config::Ptr{PipelineConfig})::Cvoid

@wsmoses
Copy link
Contributor Author

wsmoses commented Aug 6, 2024

In discussion with @gbaraldi I think this also presently breaks https://github.com/JuliaLang/AllocCheck.jl

@maleadt
Copy link
Collaborator

maleadt commented Aug 6, 2024

at minimum I can't presently run the Julia pass pipeline in

@ccall jl_build_newpm_pipeline(mpm.ref::Ptr{Cvoid}, pb.ref::Ptr{Cvoid}, config::Ptr{PipelineConfig})::Cvoid

Yeah, that function isn't used/supported anymore. You can run the Julia pipeline by simply using run("julia"): https://github.com/maleadt/LLVM.jl/blob/ed30abdaf2bf26c1308585c8a89137b2a58479fa/test/newpm_tests.jl#L220-L227

LLVM.jl 8.0 was a breaking release for exactly that reason. Downstream users need to be updated to the new PM wrappers, which now match LLVM's upstream design instead of rolling our own.

@wsmoses
Copy link
Contributor Author

wsmoses commented Aug 6, 2024 via email

@wsmoses
Copy link
Contributor Author

wsmoses commented Aug 6, 2024

In particular the previous function had options for [which we added to julia itself (https://github.com/JuliaLang/julia/pull/52544/files) as we used]

struct PipelineConfig
    Speedup::Cint
    Size::Cint
    lower_intrinsics::Cint
    dump_native::Cint
    external_use::Cint
    llvm_only::Cint
    always_inline::Cint
    enable_early_simplifications::Cint
    enable_early_optimizations::Cint
    enable_scalar_optimizations::Cint
    enable_loop_optimizations::Cint
    enable_vector_pipeline::Cint
    remove_ni::Cint
    cleanup::Cint
end

@maleadt
Copy link
Collaborator

maleadt commented Aug 6, 2024

I think that's a Julia "bug", where the string-based pipeline configurability isn't the same as the PipelineConfig object: https://github.com/maleadt/LLVM.jl/blob/ed30abdaf2bf26c1308585c8a89137b2a58479fa/src/interop/passes.jl#L48-L69

What other options do you need?

I'm not entirely sure this is the intended/best design though, as upstream also uses PipelineTuningOptions passed into LLVMRunJuliaPasses (which is what run! uses).

@wsmoses
Copy link
Contributor Author

wsmoses commented Aug 6, 2024

So essentially we need to run individual parts of the julia pipeline, and not its whole thing. Hence the options above let us disable all the pipeline except individual pieces

@maleadt
Copy link
Collaborator

maleadt commented Aug 6, 2024

So you want the enable_ flags? That'd probably be a good thing to add to the julia pipeline string parsing, but not worth maintaining all of the old NewPM wrappers for. Awaiting that feature, and support for it in 1.11, you can just maintain your own temporary copy of the Julia pass pipeline. LLVM.jl already has a relatively functional version: https://github.com/maleadt/LLVM.jl/blob/ed30abdaf2bf26c1308585c8a89137b2a58479fa/test/newpm_tests.jl#L231-L358

The GPUCompiler.jl optim pipeline with uses_julia_runtime=true should also be close.

@wsmoses
Copy link
Contributor Author

wsmoses commented Aug 6, 2024

We do, but it's presently in the OldPM for reasons of other non-julia passes. Trying to shim newpm passes into the oldpm is where we hit the problem at hand.

@maleadt
Copy link
Collaborator

maleadt commented Aug 6, 2024

Trying to shim newpm passes into the oldpm is where we hit the problem at hand.

I'm not sure what you mean by that. It may be relevant how GPUCompiler.jl did something like that before requiring NewPM: https://github.com/JuliaGPU/GPUCompiler.jl/pull/591/files#diff-5704fd743c05cadd3686d40b99332cdf0bd505e8186d37679496024d1dc5bfc3R651


All that said, can you please try to file more actionable issues? "Restore the old functionality" is not an productive ask, and "there is no longer a way to run function passes in the new PM" as the issue summary is just not true. I'm happy to help provide whatever functionality Enzyme.jl needs, but it's entirely unclear to me what you need.

@wsmoses
Copy link
Contributor Author

wsmoses commented Aug 6, 2024

Yeah sorry about that, I should've marked this as a question and/or discussion rather than issue as I try to figure out how to update things to LLVM8/1.11.

My original ask was restoring the ModulePass/AnalysisManager constructor/destructor functions in the jll for us to be able to use jl_build_newpm_pipeline

And my original issue was I didn't see a way in LLVM.jl to run new pass manager passes on a single function [in contrast to an entire module].

As for the first issue, @gbaraldi concurrently suggested your idea of upstreaming the config change into Julia, which I think would solve issues I'm hitting there.

As for the second issue, I think I can work around things by forcing a bunch of function passes to become module passes, but it's still a bit odd.

In essence, the shim I had earlier was something like the following to run some newpm code in the old pm:

    function propagate_julia_addrsp_tm!(pm, tm)
        function prop_julia_addr(f)
            @dispose pb=NewPMPassBuilder() begin
                fpm = NewPMFunctionPassManager()
                add!(fpm, PropagateJuliaAddrspacesPass())
                run!(pb, fpm, f, tm)
            end
            return true
        end
        add!(pm, FunctionPass("PropagateJuliaAddrSpace", prop_julia_addr))
    end

Now I think this works by changing it to be a module pass, which is fine. I'm a bit unclear on alias analysis though since I don't think those are currently registered now in the NewPM

@maleadt
Copy link
Collaborator

maleadt commented Aug 6, 2024

I didn't see a way in LLVM.jl to run new pass manager passes on a single function [in contrast to an entire module]

Ah yes, that's indeed a missing feature. We could probably generalize LLVMRunPasses to accept Function* inputs too, and use a FPM in there instead of a MPM. This should also be upstreamable.

@wsmoses wsmoses changed the title Unable to run functionpass in new PM Unable to run functionpass in new PM on a single function Aug 6, 2024
@maleadt maleadt changed the title Unable to run functionpass in new PM on a single function Add back support for optimizing a single function Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants