RFC: CodegenParams for external language implementations #19046
Conversation
| _feat_test(ctx, __FUNCTION__, (ctx)->params->feature, #feature) | ||
| static inline bool _feat_test(jl_codectx_t* ctx, const char *caller, int featval, const char *featname) { | ||
| if (featval < 0) | ||
| jl_errorf("%s for %s requires the %s language feature, which is not allowed", |
There was a problem hiding this comment.
missing :%d for *ctx->line ?
| @@ -1775,6 +1783,7 @@ static void cg_bdw(jl_binding_t *b, jl_codectx_t *ctx) | |||
| // try to statically evaluate, NULL if not possible | |||
| static jl_value_t *static_eval(jl_value_t *ex, jl_codectx_t *ctx, int sparams=true, int allow_alloc=true) | |||
| { | |||
| if (!JL_FEAT_TEST(ctx, static_alloc)) allow_alloc = 0; | |||
There was a problem hiding this comment.
Maybe this should be allow_alloc=false similar to the function parameters.
There was a problem hiding this comment.
allow_alloc sounds ambiguous, maybe not in the context of static_eval but on its own as a codegen param. Also, to contrast against dynamic_alloc...
There was a problem hiding this comment.
I meant if (!JL_FEAT_TEST(ctx, static_alloc)) allow_alloc = false; ;), just a minor nitpick
maleadt
force-pushed
the
tb/codegen_params
branch
from
d47993d
to
caf70f2
Compare
| end | ||
|
|
||
| function _dump_function(linfo::Core.MethodInstance, native::Bool, wrapper::Bool, strip_ir_metadata::Bool, dump_module::Bool, syntax::Symbol=:att) | ||
| function _dump_function(linfo::Core.MethodInstance, native::Bool, wrapper::Bool, strip_ir_metadata::Bool, dump_module::Bool, syntax::Symbol=:att, optimize::Bool=true, params::CodegenParams=CodegenParams()) |
There was a problem hiding this comment.
these are getting long, line break?
maleadt
force-pushed
the
tb/codegen_params
branch
11 times, most recently
from
c91c3c7
to
cb0f01e
Compare
maleadt
changed the title
maleadt
force-pushed
the
tb/codegen_params
branch
from
cb0f01e
to
d45ec8d
Compare
|
Took of the WIP label; I'd like some comments & review before merging this. Some things I know to be non-ideal/hacky, but can be fixed later:
|
maleadt
force-pushed
the
tb/codegen_params
branch
from
d45ec8d
to
7a11af5
Compare
|
macOS build failure seems unrelated. Any final comments? I'd like to (rebase, retest and finally) merge this PR soon. |
maleadt
force-pushed
the
tb/codegen_params
branch
from
7a11af5
to
bd8ed80
Compare
maleadt
force-pushed
the
tb/codegen_params
branch
from
bd8ed80
to
60a950b
Compare
|
I'm going to go ahead and merge this, we can always improve it later. |
Part of #18338, codegen counterpart to #18496. Used in CUDAnative.jl.
What
This PR adds a
CodegenParamsstruct containing parameters influencing how code is generated (surprise surprise). For now, it is only accepted as an argument to reflection entry-points (ie._dump_function). The struct contains two types of fields:cachedIn addition, this PR also adds an
optimizedflag to_dump_functionto dump IR, and adds a reference to the current line number for better error reporting (still need to rethink this one).Why
The goal of the
cachedparameter is not to reuse from / pollute the cache with functions that have previously been compiled, because those functions might have been compiled with different parameters, or might already be registered with the Host EE in which case we'll just get a pointer.The language feature parameters make sure we generate valid, GPU compatible IR, and generate a somewhat useful error message if we don't.
How
For us to use this functionality, there's some missing pieces still: codegen hooks (again, cfr. the inference hooks I've proposed in #18338). These allow overriding certain codegen functionality, eg.
module_setup(create module with data layout, DWARF version) ormodule_activation(CPU: register with EE, active DI; GPU: collect IR, link and mcgen afterwards).We could also generalize these hooks to replace the language features struct, eg.
hooks.dynamic_alloc = throw().Plan
Everything in this PR is up for discussion, but it's kind of the functionality I think I need to support GPU compilation. Nonetheless, I'd prefer to have it part of
master(albeit undocumented/unsupported) and iterate on it from there on. This makes it much easier for people to try and use our GPU support, at which point we'll probably get to understand how to improve CUDAnative.jl and its interface with the compiler.Also, keeping up with
masteris annoying.cc @JeffBezanson @vtjnash @Keno @yuyichao @vchuravy