Skip to content

Commit 1b0fba7

Browse files
vtjnashvtjnash
authored
debuginfo: fix GC error in OC debuginfo (#57036)
Also add some extra annotations that seemed to be required locally, though unrelated to the primary change. Observed on CI: https://buildkite.com/julialang/julia-master/builds/43693#01946057-7150-4741-a756-79e7d59a7717 Fixes #57042
1 parent b23f557 commit 1b0fba7

File tree

4 files changed

+72
-61
lines changed

4 files changed

+72
-61
lines changed

‎src/aotcompile.cpp

Lines changed: 57 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -2209,65 +2209,67 @@ void jl_get_llvmf_defn_impl(jl_llvmf_dump_t* dump, jl_method_instance_t *mi, jl_
22092209
if (src && jl_is_code_info(src)) {
22102210
auto ctx = jl_ExecutionEngine->makeContext();
22112211
orc::ThreadSafeModule m = jl_create_ts_module(name_from_method_instance(mi), ctx);
2212-
uint64_t compiler_start_time = 0;
2213-
uint8_t measure_compile_time_enabled = jl_atomic_load_relaxed(&jl_measure_compile_time_enabled);
2214-
if (measure_compile_time_enabled)
2215-
compiler_start_time = jl_hrtime();
2216-
auto target_info = m.withModuleDo([&](Module &M) {
2217-
return std::make_pair(M.getDataLayout(), Triple(M.getTargetTriple()));
2218-
});
2219-
jl_codegen_params_t output(ctx, std::move(target_info.first), std::move(target_info.second));
2220-
output.params = &params;
2221-
output.imaging_mode = jl_options.image_codegen;
2222-
output.temporary_roots = jl_alloc_array_1d(jl_array_any_type, 0);
2223-
JL_GC_PUSH1(&output.temporary_roots);
2224-
auto decls = jl_emit_code(m, mi, src, NULL, output);
2225-
output.temporary_roots = nullptr;
2226-
JL_GC_POP(); // GC the global_targets array contents now since reflection doesn't need it
2227-
2228-
Function *F = NULL;
2229-
if (m) {
2230-
// if compilation succeeded, prepare to return the result
2231-
// Similar to jl_link_global from jitlayers.cpp,
2232-
// so that code_llvm shows similar codegen to the jit
2233-
for (auto &global : output.global_targets) {
2234-
if (jl_options.image_codegen) {
2235-
global.second->setLinkage(GlobalValue::ExternalLinkage);
2212+
Function *F = nullptr;
2213+
{
2214+
uint64_t compiler_start_time = 0;
2215+
uint8_t measure_compile_time_enabled = jl_atomic_load_relaxed(&jl_measure_compile_time_enabled);
2216+
if (measure_compile_time_enabled)
2217+
compiler_start_time = jl_hrtime();
2218+
auto target_info = m.withModuleDo([&](Module &M) {
2219+
return std::make_pair(M.getDataLayout(), Triple(M.getTargetTriple()));
2220+
});
2221+
jl_codegen_params_t output(ctx, std::move(target_info.first), std::move(target_info.second));
2222+
output.params = &params;
2223+
output.imaging_mode = jl_options.image_codegen;
2224+
output.temporary_roots = jl_alloc_array_1d(jl_array_any_type, 0);
2225+
JL_GC_PUSH1(&output.temporary_roots);
2226+
auto decls = jl_emit_code(m, mi, src, NULL, output);
2227+
output.temporary_roots = nullptr;
2228+
JL_GC_POP(); // GC the global_targets array contents now since reflection doesn't need it
2229+
2230+
if (m) {
2231+
// if compilation succeeded, prepare to return the result
2232+
// Similar to jl_link_global from jitlayers.cpp,
2233+
// so that code_llvm shows similar codegen to the jit
2234+
for (auto &global : output.global_targets) {
2235+
if (jl_options.image_codegen) {
2236+
global.second->setLinkage(GlobalValue::ExternalLinkage);
2237+
}
2238+
else {
2239+
auto p = literal_static_pointer_val(global.first, global.second->getValueType());
2240+
Type *elty = PointerType::get(output.getContext(), 0);
2241+
// For pretty printing, when LLVM inlines the global initializer into its loads
2242+
auto alias = GlobalAlias::create(elty, 0, GlobalValue::PrivateLinkage, global.second->getName() + ".jit", p, global.second->getParent());
2243+
global.second->setInitializer(ConstantExpr::getBitCast(alias, global.second->getValueType()));
2244+
global.second->setConstant(true);
2245+
global.second->setLinkage(GlobalValue::PrivateLinkage);
2246+
global.second->setUnnamedAddr(GlobalValue::UnnamedAddr::Global);
2247+
global.second->setVisibility(GlobalValue::DefaultVisibility);
2248+
}
22362249
}
2237-
else {
2238-
auto p = literal_static_pointer_val(global.first, global.second->getValueType());
2239-
Type *elty = PointerType::get(output.getContext(), 0);
2240-
// For pretty printing, when LLVM inlines the global initializer into its loads
2241-
auto alias = GlobalAlias::create(elty, 0, GlobalValue::PrivateLinkage, global.second->getName() + ".jit", p, global.second->getParent());
2242-
global.second->setInitializer(ConstantExpr::getBitCast(alias, global.second->getValueType()));
2243-
global.second->setConstant(true);
2244-
global.second->setLinkage(GlobalValue::PrivateLinkage);
2245-
global.second->setUnnamedAddr(GlobalValue::UnnamedAddr::Global);
2246-
global.second->setVisibility(GlobalValue::DefaultVisibility);
2250+
if (!jl_options.image_codegen) {
2251+
optimizeDLSyms(*m.getModuleUnlocked());
22472252
}
2248-
}
2249-
if (!jl_options.image_codegen) {
2250-
optimizeDLSyms(*m.getModuleUnlocked());
2251-
}
2252-
assert(!verifyLLVMIR(*m.getModuleUnlocked()));
2253-
if (optimize) {
2254-
NewPM PM{jl_ExecutionEngine->cloneTargetMachine(), getOptLevel(jl_options.opt_level)};
2255-
//Safe b/c context lock is held by output
2256-
PM.run(*m.getModuleUnlocked());
22572253
assert(!verifyLLVMIR(*m.getModuleUnlocked()));
2254+
if (optimize) {
2255+
NewPM PM{jl_ExecutionEngine->cloneTargetMachine(), getOptLevel(jl_options.opt_level)};
2256+
//Safe b/c context lock is held by output
2257+
PM.run(*m.getModuleUnlocked());
2258+
assert(!verifyLLVMIR(*m.getModuleUnlocked()));
2259+
}
2260+
const std::string *fname;
2261+
if (decls.functionObject == "jl_fptr_args" || decls.functionObject == "jl_fptr_sparam")
2262+
getwrapper = false;
2263+
if (!getwrapper)
2264+
fname = &decls.specFunctionObject;
2265+
else
2266+
fname = &decls.functionObject;
2267+
F = cast<Function>(m.getModuleUnlocked()->getNamedValue(*fname));
2268+
}
2269+
if (measure_compile_time_enabled) {
2270+
auto end = jl_hrtime();
2271+
jl_atomic_fetch_add_relaxed(&jl_cumulative_compile_time, end - compiler_start_time);
22582272
}
2259-
const std::string *fname;
2260-
if (decls.functionObject == "jl_fptr_args" || decls.functionObject == "jl_fptr_sparam")
2261-
getwrapper = false;
2262-
if (!getwrapper)
2263-
fname = &decls.specFunctionObject;
2264-
else
2265-
fname = &decls.functionObject;
2266-
F = cast<Function>(m.getModuleUnlocked()->getNamedValue(*fname));
2267-
}
2268-
if (measure_compile_time_enabled) {
2269-
auto end = jl_hrtime();
2270-
jl_atomic_fetch_add_relaxed(&jl_cumulative_compile_time, end - compiler_start_time);
22712273
}
22722274
if (F) {
22732275
dump->TSM = wrap(new orc::ThreadSafeModule(std::move(m)));

‎src/debuginfo.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,14 +162,18 @@ static void jl_profile_atomic(T f) JL_NOTSAFEPOINT
162162

163163

164164
// --- storing and accessing source location metadata ---
165-
void jl_add_code_in_flight(StringRef name, jl_code_instance_t *codeinst, const DataLayout &DL)
165+
void jl_add_code_in_flight(StringRef name, jl_code_instance_t *codeinst, const DataLayout &DL) JL_NOTSAFEPOINT_LEAVE JL_NOTSAFEPOINT_ENTER
166166
{
167167
// Non-opaque-closure MethodInstances are considered globally rooted
168168
// through their methods, but for OC, we need to create a global root
169169
// here.
170170
jl_method_instance_t *mi = jl_get_ci_mi(codeinst);
171-
if (jl_is_method(mi->def.value) && mi->def.method->is_for_opaque_closure)
171+
if (jl_is_method(mi->def.value) && mi->def.method->is_for_opaque_closure) {
172+
jl_task_t *ct = jl_current_task;
173+
int8_t gc_state = jl_gc_unsafe_enter(ct->ptls);
172174
jl_as_global_root((jl_value_t*)mi, 1);
175+
jl_gc_unsafe_leave(ct->ptls, gc_state);
176+
}
173177
getJITDebugRegistry().add_code_in_flight(name, codeinst, DL);
174178
}
175179

‎src/jitlayers.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -699,7 +699,7 @@ static void jl_compile_codeinst_now(jl_code_instance_t *codeinst)
699699
}
700700
}
701701

702-
void jl_add_code_in_flight(StringRef name, jl_code_instance_t *codeinst, const DataLayout &DL);
702+
void jl_add_code_in_flight(StringRef name, jl_code_instance_t *codeinst, const DataLayout &DL) JL_NOTSAFEPOINT_LEAVE JL_NOTSAFEPOINT_ENTER;
703703

704704
extern "C" JL_DLLEXPORT_CODEGEN
705705
void jl_emit_codeinst_to_jit_impl(

‎src/jitlayers.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,10 +178,15 @@ struct jl_locked_stream {
178178
}
179179
};
180180

181-
typedef struct _jl_llvm_functions_t {
181+
struct jl_llvm_functions_t {
182182
std::string functionObject; // jlcall llvm Function name
183183
std::string specFunctionObject; // specialized llvm Function name
184-
} jl_llvm_functions_t;
184+
jl_llvm_functions_t() JL_NOTSAFEPOINT = default;
185+
jl_llvm_functions_t &operator=(const jl_llvm_functions_t&) JL_NOTSAFEPOINT = default;
186+
jl_llvm_functions_t(const jl_llvm_functions_t &) JL_NOTSAFEPOINT = default;
187+
jl_llvm_functions_t(jl_llvm_functions_t &&) JL_NOTSAFEPOINT = default;
188+
~jl_llvm_functions_t() JL_NOTSAFEPOINT = default;
189+
};
185190

186191
struct jl_returninfo_t {
187192
llvm::FunctionCallee decl;
@@ -642,7 +647,7 @@ Module &jl_codegen_params_t::shared_module() JL_NOTSAFEPOINT {
642647
}
643648
void fixupTM(TargetMachine &TM) JL_NOTSAFEPOINT;
644649

645-
void optimizeDLSyms(Module &M);
650+
void optimizeDLSyms(Module &M) JL_NOTSAFEPOINT_LEAVE JL_NOTSAFEPOINT_ENTER;
646651

647652
// NewPM
648653
#include "passes.h"

0 commit comments

Comments
 (0)