Skip to content

Commit

Permalink
Name LLVM variables from codegen (#50094)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pchintalapudi
pchintalapudi authored Jun 20, 2023
1 parent 427b123 commit 0da46e2
Show file tree
Hide file tree
Showing 9 changed files with 506 additions and 112 deletions.
11 changes: 11 additions & 0 deletions src/aotcompile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ void *jl_create_native_impl(jl_array_t *methods, LLVMOrcThreadSafeModuleRef llvm
jl_codegen_params_t params(ctxt, std::move(target_info.first), std::move(target_info.second));
params.params = cgparams;
params.imaging = imaging;
params.debug_level = jl_options.debug_level;
params.external_linkage = _external_linkage;
size_t compile_for[] = { jl_typeinf_world, _world };
for (int worlds = 0; worlds < 2; worlds++) {
Expand Down Expand Up @@ -2082,6 +2083,16 @@ void jl_get_llvmf_defn_impl(jl_llvmf_dump_t* dump, jl_method_instance_t *mi, siz
jl_codegen_params_t output(*ctx, std::move(target_info.first), std::move(target_info.second));
output.world = world;
output.params = &params;
output.imaging = imaging_default();
// This would be nice, but currently it causes some assembly regressions that make printed output
// differ very significantly from the actual non-imaging mode code.
// // Force imaging mode for names of pointers
// output.imaging = true;
// This would also be nice, but it seems to cause OOMs on the windows32 builder
// // Force at least medium debug info for introspection
// No debug info = no variable names,
// max debug info = llvm.dbg.declare/value intrinsics which clutter IR output
output.debug_level = jl_options.debug_level;
auto decls = jl_emit_code(m, mi, src, jlrettype, output);
JL_UNLOCK(&jl_codegen_lock); // Might GC

Expand Down
57 changes: 53 additions & 4 deletions src/ccall.cpp
View file

Large diffs are not rendered by default.

145 changes: 121 additions & 24 deletions src/cgutils.cpp
View file

Large diffs are not rendered by default.

138 changes: 114 additions & 24 deletions src/codegen.cpp
View file

Large diffs are not rendered by default.

110 changes: 79 additions & 31 deletions src/intrinsics.cpp
View file

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions src/jitlayers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,8 @@ static jl_callptr_t _jl_compile_codeinst(
jl_codegen_params_t params(std::move(context), jl_ExecutionEngine->getDataLayout(), jl_ExecutionEngine->getTargetTriple()); // Locks the context
params.cache = true;
params.world = world;
params.imaging = imaging_default();
params.debug_level = jl_options.debug_level;
jl_workqueue_t emitted;
{
orc::ThreadSafeModule result_m =
Expand Down Expand Up @@ -358,6 +360,8 @@ int jl_compile_extern_c_impl(LLVMOrcThreadSafeModuleRef llvmmod, void *p, void *
return std::make_pair(M.getDataLayout(), Triple(M.getTargetTriple()));
});
jl_codegen_params_t params(into->getContext(), std::move(target_info.first), std::move(target_info.second));
params.imaging = imaging_default();
params.debug_level = jl_options.debug_level;
if (pparams == NULL)
pparams = &params;
assert(pparams->tsctx.getContext() == into->getContext().getContext());
Expand Down
1 change: 1 addition & 0 deletions src/jitlayers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ typedef struct _jl_codegen_params_t {
bool cache = false;
bool external_linkage = false;
bool imaging;
int debug_level;
_jl_codegen_params_t(orc::ThreadSafeContext ctx, DataLayout DL, Triple triple)
: tsctx(std::move(ctx)), tsctx_lock(tsctx.getLock()),
DL(std::move(DL)), TargetTriple(std::move(triple)), imaging(imaging_default()) {}
Expand Down
51 changes: 28 additions & 23 deletions test/cmdlineargs.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -519,29 +519,34 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`

# -g
@test readchomp(`$exename -E "Base.JLOptions().debug_level" -g`) == "2"
let code = writereadpipeline("code_llvm(stdout, +, (Int64, Int64), raw=true, dump_module=true)", `$exename -g0`)
@test code[2]
code = code[1]
@test occursin("llvm.module.flags", code)
@test !occursin("llvm.dbg.cu", code)
@test !occursin("int.jl", code)
@test !occursin("Int64", code)
end
let code = writereadpipeline("code_llvm(stdout, +, (Int64, Int64), raw=true, dump_module=true)", `$exename -g1`)
@test code[2]
code = code[1]
@test occursin("llvm.module.flags", code)
@test occursin("llvm.dbg.cu", code)
@test occursin("int.jl", code)
@test !occursin("Int64", code)
end
let code = writereadpipeline("code_llvm(stdout, +, (Int64, Int64), raw=true, dump_module=true)", `$exename -g2`)
@test code[2]
code = code[1]
@test occursin("llvm.module.flags", code)
@test occursin("llvm.dbg.cu", code)
@test occursin("int.jl", code)
@test occursin("\"Int64\"", code)
# --print-before/--print-after with pass names is broken on Windows due to no-gnu-unique issues
if !Sys.iswindows()
withenv("JULIA_LLVM_ARGS" => "--print-before=FinalLowerGC") do
let code = readchomperrors(`$exename -g0 -E "@eval Int64(1)+Int64(1)"`)
@test code[1]
code = code[3]
@test occursin("llvm.module.flags", code)
@test !occursin("llvm.dbg.cu", code)
@test !occursin("int.jl", code)
@test !occursin("\"Int64\"", code)
end
let code = readchomperrors(`$exename -g1 -E "@eval Int64(1)+Int64(1)"`)
@test code[1]
code = code[3]
@test occursin("llvm.module.flags", code)
@test occursin("llvm.dbg.cu", code)
@test occursin("int.jl", code)
@test !occursin("\"Int64\"", code)
end
let code = readchomperrors(`$exename -g2 -E "@eval Int64(1)+Int64(1)"`)
@test code[1]
code = code[3]
@test occursin("llvm.module.flags", code)
@test occursin("llvm.dbg.cu", code)
@test occursin("int.jl", code)
@test occursin("\"Int64\"", code)
end
end
end

# --check-bounds
Expand Down
101 changes: 95 additions & 6 deletions test/llvmpasses/llvmcall.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,109 @@ end
@generated foo(x)=:(ccall("extern foo", llvmcall, $x, ($x,), x))
bar(x) = ntuple(i -> VecElement{Float16}(x[i]), 2)

# CHECK: call half @foo(half %{{[0-9]+}})
# CHECK: define
# CHECK-SAME: half @julia_foo
# CHECK-SAME: {
# CHECK-NOT: define
# CHECK: [[FOO_RET:%.*]] = call half @foo(half [[FOO_ARG:%.*]])
# CHECK-NOT: define
# CHECK: ret half
# CHECK-NOT: define
# CHECK: }
emit(foo, Float16)

# CHECK: call [2 x half] @foo([2 x half] %{{[0-9]+}})
# COM: Make sure that we don't miss a function by accident (helps localize errors)
# CHECK-NOT: {
# CHECK-NOT: }
# CHECK: define
# CHECK-SAME: nonnull {} addrspace(10)* @jfptr
# CHECK-SAME: {

# CHECK: define
# CHECK-SAME: [2 x half] @julia_foo
# CHECK-SAME: {
# CHECK-NOT: define
# CHECK: [[FOO_RET:%.*]] = call [2 x half] @foo([2 x half] [[FOO_ARG:%.*]])
# CHECK-NOT: define
# CHECK: ret [2 x half]
# CHECK-NOT: define
# CHECK: }
emit(foo, NTuple{2, Float16})

# CHECK: call <2 x half> @foo(<2 x half> %{{[0-9]+}})
# COM: Make sure that we don't miss a function by accident (helps localize errors)
# CHECK-NOT: {
# CHECK-NOT: }
# CHECK: define
# CHECK-SAME: nonnull {} addrspace(10)* @jfptr
# CHECK-SAME: {

# CHECK: define
# CHECK-SAME: <2 x half> @julia_foo
# CHECK-SAME: {
# CHECK-NOT: define
# CHECK: [[FOO_RET:%.*]] call <2 x half> @foo(<2 x half> [[FOO_ARG:%.*]])
# CHECK-NOT: define
# CHECK: ret <2 x half>
# CHECK-NOT: define
# CHECK: }
emit(foo, NTuple{2, VecElement{Float16}})

# CHECK: call i8 addrspace(3)* @foo(i8 addrspace(3)* %{{[0-9]+}})
# COM: Make sure that we don't miss a function by accident (helps localize errors)
# CHECK-NOT: {
# CHECK-NOT: }
# CHECK: define
# CHECK-SAME: nonnull {} addrspace(10)* @jfptr
# CHECK-SAME: {

# CHECK: define
# CHECK-SAME: i8 addrspace(3)* @julia_foo
# CHECK-SAME: {
# CHECK-NOT: define
# CHECK: [[FOO_RET:%.*]] call i8 addrspace(3)* @foo(i8 addrspace(3)* [[FOO_ARG:%.*]])
# CHECK-NOT: define
# CHECK: ret i8 addrspace(3)*
# CHECK-NOT: define
# CHECK: }
emit(foo, Core.LLVMPtr{Float32, 3})

# CHECK: call { i32, i32 } @foo({ i32, i32 } %{{[0-9]+}})
# COM: Make sure that we don't miss a function by accident (helps localize errors)
# CHECK-NOT: {
# CHECK-NOT: }
# CHECK: define
# CHECK-SAME: nonnull {} addrspace(10)* @jfptr
# CHECK-SAME: {

# CHECK: define
# CHECK-SAME: [2 x i32] @julia_foo
# CHECK-SAME: {
# CHECK-NOT: define
# CHECK: [[FOO_RET:%.*]] call { i32, i32 } @foo({ i32, i32 } [[FOO_ARG:%.*]])
# CHECK-NOT: define
# CHECK: ret [2 x i32]
# CHECK-NOT: define
# CHECK: }
emit(foo, Foo)

# CHECK: define {{(swiftcc )?}}<2 x half> @julia_bar_{{[0-9]+}}(
# COM: Make sure that we don't miss a function by accident (helps localize errors)
# CHECK-NOT: {
# CHECK-NOT: }
# CHECK: define
# CHECK-SAME: nonnull {} addrspace(10)* @jfptr
# CHECK-SAME: {

# CHECK: define
# CHECK-SAME: <2 x half> @julia_bar
# CHECK-SAME: [2 x half]
# CHECK-SAME: {
# CHECK-NOT: define
# CHECK: ret <2 x half>
# CHECK-NOT: define
# CHECK: }
emit(bar, NTuple{2, Float16})

# COM: Make sure that we don't miss a function by accident (helps localize errors)
# CHECK-NOT: {
# CHECK-NOT: }
# CHECK: define
# CHECK-SAME: nonnull {} addrspace(10)* @jfptr
# CHECK-SAME: {

14 comments on commit 0da46e2

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

Your package evaluation job has completed - possible new issues 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.

@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

Choose a reason for hiding this comment

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

The "inference" group looks like it took quite a hit this time in performance regressions across the board in abstract interpretation. Seems like the related PRs are mostly from @Keno which didn't have runbenchmarks pre-merging. @aviatesk could you see which change might be at fault here?

@aviatesk
Copy link
Member

Choose a reason for hiding this comment

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

9d839f9 maybe? I will try to bisect it.

@aviatesk
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("inference", vs="@49572a549983c8d84575a379ccf764558e1893c3")

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

@aviatesk
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("inference", vs="@e1c0d83692accffcc63191233f7f9dd758c23f1b")

@aviatesk
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("inference", vs="@320e00db00bb95ab5e7a32bf7e00a5346fecb911")

@aviatesk
Copy link
Member

Choose a reason for hiding this comment

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

Looks like the benchmark is confused since the compared commit (49572a5) is one-year old one, which was merged without rebasing (#46410).
We should probably rebase commits before merging and avoid leaving Merge 'master' commit stuff (cc @oscardssmith ).

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

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.

@aviatesk I don't follow what the result here is. Looking at the comparison diff here, Github finds there are 46 new commits to master since the previous run, the oldest of which (49572a5) was authored a year ago, but merged more recently than that. When I try to remove that commit, I get more commits listed by Github, which seems odd, but you still seem to have reproduced the regression on that range too: https://github.com/JuliaCI/NanosoldierReports/blob/master/benchmark/by_hash/82ab124_vs_e1c0d83/report.md

Please sign in to comment.