Skip to content

Commit

Permalink
Initialize gvars with their pointer values in create_native (#50838)
Browse files Browse the repository at this point in the history 
GPUCompiler won't have access to the runtime pointers for global
variables emitted from `jl_create_native`, so we need to provide them
here.
  • Loading branch information
@pchintalapudi
pchintalapudi authored Aug 10, 2023
1 parent 64c5292 commit 2d24155
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/aotcompile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ void *jl_create_native_impl(jl_array_t *methods, LLVMOrcThreadSafeModuleRef llvm
size_t idx = 0;
for (auto &global : params.global_targets) {
gvars[idx] = global.second->getName().str();
global.second->setInitializer(literal_static_pointer_val(global.first, global.second->getValueType()));
assert(gvars_set.insert(global.second).second && "Duplicate gvar in params!");
assert(gvars_names.insert(gvars[idx]).second && "Duplicate gvar name in params!");
data->jl_value_to_llvm[idx] = global.first;
Expand Down Expand Up @@ -431,7 +432,7 @@ void *jl_create_native_impl(jl_array_t *methods, LLVMOrcThreadSafeModuleRef llvm
for (auto &global : gvars) {
//Safe b/c context is locked by params
GlobalVariable *G = cast<GlobalVariable>(clone.getModuleUnlocked()->getNamedValue(global));
G->setInitializer(ConstantPointerNull::get(cast<PointerType>(G->getValueType())));
assert(G->hasInitializer());
G->setLinkage(GlobalValue::InternalLinkage);
G->setDSOLocal(true);
data->jl_sysimg_gvars.push_back(G);
Expand Down Expand Up @@ -1568,6 +1569,11 @@ void jl_dump_native_impl(void *native_code,

Type *T_psize = dataM.getDataLayout().getIntPtrType(Context)->getPointerTo();

// Wipe the global initializers, we'll reset them at load time
for (auto gv : data->jl_sysimg_gvars) {
cast<GlobalVariable>(gv)->setInitializer(Constant::getNullValue(gv->getValueType()));
}

// add metadata information
if (imaging_mode) {
multiversioning_preannotate(dataM);
Expand Down

7 comments on commit 2d24155

@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)

@vtjnash
Copy link
Member

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.

@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

Choose a reason for hiding this comment

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

skipmissing seemed to get wrecked (50% slower) by something about these 22 commits?

["union", "array", ("skipmissing", "collect", "Int64", 0)]	1.80 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "eachindex", "Union{Missing, BigFloat}", 1)]	1.37 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "eachindex", "Union{Missing, BigInt}", 1)]	1.41 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "eachindex", "Union{Missing, Bool}", 1)]	1.54 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "eachindex", "Union{Missing, ComplexF64}", 1)]	1.50 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "eachindex", "Union{Missing, Float32}", 1)]	1.51 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "eachindex", "Union{Missing, Float64}", 1)]	1.51 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "eachindex", "Union{Missing, Int64}", 1)]	1.42 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "eachindex", "Union{Missing, Int8}", 1)]	1.54 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "keys", "Union{Missing, BigFloat}", 1)]	1.42 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "keys", "Union{Missing, BigInt}", 1)]	1.41 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "keys", "Union{Missing, Bool}", 1)]	1.45 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "keys", "Union{Missing, ComplexF64}", 1)]	1.50 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "keys", "Union{Missing, Float32}", 1)]	1.46 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "keys", "Union{Missing, Float64}", 1)]	1.45 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "keys", "Union{Missing, Int64}", 1)]	1.45 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "keys", "Union{Missing, Int8}", 1)]	1.45 (5%) ❌	1.00 (1%)
["union", "array", ("skipmissing", "perf_sumskipmissing", "Int8", 0)]	1.14 (5%) ❌	1.00 (1%)

@nanosoldier runbenchmarks("skipmissing", vs="@117ef2e6911339a6eed8a6209853f59518f9bc24")

@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

Choose a reason for hiding this comment

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

Seems to have just been an odd fluke

Please sign in to comment.