Skip to content

Commit

Permalink
Document and clean up CodegenParams (#51140)
Browse files Browse the repository at this point in the history
- Add documentation to CodegenParams fields per request in #51123
- Fix compare_cgparams which hadn't been updated for recent additions
- Remove unused and untested generic_context

The latter was a codegen option that I added, but eventually ended up
not using anywhere, so remove it for the time being. That said, I may
end up in a situation where I need it again in the very near future, so
I may end up eating my words here, but if I need to put it back, I'll
include a test at least ;).
  • Loading branch information
Keno committed Sep 2, 2023
1 parent ec2f1d3 commit 6c1168a
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 20 deletions.
75 changes: 70 additions & 5 deletions base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1212,33 +1212,98 @@ default_debug_info_kind() = unsafe_load(cglobal(:jl_default_debug_info_kind, Cin

# this type mirrors jl_cgparams_t (documented in julia.h)
struct CodegenParams
"""
If enabled, generate the necessary code to support the --track-allocations
command line flag to julia itself. Note that the option itself does not enable
allocation tracking. Rather, it merely generates the support code necessary
to perform allocation tracking if requested by the command line option.
"""
track_allocations::Cint

"""
If enabled, generate the necessary code to support the --code-coverage
command line flag to julia itself. Note that the option itself does not enable
code coverage. Rather, it merely generates the support code necessary
to code coverage if requested by the command line option.
"""
code_coverage::Cint

"""
If enabled, force the compiler to use the specialized signature
for all generated functions, whenever legal. If disabled, the choice is made
heuristically and specsig is only used when deemed profitable.
"""
prefer_specsig::Cint

"""
If enabled, enable emission of `.debug_names` sections.
"""
gnu_pubnames::Cint

"""
Controls what level of debug info to emit. Currently supported values are:
- 0: no debug info
- 1: full debug info
- 2: Line tables only
- 3: Debug directives only
The integer values currently match the llvm::DICompilerUnit::DebugEmissionKind enum,
although this is not guaranteed.
"""
debug_info_kind::Cint

"""
If enabled, generate a GC safepoint at the entry to every function. Emitting
these extra safepoints can reduce the amount of time that other threads are
waiting for the currently running thread to reach a safepoint. The cost for
a safepoint is small, but non-zero. The option is enabled by default.
"""
safepoint_on_entry::Cint

"""
If enabled, add an implicit argument to each function call that is used to
pass down the current task local state pointer. This argument is passed
using the `swiftself` convention, which in the ordinary case means that the
pointer is kept in a register and accesses are thus very fast. If this option
is disabled, the task local state pointer must be loaded from thread local
stroage, which incurs a small amount of additional overhead. The option is enabled by
default.
"""
gcstack_arg::Cint

"""
If enabled, use the Julia PLT mechanism to support lazy-resolution of `ccall`
targets. The option may be disabled for use in environments where the julia
runtime is unavailable, but is otherwise recommended to be enabled, even if
lazy resolution is not required, as the Julia PLT mechanism may have superior
performance compared to the native platform mechanism. The options is enabled by default.
"""
use_jlplt::Cint

lookup::Ptr{Cvoid}
"""
A pointer of type
generic_context::Any
typedef jl_value_t *(*jl_codeinstance_lookup_t)(jl_method_instance_t *mi JL_PROPAGATES_ROOT,
size_t min_world, size_t max_world);
that may be used by external compilers as a callback to look up the code instance corresponding
to a particular method instance.
"""
lookup::Ptr{Cvoid}

function CodegenParams(; track_allocations::Bool=true, code_coverage::Bool=true,
prefer_specsig::Bool=false,
gnu_pubnames=true, debug_info_kind::Cint = default_debug_info_kind(),
safepoint_on_entry::Bool=true,
gcstack_arg::Bool=true, use_jlplt::Bool=true,
lookup::Ptr{Cvoid}=unsafe_load(cglobal(:jl_rettype_inferred_addr, Ptr{Cvoid})),
generic_context = nothing)
lookup::Ptr{Cvoid}=unsafe_load(cglobal(:jl_rettype_inferred_addr, Ptr{Cvoid})))
return new(
Cint(track_allocations), Cint(code_coverage),
Cint(prefer_specsig),
Cint(gnu_pubnames), debug_info_kind,
Cint(safepoint_on_entry),
Cint(gcstack_arg), Cint(use_jlplt),
lookup, generic_context)
lookup)
end
end

Expand Down
6 changes: 4 additions & 2 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4146,8 +4146,10 @@ static int compare_cgparams(const jl_cgparams_t *a, const jl_cgparams_t *b)
(a->prefer_specsig == b->prefer_specsig) &&
(a->gnu_pubnames == b->gnu_pubnames) &&
(a->debug_info_kind == b->debug_info_kind) &&
(a->lookup == b->lookup) &&
(a->generic_context == b->generic_context);
(a->safepoint_on_entry == b->safepoint_on_entry) &&
(a->gcstack_arg == b->gcstack_arg) &&
(a->use_jlplt == b->use_jlplt) &&
(a->lookup == b->lookup);
}
#endif

Expand Down
12 changes: 3 additions & 9 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1366,8 +1366,7 @@ extern "C" {
/* safepoint_on_entry */ 1,
/* gcstack_arg */ 1,
/* use_jlplt*/ 1,
/* lookup */ jl_rettype_inferred_addr,
/* generic_context */ NULL };
/* lookup */ jl_rettype_inferred_addr };
}


Expand Down Expand Up @@ -4632,15 +4631,11 @@ static jl_cgval_t emit_call(jl_codectx_t &ctx, jl_expr_t *ex, jl_value_t *rt, bo
return emit_intrinsic(ctx, fi, args, nargs - 1);
}

jl_value_t *context = ctx.params->generic_context == jl_nothing ? nullptr : ctx.params->generic_context;
size_t n_generic_args = nargs + (context ? 1 : 0);
size_t n_generic_args = nargs;

SmallVector<jl_cgval_t> generic_argv(n_generic_args);
jl_cgval_t *argv = generic_argv.data();
if (context) {
generic_argv[0] = mark_julia_const(ctx, context);
argv = &generic_argv[1];
}

argv[0] = f;
for (size_t i = 1; i < nargs; ++i) {
argv[i] = emit_expr(ctx, args[i]);
Expand Down Expand Up @@ -9304,7 +9299,6 @@ extern "C" void jl_init_llvm(void)
{
jl_page_size = jl_getpagesize();
jl_default_debug_info_kind = (int) DICompileUnit::DebugEmissionKind::FullDebug;
jl_default_cgparams.generic_context = jl_nothing;

InitializeNativeTarget();
InitializeNativeTargetAsmPrinter();
Expand Down
4 changes: 0 additions & 4 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -2381,10 +2381,6 @@ typedef struct {

// Cache access. Default: jl_rettype_inferred.
jl_codeinstance_lookup_t lookup;

// If not `nothing`, rewrite all generic calls to call
// generic_context(f, args...) instead of f(args...).
jl_value_t *generic_context;
} jl_cgparams_t;
extern JL_DLLEXPORT int jl_default_debug_info_kind;

Expand Down

5 comments on commit 6c1168a

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

@vtjnash
Copy link
Member

@vtjnash vtjnash commented on 6c1168a Sep 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

@vtjnash
Copy link
Member

@vtjnash vtjnash commented on 6c1168a Sep 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like some good inference improvement (maybe #51092?), and maybe also skipmissing is a bit faster too, but also seems like that might be noise, since I am not sure why that should be any faster

Please sign in to comment.