Move arguments to compile/codegen into the CompilerConfig struct #668
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
And improve error message to make this easier to debug.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #668 +/- ##
==========================================
+ Coverage 70.37% 71.09% +0.71%
==========================================
Files 24 24
Lines 3403 3383 -20
==========================================
+ Hits 2395 2405 +10
+ Misses 1008 978 -30 ☔ View full report in Codecov by Sentry. |
|
Your PR requires formatting changes to meet the project's style guidelines. Click here to view the suggested changes.diff --git a/src/driver.jl b/src/driver.jl
index 3b372e0..5bed1cb 100644
--- a/src/driver.jl
+++ b/src/driver.jl
@@ -74,7 +74,7 @@ function codegen(output::Symbol, @nospecialize(job::CompilerJob); kwargs...)
config = CompilerConfig(job.config; kwargs...)
job = CompilerJob(job.source, config)
end
- compile_unhooked(output, job)
+ return compile_unhooked(output, job)
end
function compile_unhooked(output::Symbol, @nospecialize(job::CompilerJob); kwargs...)
@@ -184,7 +184,7 @@ const __llvm_initialized = Ref(false)
# deferred code generation
has_deferred_jobs = job.config.toplevel && !job.config.only_entry &&
- haskey(functions(ir), "deferred_codegen")
+ haskey(functions(ir), "deferred_codegen")
jobs = Dict{CompilerJob, String}(job => entry_fn)
if has_deferred_jobs
dyn_marker = functions(ir)["deferred_codegen"]
@@ -222,7 +222,7 @@ const __llvm_initialized = Ref(false)
for dyn_job in keys(worklist)
# cached compilation
dyn_entry_fn = get!(jobs, dyn_job) do
- config = CompilerConfig(dyn_job.config; toplevel=false)
+ config = CompilerConfig(dyn_job.config; toplevel = false)
dyn_ir, dyn_meta = codegen(:llvm, CompilerJob(dyn_job; config))
dyn_entry_fn = LLVM.name(dyn_meta.entry)
merge!(compiled, dyn_meta.compiled)
@@ -391,8 +391,10 @@ const __llvm_initialized = Ref(false)
return ir, (; entry, compiled)
end
-@locked function emit_asm(@nospecialize(job::CompilerJob), ir::LLVM.Module,
- format::LLVM.API.LLVMCodeGenFileType)
+@locked function emit_asm(
+ @nospecialize(job::CompilerJob), ir::LLVM.Module,
+ format::LLVM.API.LLVMCodeGenFileType
+ )
# NOTE: strip after validation to get better errors
if job.config.strip
@timeit_debug to "Debug info removal" strip_debuginfo!(ir)
diff --git a/src/execution.jl b/src/execution.jl
index 9b4940a..0c0e09d 100644
--- a/src/execution.jl
+++ b/src/execution.jl
@@ -16,7 +16,7 @@ function split_kwargs(kwargs, kw_groups...)
if Meta.isexpr(kwarg, :(=))
# use in macros
key, val = kwarg.args
- elseif kwarg isa Pair{Symbol,<:Any}
+ elseif kwarg isa Pair{Symbol, <:Any}
# use in functions
key, val = kwarg
else
@@ -272,13 +272,15 @@ end
if ci === nothing
ci = ci_cache_lookup(ci_cache(job), src, world, world)
if ci === nothing
- error("""Did not find CodeInstance for $job.
+ error(
+ """Did not find CodeInstance for $job.
- Pleaase make sure that the `compiler` function passed to `cached_compilation`
- invokes GPUCompiler with exactly the same configuration as passed to the API.
+ Pleaase make sure that the `compiler` function passed to `cached_compilation`
+ invokes GPUCompiler with exactly the same configuration as passed to the API.
- Note that you should do this by calling `GPUCompiler.compile`, and not by
- using reflection functions (which alter the compiler configuration).""")
+ Note that you should do this by calling `GPUCompiler.compile`, and not by
+ using reflection functions (which alter the compiler configuration)."""
+ )
end
key = (ci, cfg)
end
diff --git a/src/interface.jl b/src/interface.jl
index f9c655b..a3972ca 100644
--- a/src/interface.jl
+++ b/src/interface.jl
@@ -63,8 +63,10 @@ export CompilerConfig
# the configuration of the compiler
-const CONFIG_KWARGS = [:kernel, :name, :entry_abi, :always_inline, :opt_level,
- :libraries, :optimize, :cleanup, :validate, :strip]
+const CONFIG_KWARGS = [
+ :kernel, :name, :entry_abi, :always_inline, :opt_level,
+ :libraries, :optimize, :cleanup, :validate, :strip,
+]
"""
CompilerConfig(target, params; kernel=true, entry_abi=:specfunc, name=nothing,
@@ -116,27 +118,32 @@ struct CompilerConfig{T,P}
toplevel::Bool
only_entry::Bool
- function CompilerConfig(target::AbstractCompilerTarget, params::AbstractCompilerParams;
- kernel=true, name=nothing, entry_abi=:specfunc, toplevel=true,
- always_inline=false, opt_level=2, optimize=toplevel,
- libraries=toplevel, cleanup=toplevel, validate=toplevel,
- strip=false, only_entry=false)
+ function CompilerConfig(
+ target::AbstractCompilerTarget, params::AbstractCompilerParams;
+ kernel = true, name = nothing, entry_abi = :specfunc, toplevel = true,
+ always_inline = false, opt_level = 2, optimize = toplevel,
+ libraries = toplevel, cleanup = toplevel, validate = toplevel,
+ strip = false, only_entry = false
+ )
if entry_abi ∉ (:specfunc, :func)
error("Unknown entry_abi=$entry_abi")
end
new{typeof(target), typeof(params)}(target, params, kernel, name, entry_abi,
- always_inline, opt_level, libraries, optimize,
- cleanup, validate, strip, toplevel, only_entry)
+ always_inline, opt_level, libraries, optimize,
+ cleanup, validate, strip, toplevel, only_entry
+ )
end
end
# copy constructor
-function CompilerConfig(cfg::CompilerConfig; target=cfg.target, params=cfg.params,
- kernel=cfg.kernel, name=cfg.name, entry_abi=cfg.entry_abi,
- always_inline=cfg.always_inline, opt_level=cfg.opt_level,
- libraries=cfg.libraries, optimize=cfg.optimize, cleanup=cfg.cleanup,
- validate=cfg.validate, strip=cfg.strip, toplevel=cfg.toplevel,
- only_entry=cfg.only_entry)
+function CompilerConfig(
+ cfg::CompilerConfig; target = cfg.target, params = cfg.params,
+ kernel = cfg.kernel, name = cfg.name, entry_abi = cfg.entry_abi,
+ always_inline = cfg.always_inline, opt_level = cfg.opt_level,
+ libraries = cfg.libraries, optimize = cfg.optimize, cleanup = cfg.cleanup,
+ validate = cfg.validate, strip = cfg.strip, toplevel = cfg.toplevel,
+ only_entry = cfg.only_entry
+ )
# deriving a non-toplevel job disables certain features
# XXX: should we keep track if any of these were set explicitly in the first place?
# see how PkgEval does that.
@@ -146,8 +153,10 @@ function CompilerConfig(cfg::CompilerConfig; target=cfg.target, params=cfg.param
cleanup = false
validate = false
end
- CompilerConfig(target, params; kernel, entry_abi, name, always_inline, opt_level,
- libraries, optimize, cleanup, validate, strip, toplevel, only_entry)
+ return CompilerConfig(
+ target, params; kernel, entry_abi, name, always_inline, opt_level,
+ libraries, optimize, cleanup, validate, strip, toplevel, only_entry
+ )
end
function Base.show(io::IO, @nospecialize(cfg::CompilerConfig{T})) where {T}
@@ -194,13 +203,14 @@ struct CompilerJob{T,P}
config::CompilerConfig{T,P}
world::UInt
- CompilerJob(source::MethodInstance, config::CompilerConfig{T,P},
+ CompilerJob(
+ source::MethodInstance, config::CompilerConfig{T, P},
world=tls_world_age()) where {T,P} =
- new{T,P}(source, config, world)
+ new{T, P}(source, config, world)
end
# copy constructor
-CompilerJob(job::CompilerJob; source=job.source, config=job.config, world=job.world) =
+CompilerJob(job::CompilerJob; source = job.source, config = job.config, world = job.world) =
CompilerJob(source, config, world)
function Base.hash(job::CompilerJob, h::UInt)
diff --git a/src/precompile.jl b/src/precompile.jl
index 8f62451..969aeff 100644
--- a/src/precompile.jl
+++ b/src/precompile.jl
@@ -28,7 +28,7 @@ using PrecompileTools: @setup_workload, @compile_workload
params = precompile_module.DummyCompilerParams()
# XXX: on Windows, compiling the GPU runtime leaks GPU code in the native cache,
# so prevent building the runtime library (see JuliaGPU/GPUCompiler.jl#601)
- config = CompilerConfig(target, params; libraries=false)
+ config = CompilerConfig(target, params; libraries = false)
job = CompilerJob(source, config)
JuliaContext() do ctx
diff --git a/src/reflection.jl b/src/reflection.jl
index df1a0a4..3a7155f 100644
--- a/src/reflection.jl
+++ b/src/reflection.jl
@@ -186,7 +186,7 @@ See also: [`@device_code_llvm`](@ref), `InteractiveUtils.code_llvm`
function code_llvm(io::IO, @nospecialize(job::CompilerJob); optimize::Bool=true, raw::Bool=false,
debuginfo::Symbol=:default, dump_module::Bool=false, kwargs...)
# NOTE: jl_dump_function_ir supports stripping metadata, so don't do it in the driver
- config = CompilerConfig(job.config; validate=false, strip=false)
+ config = CompilerConfig(job.config; validate = false, strip = false)
str = JuliaContext() do ctx
ir, meta = compile(:llvm, CompilerJob(job; config))
ts_mod = ThreadSafeModule(ir)
@@ -215,9 +215,11 @@ The following keyword arguments are supported:
See also: [`@device_code_native`](@ref), `InteractiveUtils.code_llvm`
"""
-function code_native(io::IO, @nospecialize(job::CompilerJob);
- raw::Bool=false, dump_module::Bool=false)
- config = CompilerConfig(job.config; strip=!raw, only_entry=!dump_module, validate=false)
+function code_native(
+ io::IO, @nospecialize(job::CompilerJob);
+ raw::Bool = false, dump_module::Bool = false
+ )
+ config = CompilerConfig(job.config; strip = !raw, only_entry = !dump_module, validate = false)
asm, meta = JuliaContext() do ctx
compile(:asm, CompilerJob(job; config))
end
diff --git a/src/rtlib.jl b/src/rtlib.jl
index 42faaeb..4e4ef25 100644
--- a/src/rtlib.jl
+++ b/src/rtlib.jl
@@ -99,7 +99,7 @@ function build_runtime(@nospecialize(job::CompilerJob))
# the compiler job passed into here is identifies the job that requires the runtime.
# derive a job that represents the runtime itself (notably with kernel=false).
- config = CompilerConfig(job.config; kernel=false, toplevel=false)
+ config = CompilerConfig(job.config; kernel = false, toplevel = false)
for method in values(Runtime.methods)
def = if isa(method.def, Symbol)
diff --git a/src/spirv.jl b/src/spirv.jl
index 7e27c34..ef09e81 100644
--- a/src/spirv.jl
+++ b/src/spirv.jl
@@ -184,7 +184,7 @@ end
# reimplementation that uses `spirv-dis`, giving much more pleasant output
function code_native(io::IO, job::CompilerJob{SPIRVCompilerTarget}; raw::Bool=false, dump_module::Bool=false)
- config = CompilerConfig(job.config; strip=!raw, only_entry=!dump_module, validate=false)
+ config = CompilerConfig(job.config; strip = !raw, only_entry = !dump_module, validate = false)
obj, _ = JuliaContext() do ctx
compile(:obj, CompilerJob(job; config))
end
diff --git a/test/helpers/bpf.jl b/test/helpers/bpf.jl
index d66b6b4..324bca4 100644
--- a/test/helpers/bpf.jl
+++ b/test/helpers/bpf.jl
@@ -11,7 +11,7 @@ function create_job(@nospecialize(func), @nospecialize(types); kwargs...)
source = methodinstance(typeof(func), Base.to_tuple_type(types), Base.get_world_counter())
target = BPFCompilerTarget()
params = CompilerParams()
- config = CompilerConfig(target, params; kernel=false, config_kwargs...)
+ config = CompilerConfig(target, params; kernel = false, config_kwargs...)
CompilerJob(source, config), kwargs
end
diff --git a/test/helpers/gcn.jl b/test/helpers/gcn.jl
index f7f54f8..95e4a92 100644
--- a/test/helpers/gcn.jl
+++ b/test/helpers/gcn.jl
@@ -11,7 +11,7 @@ function create_job(@nospecialize(func), @nospecialize(types); kwargs...)
source = methodinstance(typeof(func), Base.to_tuple_type(types), Base.get_world_counter())
target = GCNCompilerTarget(dev_isa="gfx900")
params = CompilerParams()
- config = CompilerConfig(target, params; kernel=false, config_kwargs...)
+ config = CompilerConfig(target, params; kernel = false, config_kwargs...)
CompilerJob(source, config), kwargs
end
diff --git a/test/helpers/metal.jl b/test/helpers/metal.jl
index d46f9a8..8f556e7 100644
--- a/test/helpers/metal.jl
+++ b/test/helpers/metal.jl
@@ -11,7 +11,7 @@ function create_job(@nospecialize(func), @nospecialize(types); kwargs...)
source = methodinstance(typeof(func), Base.to_tuple_type(types), Base.get_world_counter())
target = MetalCompilerTarget(; macos=v"12.2", metal=v"3.0", air=v"3.0")
params = CompilerParams()
- config = CompilerConfig(target, params; kernel=false, config_kwargs...)
+ config = CompilerConfig(target, params; kernel = false, config_kwargs...)
CompilerJob(source, config), kwargs
end
diff --git a/test/helpers/native.jl b/test/helpers/native.jl
index d53ff17..f933905 100644
--- a/test/helpers/native.jl
+++ b/test/helpers/native.jl
@@ -20,13 +20,15 @@ GPUCompiler.runtime_module(::NativeCompilerJob) = TestRuntime
GPUCompiler.method_table(@nospecialize(job::NativeCompilerJob)) = job.config.params.method_table
GPUCompiler.can_safepoint(@nospecialize(job::NativeCompilerJob)) = job.config.params.entry_safepoint
-function create_job(@nospecialize(func), @nospecialize(types);
- entry_safepoint::Bool=false, method_table=test_method_table, kwargs...)
+function create_job(
+ @nospecialize(func), @nospecialize(types);
+ entry_safepoint::Bool = false, method_table = test_method_table, kwargs...
+ )
config_kwargs, kwargs = split_kwargs(kwargs, GPUCompiler.CONFIG_KWARGS)
source = methodinstance(typeof(func), Base.to_tuple_type(types), Base.get_world_counter())
target = NativeCompilerTarget()
params = CompilerParams(entry_safepoint, method_table)
- config = CompilerConfig(target, params; kernel=false, config_kwargs...)
+ config = CompilerConfig(target, params; kernel = false, config_kwargs...)
CompilerJob(source, config), kwargs
end
@@ -81,7 +83,7 @@ end
# simulates cached codegen
function cached_execution(@nospecialize(func), @nospecialize(types); kwargs...)
- job, kwargs = create_job(func, types; validate=false, kwargs...)
+ job, kwargs = create_job(func, types; validate = false, kwargs...)
GPUCompiler.cached_compilation(runtime_cache, job.source, job.config, compiler, linker)
end
diff --git a/test/helpers/ptx.jl b/test/helpers/ptx.jl
index 5f8a3c4..6abb19c 100644
--- a/test/helpers/ptx.jl
+++ b/test/helpers/ptx.jl
@@ -35,15 +35,17 @@ module PTXTestRuntime
end
GPUCompiler.runtime_module(::PTXCompilerJob) = PTXTestRuntime
-function create_job(@nospecialize(func), @nospecialize(types);
- minthreads=nothing, maxthreads=nothing,
- blocks_per_sm=nothing, maxregs=nothing,
- kwargs...)
+function create_job(
+ @nospecialize(func), @nospecialize(types);
+ minthreads = nothing, maxthreads = nothing,
+ blocks_per_sm = nothing, maxregs = nothing,
+ kwargs...
+ )
config_kwargs, kwargs = split_kwargs(kwargs, GPUCompiler.CONFIG_KWARGS)
source = methodinstance(typeof(func), Base.to_tuple_type(types), Base.get_world_counter())
- target = PTXCompilerTarget(; cap=v"7.0", minthreads, maxthreads, blocks_per_sm, maxregs)
+ target = PTXCompilerTarget(; cap = v"7.0", minthreads, maxthreads, blocks_per_sm, maxregs)
params = CompilerParams()
- config = CompilerConfig(target, params; kernel=false, config_kwargs...)
+ config = CompilerConfig(target, params; kernel = false, config_kwargs...)
CompilerJob(source, config), kwargs
end
diff --git a/test/helpers/spirv.jl b/test/helpers/spirv.jl
index 73d030d..8782ac2 100644
--- a/test/helpers/spirv.jl
+++ b/test/helpers/spirv.jl
@@ -7,14 +7,15 @@ struct CompilerParams <: AbstractCompilerParams end
GPUCompiler.runtime_module(::CompilerJob{<:Any,CompilerParams}) = TestRuntime
function create_job(@nospecialize(func), @nospecialize(types);
- supports_fp16=true, supports_fp64=true, backend::Symbol,
- kwargs...)
+ supports_fp16 = true, supports_fp64 = true, backend::Symbol,
+ kwargs...
+ )
config_kwargs, kwargs = split_kwargs(kwargs, GPUCompiler.CONFIG_KWARGS)
source = methodinstance(typeof(func), Base.to_tuple_type(types), Base.get_world_counter())
target = SPIRVCompilerTarget(; backend, validate=true, optimize=true,
supports_fp16, supports_fp64)
params = CompilerParams()
- config = CompilerConfig(target, params; kernel=false, config_kwargs...)
+ config = CompilerConfig(target, params; kernel = false, config_kwargs...)
CompilerJob(source, config), kwargs
end
diff --git a/test/metal.jl b/test/metal.jl
index ae85435..be10acf 100644
--- a/test/metal.jl
+++ b/test/metal.jl
@@ -114,7 +114,7 @@ end
return
end
- @test_throws_message(InvalidIRError, Metal.code_execution(kernel2, Tuple{Core.LLVMPtr{Float64,1}})) do msg
+ @test_throws_message(InvalidIRError, Metal.code_execution(kernel2, Tuple{Core.LLVMPtr{Float64, 1}})) do msg
occursin("unsupported use of double value", msg)
end
end
diff --git a/test/native.jl b/test/native.jl
index 19a6297..3c224dd 100644
--- a/test/native.jl
+++ b/test/native.jl
@@ -50,7 +50,7 @@ end
@noinline inner(x) = x+1
foo(x) = sum(inner, fill(x, 10, 10))
- job, _ = Native.create_job(foo, (Float64,); validate=false)
+ job, _ = Native.create_job(foo, (Float64,); validate = false)
JuliaContext() do ctx
# shouldn't segfault
ir, meta = GPUCompiler.compile(:llvm, job)
diff --git a/test/native/precompile.jl b/test/native/precompile.jl
index 6fe981a..fe1be1a 100644
--- a/test/native/precompile.jl
+++ b/test/native/precompile.jl
@@ -56,7 +56,7 @@ precompile_test_harness("Inference caching") do load_path
GPUCompiler.enable_disk_cache!()
@test GPUCompiler.disk_cache_enabled() == true
- job, _ = NativeCompiler.Native.create_job(NativeBackend.kernel, (Vector{Int}, Int); validate=false)
+ job, _ = NativeCompiler.Native.create_job(NativeBackend.kernel, (Vector{Int}, Int); validate = false)
@assert job.source == kernel_mi
ci = GPUCompiler.ci_cache_lookup(GPUCompiler.ci_cache(job), job.source, job.world, job.world)
@assert ci !== nothing
diff --git a/test/spirv.jl b/test/spirv.jl
index e14ccf7..3d0ce6d 100644
--- a/test/spirv.jl
+++ b/test/spirv.jl
@@ -48,28 +48,40 @@ end
end
ir = sprint(io->SPIRV.code_llvm(io, mod.kernel, Tuple{Ptr{Float16}, Float16};
- backend))
+ backend
+ )
+ )
@test occursin("store half", ir)
ir = sprint(io->SPIRV.code_llvm(io, mod.kernel, Tuple{Ptr{Float32}, Float32};
- backend))
+ backend
+ )
+ )
@test occursin("store float", ir)
ir = sprint(io->SPIRV.code_llvm(io, mod.kernel, Tuple{Ptr{Float64}, Float64};
- backend))
+ backend
+ )
+ )
@test occursin("store double", ir)
@test_throws_message(InvalidIRError,
- SPIRV.code_execution(mod.kernel, Tuple{Ptr{Float16}, Float16};
- backend, supports_fp16=false)) do msg
+ SPIRV.code_execution(
+ mod.kernel, Tuple{Ptr{Float16}, Float16};
+ backend, supports_fp16 = false
+ )
+ ) do msg
occursin("unsupported use of half value", msg) &&
occursin("[1] unsafe_store!", msg) &&
occursin("[2] kernel", msg)
end
@test_throws_message(InvalidIRError,
- SPIRV.code_execution(mod.kernel, Tuple{Ptr{Float64}, Float64};
- backend, supports_fp64=false)) do msg
+ SPIRV.code_execution(
+ mod.kernel, Tuple{Ptr{Float64}, Float64};
+ backend, supports_fp64 = false
+ )
+ ) do msg
occursin("unsupported use of double value", msg) &&
occursin("[1] unsafe_store!", msg) &&
occursin("[2] kernel", msg) |
|
@wsmoses Enzyme.jl should not call |
|
@wsmoses Looks like Enzyme.jl is still broken. Can you take a look? I'll wait with tagging this change. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The idea behind
CompilerConfigwas that it describes how the compiler should behave, so it should include most of the flags we currently pass tocompile/codegen(e.g.,optimize,libraries, etc).This should also make caching more correct, as we currently were not including e.g. the optimization level in the logic that loads previous compilation results from the in-memory or disk cache.
This is an API change, so will break back-ends.
EDIT: managed to make it non-breaking.