gf.c: include const-return methods in --trace-compile
#237
Conversation
kpamnany
force-pushed
the
kp-backport-trace-const-return-methods
branch
from
4533648 to
ba3cfb0
Compare
kpamnany
force-pushed
the
kp-backport-trace-const-return-methods
branch
from
ba3cfb0 to
fa0f780
Compare
src/gf.c
Outdated
| @@ -2420,7 +2426,10 @@ static void record_precompile_statement(jl_method_instance_t *mi, double compila | |||
| jl_printf(s_precompile, "#= %6.1f =# ", compilation_time / 1e6); | |||
| jl_printf(s_precompile, "precompile("); | |||
| jl_static_show(s_precompile, mi->specTypes); | |||
| jl_printf(s_precompile, ")\n"); | |||
| if (is_const_return_abi) | |||
| jl_printf(s_precompile, ") #= const-return =#\n"); | |||
There was a problem hiding this comment.
is this going to negatively interact with any trace-processing tools we have?
why do we want this in the output?
There was a problem hiding this comment.
Great observation. I think I did see some CI errors due to this comment when I had added the TPC-H trace (before I removed that trace from testing). Those should be easily fixable; I'll check.
Upstream, they are (for now) maintaining the idea that compilation traces are human-readable, which is why this is in the output. We could remove it locally if we're okay with diverging.
There was a problem hiding this comment.
compilation traces are human-readable, which is why this is in the output
i wonder in what situations seeing the #= const-return =# comment is helpful?
There was a problem hiding this comment.
I think we should be able to handle these trailing comments, but it's definitely a good idea to check 👍
There was a problem hiding this comment.
julia> RAI_CompilationsCache.PrecompileStatementParsing.is_precompileable(Base, "#= 1.0 =# precompile(Tuple{typeof(Base.:+), Int, Int}) #= const-return =#")
pr_success::PrecompilationResult = 1
julia> RAI_CompilationsCache.PrecompileStatementParsing.run_precompile(Base, "#= 1.0 =# precompile(Tuple{typeof(Base.:+), Int, Int}) #= const-return =#")
pr_success::PrecompilationResult = 1We could end up with duplicate traces though -- if on engine 1 we produced
#= 1.0 =# precompile(Tuple{typeof(Base.:+), Int, Int}) #= const-return =#and on engine 2, which called the function in a different context, we'd get
#= 1.0 =# precompile(Tuple{typeof(Base.:+), Int, Int})That could cause the precompile to take up 2 spots in the cache, so we probably need to augment the cache to strip any trailing comments.
These are never compiled by LLVM, but we want to log them since they are inferred / compiled by our own compiler.
kpamnany
force-pushed
the
kp-backport-trace-const-return-methods
branch
from
fa0f780 to
9202e9c
Compare
PR Description
This is a quick-fix for an issue where methods that are inferred to return a
Constdo not emit compilation traces. Although they are never compiled by LLVM, we want to log them since they are inferred / compiled by Julia. As with #236, it isn't yet clear what the actual upstream fix will look like on 1.12+.Checklist
Requirements for merging:
--trace-compileignores inferred const-return Methods JuliaLang/julia#58482port-to-*labels that don't apply.