diff --git a/Make.inc b/Make.inc index 23ccbb3539ebb..3b0b2748d1a03 100644 --- a/Make.inc +++ b/Make.inc @@ -385,7 +385,7 @@ JCFLAGS := -std=gnu99 -pipe $(fPIC) -fno-strict-aliasing -D_FILE_OFFSET_BITS=64 JCPPFLAGS := -fasynchronous-unwind-tables JCXXFLAGS := -pipe $(fPIC) -fno-rtti ifneq ($(OS), WINNT) -# Do no enable on windows to avoid warnings from libuv. +# Do not enable on windows to avoid warnings from libuv. JCXXFLAGS += -pedantic endif DEBUGFLAGS := -O0 -ggdb2 -DJL_DEBUG_BUILD -fstack-protector-all diff --git a/base/inference.jl b/base/inference.jl index 73066fa593f51..0e018ec791df4 100644 --- a/base/inference.jl +++ b/base/inference.jl @@ -1908,8 +1908,9 @@ function finish(me::InferenceState) end type_annotate!(me.linfo, me.stmt_types, me, me.nargs) - # run optimization passes on fulltree + do_coverage = coverage_enabled() force_noinline = false + # run optimization passes on fulltree if me.optimize # This pass is required for the AST to be valid in codegen # if any `SSAValue` is created by type inference. Ref issue #6068 @@ -1923,7 +1924,7 @@ function finish(me::InferenceState) getfield_elim_pass!(me.linfo, me) # Clean up for `alloc_elim_pass!` and `getfield_elim_pass!` void_use_elim_pass!(me.linfo, me) - meta_elim_pass!(me.linfo, me.linfo.code::Array{Any,1}) + meta_elim_pass!(me.linfo, me.linfo.code::Array{Any,1}, do_coverage) # Pop metadata before label reindexing force_noinline = popmeta!(me.linfo.code::Array{Any,1}, :noinline)[1] reindex_labels!(me.linfo, me) @@ -1933,8 +1934,11 @@ function finish(me::InferenceState) ispure = me.linfo.pure ccall(:jl_set_lambda_rettype, Void, (Any, Any), me.linfo, widenconst(me.bestguess)) - if (isa(me.bestguess,Const) && me.bestguess.val !== nothing) || - (isType(me.bestguess) && !has_typevars(me.bestguess.parameters[1],true)) + # Do not emit `jlcall_api == 2` if coverage is enabled so that we don't + # need to add coverage support to the `jl_call_method_internal` fast path + if !do_coverage && + ((isa(me.bestguess,Const) && me.bestguess.val !== nothing) || + (isType(me.bestguess) && !has_typevars(me.bestguess.parameters[1],true))) if !ispure && length(me.linfo.code) < 10 ispure = true for stmt in me.linfo.code @@ -2724,18 +2728,48 @@ function inlineable(f::ANY, ft::ANY, e::Expr, atypes::Vector{Any}, sv::Inference end end - if !isempty(stmts) - if all(stmt -> (isa(stmt, Expr) && is_meta_expr(stmt::Expr)) || isa(stmt, LineNumberNode) || stmt === nothing, - stmts) + do_coverage = coverage_enabled() + inlining_ignore = function (stmt::ANY) + isa(stmt, Expr) && return is_meta_expr(stmt::Expr) + isa(stmt, LineNumberNode) && return true + stmt === nothing && return true + return false + end + if do_coverage + line = if !isempty(stmts) && isa(stmts[1], LineNumberNode) + (shift!(stmts)::LineNumberNode).line + else + linfo.def.line + end + # Check if we are switching module, which is necessary to catch user + # code inlined into `Base` with `--code-coverage=user`. + # Assume we are inlining directly into `enclosing` instead of another + # function inlined in it + mod = linfo.def.module + if mod === sv.mod + unshift!(stmts, Expr(:meta, :push_loc, linfo.def.file, + linfo.def.name, line)) + else + unshift!(stmts, Expr(:meta, :push_loc, linfo.def.file, + linfo.def.name, line, mod)) + end + push!(stmts, Expr(:meta, :pop_loc)) + elseif !isempty(stmts) + if all(inlining_ignore, stmts) empty!(stmts) else - local line::Int = linfo.def.line - if isa(stmts[1], LineNumberNode) - line = shift!(stmts).line + line = if isa(stmts[1], LineNumberNode) + (shift!(stmts)::LineNumberNode).line + else + linfo.def.line + end + unshift!(stmts, Expr(:meta, :push_loc, linfo.def.file, + linfo.def.name, line)) + if isa(stmts[end], LineNumberNode) + stmts[end] = Expr(:meta, :pop_loc) + else + push!(stmts, Expr(:meta, :pop_loc)) end - unshift!(stmts, Expr(:meta, :push_loc, linfo.def.file, linfo.def.name, line)) - isa(stmts[end], LineNumberNode) && pop!(stmts) - push!(stmts, Expr(:meta, :pop_loc)) end end if !isempty(stmts) && !propagate_inbounds @@ -3227,7 +3261,7 @@ function void_use_elim_pass!(linfo::LambdaInfo, sv) return end -function meta_elim_pass!(linfo::LambdaInfo, code::Array{Any,1}) +function meta_elim_pass!(linfo::LambdaInfo, code::Array{Any,1}, do_coverage) # 1. Remove place holders # # 2. If coverage is off, remove line number nodes that don't mark any @@ -3267,7 +3301,6 @@ function meta_elim_pass!(linfo::LambdaInfo, code::Array{Any,1}) # `Expr(:boundscheck)` (e.g. when they don't enclose any non-meta # expressions). Those are a little harder to detect and are hopefully # not too common. - do_coverage = coverage_enabled() check_bounds = JLOptions().check_bounds inbounds_stack = linfo.propagate_inbounds ? Bool[] : [false] diff --git a/base/pkg/entry.jl b/base/pkg/entry.jl index 48d94c8113101..9e92968617589 100644 --- a/base/pkg/entry.jl +++ b/base/pkg/entry.jl @@ -712,7 +712,7 @@ function test!(pkg::AbstractString, cd(dirname(test_path)) do try color = Base.have_color? "--color=yes" : "--color=no" - codecov = coverage? ["--code-coverage=user", "--inline=no"] : ["--code-coverage=none"] + codecov = coverage? ["--code-coverage=user"] : ["--code-coverage=none"] compilecache = "--compilecache=" * (Bool(Base.JLOptions().use_compilecache) ? "yes" : "no") julia_exe = Base.julia_cmd() run(`$julia_exe --check-bounds=yes $codecov $color $compilecache $test_path`) diff --git a/src/codegen.cpp b/src/codegen.cpp index 1021838164acd..94a6a56ba55a4 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -4041,12 +4041,8 @@ static std::unique_ptr emit_function(jl_lambda_info_t *lam, jl_llvm_func ctx.f = f; // step 5. set up debug info context and create first basic block - bool in_user_code = !jl_is_submodule(ctx.module, jl_base_module) && - !jl_is_submodule(ctx.module, jl_core_module); - bool do_coverage = jl_options.code_coverage == JL_LOG_ALL || - (jl_options.code_coverage == JL_LOG_USER && in_user_code); - bool do_malloc_log = jl_options.malloc_log == JL_LOG_ALL || - (jl_options.malloc_log == JL_LOG_USER && in_user_code); + int coverage_mode = jl_options.code_coverage; + int malloc_log_mode = jl_options.malloc_log; StringRef filename = ""; StringRef dbgFuncName = ctx.name; int toplineno = -1; @@ -4076,8 +4072,8 @@ static std::unique_ptr emit_function(jl_lambda_info_t *lam, jl_llvm_func ctx.debug_enabled = true; if (dbgFuncName.empty()) { // special value: if function name is empty, disable debug info - do_coverage = false; - do_malloc_log = false; + coverage_mode = JL_LOG_NONE; + malloc_log_mode = JL_LOG_NONE; //dbgFuncName = filename; // for testing, uncomment this line ctx.debug_enabled = !dbgFuncName.empty(); } @@ -4486,12 +4482,17 @@ static std::unique_ptr emit_function(jl_lambda_info_t *lam, jl_llvm_func // step 11. Compute properties for each statements // This needs to be computed by iterating in the IR order // instead of control flow order. + auto in_user_mod = [] (jl_module_t *mod) { + return (!jl_is_submodule(mod, jl_base_module) && + !jl_is_submodule(mod, jl_core_module)); + }; #ifdef LLVM37 struct DbgState { DebugLoc loc; DISubprogram *sp; StringRef file; ssize_t line; + bool in_user_code; }; #else struct DbgState { @@ -4508,6 +4509,7 @@ static std::unique_ptr emit_function(jl_lambda_info_t *lam, jl_llvm_func bool is_inbounds; bool loc_changed; bool is_poploc; + bool in_user_code; }; std::vector stmtprops(stmtslen); std::vector DI_stack; @@ -4520,7 +4522,12 @@ static std::unique_ptr emit_function(jl_lambda_info_t *lam, jl_llvm_func inbounds |= inbounds_stack[sz - 2]; return inbounds; }; - StmtProp cur_prop{topdebugloc, filename, toplineno, false, true, false}; + StmtProp cur_prop{topdebugloc, filename, toplineno, + false, true, false, false}; + if (coverage_mode != JL_LOG_NONE || malloc_log_mode) { + cur_prop.in_user_code = (!jl_is_submodule(ctx.module, jl_base_module) && + !jl_is_submodule(ctx.module, jl_core_module)); + } for (i = 0; i < stmtslen; i++) { cur_prop.loc_changed = false; cur_prop.is_poploc = false; @@ -4574,7 +4581,8 @@ static std::unique_ptr emit_function(jl_lambda_info_t *lam, jl_llvm_func if (ctx.debug_enabled) new_file = dbuilder.createFile(new_filename, "."); DI_stack.push_back({cur_prop.loc, SP, - cur_prop.file, cur_prop.line}); + cur_prop.file, cur_prop.line, + cur_prop.in_user_code}); const char *inl_name = ""; int inlined_func_lineno = 0; if (jl_array_len(expr->args) > 2) { @@ -4586,6 +4594,10 @@ static std::unique_ptr emit_function(jl_lambda_info_t *lam, jl_llvm_func inlined_func_lineno = jl_unbox_int32(arg); else if (jl_is_int64(arg)) inlined_func_lineno = jl_unbox_int64(arg); + else if (jl_is_module(arg)) { + jl_module_t *mod = (jl_module_t*)arg; + cur_prop.in_user_code = in_user_mod(mod); + } } } else { @@ -4624,6 +4636,7 @@ static std::unique_ptr emit_function(jl_lambda_info_t *lam, jl_llvm_func cur_prop.loc = DI.loc; cur_prop.file = DI.file; cur_prop.line = DI.line; + cur_prop.in_user_code = DI.in_user_code; DI_stack.pop_back(); cur_prop.loc_changed = true; } @@ -4655,11 +4668,16 @@ static std::unique_ptr emit_function(jl_lambda_info_t *lam, jl_llvm_func // step 12. Do codegen in control flow order std::vector> workstack; int cursor = 0; + // Whether we are doing codegen in statement order. + // We need to update debug location if this is false even if + // `loc_changed` is false. + bool linear_codegen = false; auto find_next_stmt = [&] (int seq_next) { // `seq_next` is the next statement we want to emit // i.e. if it exists, it's the next one following control flow and // should be emitted into the current insert point. if (seq_next >= 0 && (unsigned)seq_next < stmtslen) { + linear_codegen = (seq_next - cursor) == 1; cursor = seq_next; return; } @@ -4667,10 +4685,12 @@ static std::unique_ptr emit_function(jl_lambda_info_t *lam, jl_llvm_func builder.CreateUnreachable(); if (workstack.empty()) { cursor = -1; + linear_codegen = false; return; } auto &item = workstack.back(); builder.SetInsertPoint(item.second); + linear_codegen = (item.first - cursor) == 1; cursor = item.first; workstack.pop_back(); }; @@ -4721,24 +4741,35 @@ static std::unique_ptr emit_function(jl_lambda_info_t *lam, jl_llvm_func return bb; }; + auto do_coverage = [&] (bool in_user_code) { + return (coverage_mode == JL_LOG_ALL || + (coverage_mode == JL_LOG_USER && in_user_code)); + }; + auto do_malloc_log = [&] (bool in_user_code) { + return (malloc_log_mode == JL_LOG_ALL || + (malloc_log_mode == JL_LOG_USER && in_user_code)); + }; + // If the first expresion changes the line number, we need to visit // the start of the function. This can happen when the first line is // a inlined function call. - if (stmtprops[0].loc_changed && do_coverage) { + if (stmtprops[0].loc_changed && coverage_mode != JL_LOG_NONE && + do_coverage(in_user_mod(ctx.module))) { + // Compute `in_user_code` using `ctx.module` instead of using + // `stmtprops[0].in_user_code` since the code property is for the first + // statement, which might have been a push_loc. if (ctx.debug_enabled) builder.SetCurrentDebugLocation(topdebugloc); coverageVisitLine(filename, toplineno); } - stmtprops[0].loc_changed = true; while (cursor != -1) { auto &props = stmtprops[cursor]; - if (props.loc_changed) { - if (ctx.debug_enabled) - builder.SetCurrentDebugLocation(props.loc); - // Disable coverage for pop_loc, it doesn't start a new expression - if (do_coverage && !props.is_poploc) { - coverageVisitLine(props.file, props.line); - } + if ((props.loc_changed || !linear_codegen) && ctx.debug_enabled) + builder.SetCurrentDebugLocation(props.loc); + // Disable coverage for pop_loc, it doesn't start a new expression + if (props.loc_changed && do_coverage(props.in_user_code) && + !props.is_poploc) { + coverageVisitLine(props.file, props.line); } ctx.is_inbounds = props.is_inbounds; jl_value_t *stmt = jl_array_ptr_ref(stmts, cursor); @@ -4772,7 +4803,7 @@ static std::unique_ptr emit_function(jl_lambda_info_t *lam, jl_llvm_func else { // undef return type retval = NULL; } - if (do_malloc_log && props.line != -1) + if (do_malloc_log(props.in_user_code) && props.line != -1) mallocVisitLine(props.file, props.line); if (type_is_ghost(retty) || ctx.sret) builder.CreateRetVoid(); @@ -4791,7 +4822,7 @@ static std::unique_ptr emit_function(jl_lambda_info_t *lam, jl_llvm_func jl_value_t *cond = args[0]; int lname = jl_unbox_long(args[1]); Value *isfalse = emit_condition(cond, "if", &ctx); - if (do_malloc_log && props.line != -1) + if (do_malloc_log(props.in_user_code) && props.line != -1) mallocVisitLine(props.file, props.line); BasicBlock *ifso = BasicBlock::Create(jl_LLVMContext, "if", f); BasicBlock *ifnot = handle_label(lname, false); @@ -4833,7 +4864,7 @@ static std::unique_ptr emit_function(jl_lambda_info_t *lam, jl_llvm_func } else { emit_stmtpos(stmt, &ctx); - if (do_malloc_log && props.line != -1) { + if (do_malloc_log(props.in_user_code) && props.line != -1) { mallocVisitLine(props.file, props.line); } }