Skip to content
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

Name LLVM function arguments #50500

Merged
merged 5 commits into from
Jul 15, 2023
Merged

Name LLVM function arguments #50500

merged 5 commits into from
Jul 15, 2023

Conversation

pchintalapudi
Copy link
Member

This makes it easier to correlate LLVM IR with the originating source code by including both argument name and argument type in the LLVM argument variable.

Example 1
julia> function f(a, b, c, d, g...)
           e = a + b + c + d
           f = does_not_exist(e) + e
           f
       end
f (generic function with 1 method)

julia> @code_llvm f(0,0,0,0,0)
;  @ REPL[1]:1 within `f`
define nonnull {}* @julia_f_141(i64 signext %"a::Int64", i64 signext %"b::Int64", i64 signext %"c::Int64", i64 signext %"d::Int64", i64 signext %"g[0]::Int64") #0 {
top:
  %0 = alloca [2 x {}*], align 8
  %gcframe3 = alloca [4 x {}*], align 16
  %gcframe3.sub = getelementptr inbounds [4 x {}*], [4 x {}*]* %gcframe3, i64 0, i64 0
  %1 = bitcast [4 x {}*]* %gcframe3 to i8*
  call void @llvm.memset.p0i8.i64(i8* align 16 %1, i8 0, i64 32, i1 true)
  %thread_ptr = call i8* asm "movq %fs:0, $0", "=r"() #7
  %tls_ppgcstack = getelementptr i8, i8* %thread_ptr, i64 -8
  %2 = bitcast i8* %tls_ppgcstack to {}****
  %tls_pgcstack = load {}***, {}**** %2, align 8
;  @ REPL[1]:3 within `f`
  %3 = bitcast [4 x {}*]* %gcframe3 to i64*
  store i64 8, i64* %3, align 16
  %4 = getelementptr inbounds [4 x {}*], [4 x {}*]* %gcframe3, i64 0, i64 1
  %5 = bitcast {}** %4 to {}***
  %6 = load {}**, {}*** %tls_pgcstack, align 8
  store {}** %6, {}*** %5, align 8
  %7 = bitcast {}*** %tls_pgcstack to {}***
  store {}** %gcframe3.sub, {}*** %7, align 8
  %Main.does_not_exist.cached = load atomic {}*, {}** @0 unordered, align 8
  %iscached.not = icmp eq {}* %Main.does_not_exist.cached, null
  br i1 %iscached.not, label %notfound, label %found

notfound:                                         ; preds = %top
  %Main.does_not_exist.found = call {}* @ijl_get_binding_or_error({}* nonnull inttoptr (i64 139831437630272 to {}*), {}* nonnull inttoptr (i64 139831600565400 to {}*))
  store atomic {}* %Main.does_not_exist.found, {}** @0 release, align 8
  br label %found

found:                                            ; preds = %notfound, %top
  %Main.does_not_exist = phi {}* [ %Main.does_not_exist.cached, %top ], [ %Main.does_not_exist.found, %notfound ]
  %8 = bitcast {}* %Main.does_not_exist to {}**
  %does_not_exist.checked = load atomic {}*, {}** %8 unordered, align 8
  %.not = icmp eq {}* %does_not_exist.checked, null
  br i1 %.not, label %err, label %ok

err:                                              ; preds = %found
  call void @ijl_undefined_var_error({}* inttoptr (i64 139831600565400 to {}*))
  unreachable

ok:                                               ; preds = %found
  %.sub = getelementptr inbounds [2 x {}*], [2 x {}*]* %0, i64 0, i64 0
;  @ REPL[1]:2 within `f`
; ┌ @ operators.jl:587 within `+` @ int.jl:87
   %9 = add i64 %"b::Int64", %"a::Int64"
   %10 = add i64 %9, %"c::Int64"
; │ @ operators.jl:587 within `+`
; │┌ @ operators.jl:544 within `afoldl`
; ││┌ @ int.jl:87 within `+`
     %11 = add i64 %10, %"d::Int64"
     %12 = getelementptr inbounds [4 x {}*], [4 x {}*]* %gcframe3, i64 0, i64 3
     store {}* %does_not_exist.checked, {}** %12, align 8
; └└└
;  @ REPL[1]:3 within `f`
  %13 = call nonnull {}* @ijl_box_int64(i64 signext %11)
  %14 = getelementptr inbounds [4 x {}*], [4 x {}*]* %gcframe3, i64 0, i64 2
  store {}* %13, {}** %14, align 16
  store {}* %13, {}** %.sub, align 8
  %15 = call nonnull {}* @ijl_apply_generic({}* nonnull %does_not_exist.checked, {}** nonnull %.sub, i32 1)
  store {}* %15, {}** %12, align 8
  %16 = call nonnull {}* @ijl_box_int64(i64 signext %11)
  store {}* %16, {}** %14, align 16
  store {}* %15, {}** %.sub, align 8
  %17 = getelementptr inbounds [2 x {}*], [2 x {}*]* %0, i64 0, i64 1
  store {}* %16, {}** %17, align 8
  %18 = call nonnull {}* @ijl_apply_generic({}* inttoptr (i64 139831370516384 to {}*), {}** nonnull %.sub, i32 2)
  %19 = load {}*, {}** %4, align 8
  %20 = bitcast {}*** %tls_pgcstack to {}**
  store {}* %19, {}** %20, align 8
;  @ REPL[1]:4 within `f`
  ret {}* %18
}
Example 2
julia> function g(a, b, c, d; kwarg=0)
           a + b + c + d + kwarg
       end
g (generic function with 1 method)

julia> @code_llvm g(0,0,0,0,kwarg=0)
;  @ REPL[3]:1 within `g`
define i64 @julia_g_160([1 x i64]* nocapture noundef nonnull readonly align 8 dereferenceable(8) %"#1::NamedTuple", i64 signext %"a::Int64", i64 signext %"b::Int64", i64 signext %"c::Int64", i64 signext %"d::Int64") #0 {
top:
  %0 = getelementptr inbounds [1 x i64], [1 x i64]* %"#1::NamedTuple", i64 0, i64 0
; ┌ @ REPL[3]:2 within `#g#1`
; │┌ @ operators.jl:587 within `+` @ int.jl:87
    %1 = add i64 %"b::Int64", %"a::Int64"
    %2 = add i64 %1, %"c::Int64"
; ││ @ operators.jl:587 within `+`
; ││┌ @ operators.jl:544 within `afoldl`
; │││┌ @ int.jl:87 within `+`
      %3 = add i64 %2, %"d::Int64"
; │││└
; │││ @ operators.jl:545 within `afoldl`
; │││┌ @ int.jl:87 within `+`
      %unbox = load i64, i64* %0, align 8
      %4 = add i64 %3, %unbox
; └└└└
  ret i64 %4
}

@pchintalapudi pchintalapudi added the compiler:llvm For issues that relate to LLVM label Jul 11, 2023
@gbaraldi
Copy link
Member

LGTM, I was thinking about this today, the result seems very cool.

@pchintalapudi
Copy link
Member Author

I guess the next question is if we want this enabled always or just when running with -g2

@Seelengrab
Copy link
Contributor

Seelengrab commented Jul 11, 2023

i64 signext %"g[0]::Int64"

That looks a bit odd, considering our indexing usually starts at 1.

@Seelengrab
Copy link
Contributor

Seelengrab commented Jul 11, 2023

i64 signext %"g[0]::Int64"

Another thought - since this is (presumably) always from slurping, we don't have to worry about offset arrays here, right?

@pchintalapudi
Copy link
Member Author

These indexes are fake anyways, by the time we're making up LLVM argument names for varargs they're already individual LLVM arguments. The index is just for pretty presentation rather than ...1, ...2, etc.

src/codegen.cpp Outdated Show resolved Hide resolved
src/codegen.cpp Outdated
f = cast<Function>(returninfo.decl.getCallee());
has_sret = (returninfo.cc == jl_returninfo_t::SRet || returninfo.cc == jl_returninfo_t::Union);
jl_init_function(f, ctx.emission_context.TargetTriple);
auto arg_typename = [&](size_t i) JL_NOTSAFEPOINT { return jl_symbol_name(((jl_datatype_t*)jl_tparam(lam->specTypes, i))->name->name); };
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Not safe to cast this to a datatype.

Copy link
Sponsor Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

What is the overhead of doing this?

@pchintalapudi
Copy link
Member Author

Probably not great for functions with large numbers of arguments, so it might be better to stick it behind -g1 and force debug info emission with code_llvm.

src/codegen.cpp Outdated
if (ctx.emission_context.debug_level > 0) {
auto arg_typename = [&](size_t i) JL_NOTSAFEPOINT {
auto tp = jl_tparam(lam->specTypes, i);
return jl_is_datatype(tp) ? jl_symbol_name(((jl_datatype_t*)tp)->name->name) : "<unknown datatype>";
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Suggested change
return jl_is_datatype(tp) ? jl_symbol_name(((jl_datatype_t*)tp)->name->name) : "<unknown datatype>";
return jl_is_datatype(tp) ? jl_symbol_name(((jl_datatype_t*)tp)->name->name) : "<unknown datatype>";

But it isn't a datatype, so why does it say "unknown datatype"

Copy link
Member Author

Choose a reason for hiding this comment

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

Can we mark it as unknown type or ::Any then?

@pchintalapudi pchintalapudi added this pull request to the merge queue Jul 15, 2023
Merged via the queue into master with commit 7735556 Jul 15, 2023
1 check passed
@pchintalapudi pchintalapudi deleted the pc/arg-names branch July 15, 2023 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:llvm For issues that relate to LLVM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants