timholy / SnoopCompile.jl

Provide insights about latency (TTFX) for Julia packages
https://timholy.github.io/SnoopCompile.jl/dev/
Other
309 stars 48 forks source link

LLVM snooping broken on nightly #270

Closed timholy closed 2 years ago

timholy commented 2 years ago

On nightly,

julia> ccall(:jl_dump_emitted_mi_name, Nothing, (Ptr{Nothing},), C_NULL)
ERROR: could not load symbol "jl_dump_emitted_mi_name":
/home/tim/src/julia-master/julia: undefined symbol: jl_dump_emitted_mi_name
Stacktrace:
 [1] top-level scope
   @ ./REPL[2]:1

I notice this ccall is not tested anywhere in Julia's tests; that would be a good idea, to ensure it remains callable.

I don't use the LLVM functionality so I'm assigning this to @NHDaly (feel free to reassign to someone else).

NHDaly commented 2 years ago

Thanks @timholy - great catch! Oops, sorry it wasn't tested... :/

It looks like maybe something changed in the julia build where you now need to put JL_DLLEXPORT in a Header file in order to get the function exported...? Currently, it's only exported from this .cpp file: https://github.com/JuliaLang/julia/blob/602b46c55387c527192d2db0d9a71a118678b816/src/codegen.cpp#L105-L109

Ah, I think this was the change, which seems to now (sometimes?) require you to list your exported function name in this .inc file: https://github.com/JuliaLang/julia/pull/38160 It looks like the exported functions we use elsewhere in the package got pulled in (probably because they were tested 😜): https://github.com/JuliaLang/julia/blob/6735cb62858d22a532d17bc810378dee3dc2ea8d/src/jl_exported_funcs.inc

So i think i'll need to add this function to that list. Thanks @timholy! 👍

timholy commented 2 years ago

Thanks so much for fixing this!

Fixed by https://github.com/JuliaLang/julia/pull/43634