Merge pull request #18196 from JuliaLang/yyc/codegen/coverage

RFT(est): Accurate coverage with inlining on

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

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]

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`)

src/codegen.cpp

@@ -4041,12 +4041,8 @@ static std::unique_ptr<Module> 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 = "<missing>";
  StringRef dbgFuncName = ctx.name;
  int toplineno = -1;
@@ -4076,8 +4072,8 @@ static std::unique_ptr<Module> 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<Module> 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<Module> 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<StmtProp> stmtprops(stmtslen);
  std::vector<DbgState> DI_stack;
@@ -4520,7 +4522,12 @@ static std::unique_ptr<Module> 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<Module> 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<Module> 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<Module> 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,22 +4668,29 @@ static std::unique_ptr<Module> emit_function(jl_lambda_info_t *lam, jl_llvm_func
  // step 12. Do codegen in control flow order
  std::vector<std::pair<int,BasicBlock*>> 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;
  }
  if (!builder.GetInsertBlock()->getTerminator())
  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<Module> 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<Module> 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<Module> 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<Module> 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);
  }
  }