-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
make better use of visibility attributes #49600
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,7 +96,7 @@ typedef struct { | |
std::vector<jl_code_instance_t*> jl_external_to_llvm; | ||
} jl_native_code_desc_t; | ||
|
||
extern "C" JL_DLLEXPORT | ||
extern "C" JL_DLLEXPORT_CODEGEN | ||
void jl_get_function_id_impl(void *native_code, jl_code_instance_t *codeinst, | ||
int32_t *func_idx, int32_t *specfunc_idx) | ||
{ | ||
|
@@ -110,7 +110,7 @@ void jl_get_function_id_impl(void *native_code, jl_code_instance_t *codeinst, | |
} | ||
} | ||
|
||
extern "C" JL_DLLEXPORT | ||
extern "C" JL_DLLEXPORT_CODEGEN | ||
void jl_get_llvm_gvs_impl(void *native_code, arraylist_t *gvs) | ||
{ | ||
// map a memory location (jl_value_t or jl_binding_t) to a GlobalVariable | ||
|
@@ -119,7 +119,7 @@ void jl_get_llvm_gvs_impl(void *native_code, arraylist_t *gvs) | |
memcpy(gvs->items, data->jl_value_to_llvm.data(), gvs->len * sizeof(void*)); | ||
} | ||
|
||
extern "C" JL_DLLEXPORT | ||
extern "C" JL_DLLEXPORT_CODEGEN | ||
void jl_get_llvm_external_fns_impl(void *native_code, arraylist_t *external_fns) | ||
{ | ||
jl_native_code_desc_t *data = (jl_native_code_desc_t*)native_code; | ||
|
@@ -128,7 +128,7 @@ void jl_get_llvm_external_fns_impl(void *native_code, arraylist_t *external_fns) | |
external_fns->len * sizeof(jl_code_instance_t*)); | ||
} | ||
|
||
extern "C" JL_DLLEXPORT | ||
extern "C" JL_DLLEXPORT_CODEGEN | ||
LLVMOrcThreadSafeModuleRef jl_get_llvm_module_impl(void *native_code) | ||
{ | ||
jl_native_code_desc_t *data = (jl_native_code_desc_t*)native_code; | ||
|
@@ -138,7 +138,7 @@ LLVMOrcThreadSafeModuleRef jl_get_llvm_module_impl(void *native_code) | |
return NULL; | ||
} | ||
|
||
extern "C" JL_DLLEXPORT | ||
extern "C" JL_DLLEXPORT_CODEGEN | ||
GlobalValue* jl_get_llvm_function_impl(void *native_code, uint32_t idx) | ||
{ | ||
jl_native_code_desc_t *data = (jl_native_code_desc_t*)native_code; | ||
|
@@ -160,10 +160,12 @@ static void emit_offset_table(Module &mod, const std::vector<GlobalValue*> &vars | |
addrs[i] = ConstantExpr::getBitCast(var, T_psize); | ||
} | ||
ArrayType *vars_type = ArrayType::get(T_psize, nvars); | ||
new GlobalVariable(mod, vars_type, true, | ||
auto GV = new GlobalVariable(mod, vars_type, true, | ||
GlobalVariable::ExternalLinkage, | ||
ConstantArray::get(vars_type, addrs), | ||
name); | ||
GV->setVisibility(GlobalValue::HiddenVisibility); | ||
GV->setDSOLocal(true); | ||
} | ||
|
||
static bool is_safe_char(unsigned char c) | ||
|
@@ -241,7 +243,7 @@ static void jl_ci_cache_lookup(const jl_cgparams_t &cgparams, jl_method_instance | |
*src_out = jl_uncompress_ir(def, codeinst, (jl_value_t*)*src_out); | ||
} | ||
if (*src_out == NULL || !jl_is_code_info(*src_out)) { | ||
if (cgparams.lookup != jl_rettype_inferred) { | ||
if (cgparams.lookup != jl_rettype_inferred_addr) { | ||
jl_error("Refusing to automatically run type inference with custom cache lookup."); | ||
} | ||
else { | ||
|
@@ -265,7 +267,7 @@ static void jl_ci_cache_lookup(const jl_cgparams_t &cgparams, jl_method_instance | |
// The `policy` flag switches between the default mode `0` and the extern mode `1` used by GPUCompiler. | ||
// `_imaging_mode` controls if raw pointers can be embedded (e.g. the code will be loaded into the same session). | ||
// `_external_linkage` create linkages between pkgimages. | ||
extern "C" JL_DLLEXPORT | ||
extern "C" JL_DLLEXPORT_CODEGEN | ||
void *jl_create_native_impl(jl_array_t *methods, LLVMOrcThreadSafeModuleRef llvmmod, const jl_cgparams_t *cgparams, int _policy, int _imaging_mode, int _external_linkage, size_t _world) | ||
{ | ||
JL_TIMING(NATIVE_AOT, NATIVE_Create); | ||
|
@@ -432,6 +434,7 @@ void *jl_create_native_impl(jl_array_t *methods, LLVMOrcThreadSafeModuleRef llvm | |
G->setInitializer(ConstantPointerNull::get(cast<PointerType>(G->getValueType()))); | ||
G->setLinkage(GlobalValue::ExternalLinkage); | ||
G->setVisibility(GlobalValue::HiddenVisibility); | ||
G->setDSOLocal(true); | ||
data->jl_sysimg_gvars.push_back(G); | ||
} | ||
CreateNativeGlobals += gvars.size(); | ||
|
@@ -456,6 +459,7 @@ void *jl_create_native_impl(jl_array_t *methods, LLVMOrcThreadSafeModuleRef llvm | |
if (!G.isDeclaration()) { | ||
G.setLinkage(GlobalValue::ExternalLinkage); | ||
G.setVisibility(GlobalValue::HiddenVisibility); | ||
G.setDSOLocal(true); | ||
makeSafeName(G); | ||
if (TT.isOSWindows() && TT.getArch() == Triple::x86_64) { | ||
// Add unwind exception personalities to functions to handle async exceptions | ||
|
@@ -523,6 +527,7 @@ static GlobalVariable *emit_shard_table(Module &M, Type *T_size, Type *T_psize, | |
auto gv = new GlobalVariable(M, T_size, constant, | ||
GlobalValue::ExternalLinkage, nullptr, name + suffix); | ||
gv->setVisibility(GlobalValue::HiddenVisibility); | ||
gv->setDSOLocal(true); | ||
return gv; | ||
}; | ||
auto table = tables.data() + i * sizeof(jl_image_shard_t) / sizeof(void *); | ||
|
@@ -540,6 +545,7 @@ static GlobalVariable *emit_shard_table(Module &M, Type *T_size, Type *T_psize, | |
auto tables_gv = new GlobalVariable(M, tables_arr->getType(), false, | ||
GlobalValue::ExternalLinkage, tables_arr, "jl_shard_tables"); | ||
tables_gv->setVisibility(GlobalValue::HiddenVisibility); | ||
tables_gv->setDSOLocal(true); | ||
return tables_gv; | ||
} | ||
|
||
|
@@ -550,12 +556,15 @@ static GlobalVariable *emit_ptls_table(Module &M, Type *T_size, Type *T_psize) { | |
new GlobalVariable(M, T_size, false, GlobalValue::ExternalLinkage, Constant::getNullValue(T_size), "jl_pgcstack_key_slot"), | ||
new GlobalVariable(M, T_size, false, GlobalValue::ExternalLinkage, Constant::getNullValue(T_size), "jl_tls_offset"), | ||
}; | ||
for (auto &gv : ptls_table) | ||
for (auto &gv : ptls_table) { | ||
cast<GlobalVariable>(gv)->setVisibility(GlobalValue::HiddenVisibility); | ||
cast<GlobalVariable>(gv)->setDSOLocal(true); | ||
} | ||
auto ptls_table_arr = ConstantArray::get(ArrayType::get(T_psize, ptls_table.size()), ptls_table); | ||
auto ptls_table_gv = new GlobalVariable(M, ptls_table_arr->getType(), false, | ||
GlobalValue::ExternalLinkage, ptls_table_arr, "jl_ptls_table"); | ||
ptls_table_gv->setVisibility(GlobalValue::HiddenVisibility); | ||
ptls_table_gv->setDSOLocal(true); | ||
return ptls_table_gv; | ||
} | ||
|
||
|
@@ -1101,6 +1110,8 @@ static void materializePreserved(Module &M, Partition &partition) { | |
if (!Preserve.contains(&F)) { | ||
F.deleteBody(); | ||
F.setLinkage(GlobalValue::ExternalLinkage); | ||
F.setVisibility(GlobalValue::HiddenVisibility); | ||
F.setDSOLocal(true); | ||
} | ||
} | ||
} | ||
|
@@ -1109,6 +1120,8 @@ static void materializePreserved(Module &M, Partition &partition) { | |
if (!Preserve.contains(&GV)) { | ||
GV.setInitializer(nullptr); | ||
GV.setLinkage(GlobalValue::ExternalLinkage); | ||
GV.setVisibility(GlobalValue::HiddenVisibility); | ||
GV.setDSOLocal(true); | ||
} | ||
} | ||
} | ||
|
@@ -1130,7 +1143,8 @@ static void materializePreserved(Module &M, Partition &partition) { | |
GA.setAliasee(F); | ||
|
||
DeletedAliases.push_back({ &GA, F }); | ||
} else { | ||
} | ||
else { | ||
auto GV = new GlobalVariable(M, GA.getValueType(), false, GlobalValue::ExternalLinkage, Constant::getNullValue(GA.getValueType())); | ||
DeletedAliases.push_back({ &GA, GV }); | ||
} | ||
|
@@ -1197,26 +1211,24 @@ static void construct_vars(Module &M, Partition &partition) { | |
GlobalVariable::ExternalLinkage, | ||
fidxs, "jl_fvar_idxs"); | ||
fidxs_var->setVisibility(GlobalValue::HiddenVisibility); | ||
fidxs_var->setDSOLocal(true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need to annotate these dso_local, they're removed by multiversioning later anyways. |
||
auto gidxs = ConstantDataArray::get(M.getContext(), gvar_idxs); | ||
auto gidxs_var = new GlobalVariable(M, gidxs->getType(), true, | ||
GlobalVariable::ExternalLinkage, | ||
gidxs, "jl_gvar_idxs"); | ||
gidxs_var->setVisibility(GlobalValue::HiddenVisibility); | ||
gidxs_var->setDSOLocal(true); | ||
} | ||
|
||
// Materialization will leave many unused declarations, which multiversioning would otherwise clone. | ||
// This function removes them to avoid unnecessary cloning of declarations. | ||
static void dropUnusedDeclarations(Module &M) { | ||
SmallVector<GlobalValue *> unused; | ||
// The GlobalDCEPass is much better at this, but we only care about removing unused | ||
// declarations, not actually about seeing if code is dead (codegen knows it is live, by construction). | ||
static void dropUnusedGlobals(Module &M) { | ||
std::vector<GlobalValue *> unused; | ||
for (auto &G : M.global_values()) { | ||
if (G.isDeclaration()) { | ||
if (G.use_empty()) { | ||
unused.push_back(&G); | ||
} else { | ||
G.setDSOLocal(false); // These are never going to be seen in the same module again | ||
G.setVisibility(GlobalValue::DefaultVisibility); | ||
} | ||
} | ||
if (G.isDeclaration() && G.use_empty()) | ||
unused.push_back(&G); | ||
} | ||
for (auto &G : unused) | ||
G->eraseFromParent(); | ||
|
@@ -1355,7 +1367,7 @@ static void add_output(Module &M, TargetMachine &TM, std::vector<std::string> &o | |
timers[i].construct.stopTimer(); | ||
|
||
timers[i].deletion.startTimer(); | ||
dropUnusedDeclarations(*M); | ||
dropUnusedGlobals(*M); | ||
timers[i].deletion.stopTimer(); | ||
|
||
add_output_impl(*M, TM, outputs_start + i * outcount, names_start + i * outcount, | ||
|
@@ -1450,7 +1462,7 @@ static unsigned compute_image_thread_count(const ModuleInfo &info) { | |
|
||
// takes the running content that has collected in the shadow module and dump it to disk | ||
// this builds the object file portion of the sysimage files for fast startup | ||
extern "C" JL_DLLEXPORT | ||
extern "C" JL_DLLEXPORT_CODEGEN | ||
void jl_dump_native_impl(void *native_code, | ||
const char *bc_fname, const char *unopt_bc_fname, const char *obj_fname, | ||
const char *asm_fname, | ||
|
@@ -1556,6 +1568,7 @@ void jl_dump_native_impl(void *native_code, | |
GlobalVariable::ExternalLinkage, | ||
gidxs, "jl_gvar_idxs"); | ||
gidxs_var->setVisibility(GlobalValue::HiddenVisibility); | ||
gidxs_var->setDSOLocal(true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, these variables are dropped by multiversioning. |
||
idxs.clear(); | ||
idxs.resize(data->jl_sysimg_fvars.size()); | ||
std::iota(idxs.begin(), idxs.end(), 0); | ||
|
@@ -1564,6 +1577,7 @@ void jl_dump_native_impl(void *native_code, | |
GlobalVariable::ExternalLinkage, | ||
fidxs, "jl_fvar_idxs"); | ||
fidxs_var->setVisibility(GlobalValue::HiddenVisibility); | ||
fidxs_var->setDSOLocal(true); | ||
dataM->addModuleFlag(Module::Error, "julia.mv.suffix", MDString::get(Context, "_0")); | ||
|
||
// reflect the address of the jl_RTLD_DEFAULT_handle variable | ||
|
@@ -2004,7 +2018,7 @@ static RegisterPass<JuliaPipeline<0,true>> XS("juliaO0-sysimg", "Runs the entire | |
static RegisterPass<JuliaPipeline<2,true>> YS("julia-sysimg", "Runs the entire julia pipeline (at -O2/sysimg mode)", false, false); | ||
static RegisterPass<JuliaPipeline<3,true>> ZS("juliaO3-sysimg", "Runs the entire julia pipeline (at -O3/sysimg mode)", false, false); | ||
|
||
extern "C" JL_DLLEXPORT | ||
extern "C" JL_DLLEXPORT_CODEGEN | ||
void jl_add_optimization_passes_impl(LLVMPassManagerRef PM, int opt_level, int lower_intrinsics) { | ||
addOptimizationPasses(unwrap(PM), opt_level, lower_intrinsics); | ||
} | ||
|
@@ -2014,7 +2028,7 @@ void jl_add_optimization_passes_impl(LLVMPassManagerRef PM, int opt_level, int l | |
// for use in reflection from Julia. | ||
// this is paired with jl_dump_function_ir, jl_dump_function_asm, jl_dump_method_asm in particular ways: | ||
// misuse will leak memory or cause read-after-free | ||
extern "C" JL_DLLEXPORT | ||
extern "C" JL_DLLEXPORT_CODEGEN | ||
void jl_get_llvmf_defn_impl(jl_llvmf_dump_t* dump, jl_method_instance_t *mi, size_t world, char getwrapper, char optimize, const jl_cgparams_t params) | ||
{ | ||
if (jl_is_method(mi->def.method) && mi->def.method->source == NULL && | ||
|
@@ -2027,20 +2041,22 @@ void jl_get_llvmf_defn_impl(jl_llvmf_dump_t* dump, jl_method_instance_t *mi, siz | |
// get the source code for this function | ||
jl_value_t *jlrettype = (jl_value_t*)jl_any_type; | ||
jl_code_info_t *src = NULL; | ||
JL_GC_PUSH2(&src, &jlrettype); | ||
jl_code_instance_t *codeinst = NULL; | ||
JL_GC_PUSH3(&src, &jlrettype, &codeinst); | ||
if (jl_is_method(mi->def.method) && mi->def.method->source != NULL && mi->def.method->source != jl_nothing && jl_ir_flag_inferred(mi->def.method->source)) { | ||
src = (jl_code_info_t*)mi->def.method->source; | ||
if (src && !jl_is_code_info(src)) | ||
src = jl_uncompress_ir(mi->def.method, NULL, (jl_value_t*)src); | ||
} | ||
else { | ||
jl_value_t *ci = jl_rettype_inferred(mi, world, world); | ||
jl_value_t *ci = jl_rettype_inferred_addr(mi, world, world); | ||
if (ci != jl_nothing) { | ||
jl_code_instance_t *codeinst = (jl_code_instance_t*)ci; | ||
codeinst = (jl_code_instance_t*)ci; | ||
src = (jl_code_info_t*)jl_atomic_load_relaxed(&codeinst->inferred); | ||
if ((jl_value_t*)src != jl_nothing && !jl_is_code_info(src) && jl_is_method(mi->def.method)) | ||
src = jl_uncompress_ir(mi->def.method, codeinst, (jl_value_t*)src); | ||
jlrettype = codeinst->rettype; | ||
codeinst = NULL; // not needed outside of this branch | ||
} | ||
if (!src || (jl_value_t*)src == jl_nothing) { | ||
src = jl_type_infer(mi, world, 0); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is legal because the global is defined in a different shard (initializer is nullptr). Come to think of it, that hidden visibility marker may also not be legal to have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am mostly just mechanically apply this to the HiddenVisibility values, since LLVM currently does that internally already, but may deprecate that in favor of expecting the front-end to specify both explicitly at the same time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point of this annotation is that it instructs the compiler to assume the shard is simply a piece of large library. Otherwise it assumes it is a standalone unit and inhibits optimizations that need to see they will form a single unit later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also would like to have this value, but a few lines down in dropUnusedDeclarations I needed to mark all the shard declarations as not dso_local because the linker would complain about not being able to fix up the relocations after compiling the system image. I don't know if that can become a problem here, but maybe it's worth a shot to try disabling that setting of not-dso_local?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see I missed quite a few more. This should hopefully work and correct some performance regressions I can see in the disassembly that were introduced by 6ab1862