Re: [PATCH v10 1/2] Add condition coverage (MC/DC)

["Jørgen Kvalsvik" <j@lambda.is>]

On 15/04/2024 10:53, Rainer Orth wrote:
> Hi Jørgen,
> 
>> On 04/04/2024 14:10, Jan Hubicka wrote:
>>>> gcc/ChangeLog:
>>>>
>>>> 	* builtins.cc (expand_builtin_fork_or_exec): Check
>>>> 	  condition_coverage_flag.
>>>> 	* collect2.cc (main): Add -fno-condition-coverage to OBSTACK.
>>>> 	* common.opt: Add new options -fcondition-coverage and
>>>> 	  -Wcoverage-too-many-conditions.
>>>> 	* doc/gcov.texi: Add --conditions documentation.
>>>> 	* doc/invoke.texi: Add -fcondition-coverage documentation.
>>>> 	* function.cc (free_after_compilation): Free cond_uids.
>>>> 	* function.h (struct function): Add cond_uids.
>>>> 	* gcc.cc: Link gcov on -fcondition-coverage.
>>>> 	* gcov-counter.def (GCOV_COUNTER_CONDS): New.
>>>> 	* gcov-dump.cc (tag_conditions): New.
>>>> 	* gcov-io.h (GCOV_TAG_CONDS): New.
>>>> 	(GCOV_TAG_CONDS_LENGTH): New.
>>>> 	(GCOV_TAG_CONDS_NUM): New.
>>>> 	* gcov.cc (class condition_info): New.
>>>> 	(condition_info::condition_info): New.
>>>> 	(condition_info::popcount): New.
>>>> 	(struct coverage_info): New.
>>>> 	(add_condition_counts): New.
>>>> 	(output_conditions): New.
>>>> 	(print_usage): Add -g, --conditions.
>>>> 	(process_args): Likewise.
>>>> 	(output_intermediate_json_line): Output conditions.
>>>> 	(read_graph_file): Read condition counters.
>>>> 	(read_count_file): Likewise.
>>>> 	(file_summary): Print conditions.
>>>> 	(accumulate_line_info): Accumulate conditions.
>>>> 	(output_line_details): Print conditions.
>>>> 	* gimplify.cc (next_cond_uid): New.
>>>> 	(reset_cond_uid): New.
>>>> 	(shortcut_cond_r): Set condition discriminator.
>>>> 	(tag_shortcut_cond): New.
>>>> 	(gimple_associate_condition_with_expr): New.
>>>> 	(shortcut_cond_expr): Set condition discriminator.
>>>> 	(gimplify_cond_expr): Likewise.
>>>> 	(gimplify_function_tree): Call reset_cond_uid.
>>>> 	* ipa-inline.cc (can_early_inline_edge_p): Check
>>>> 	  condition_coverage_flag.
>>>> 	* ipa-split.cc (pass_split_functions::gate): Likewise.
>>>> 	* passes.cc (finish_optimization_passes): Likewise.
>>>> 	* profile.cc (struct condcov): New declaration.
>>>> 	(cov_length): Likewise.
>>>> 	(cov_blocks): Likewise.
>>>> 	(cov_masks): Likewise.
>>>> 	(cov_maps): Likewise.
>>>> 	(cov_free): Likewise.
>>>> 	(instrument_decisions): New.
>>>> 	(read_thunk_profile): Control output to file.
>>>> 	(branch_prob): Call find_conditions, instrument_decisions.
>>>> 	(init_branch_prob): Add total_num_conds.
>>>> 	(end_branch_prob): Likewise.
>>>> 	* tree-core.h (struct tree_exp): Add condition_uid.
>>>> 	* tree-profile.cc (struct conds_ctx): New.
>>>> 	(CONDITIONS_MAX_TERMS): New.
>>>> 	(EDGE_CONDITION): New.
>>>> 	(topological_cmp): New.
>>>> 	(index_of): New.
>>>> 	(single_p): New.
>>>> 	(single_edge): New.
>>>> 	(contract_edge_up): New.
>>>> 	(struct outcomes): New.
>>>> 	(conditional_succs): New.
>>>> 	(condition_index): New.
>>>> 	(condition_uid): New.
>>>> 	(masking_vectors): New.
>>>> 	(emit_assign): New.
>>>> 	(emit_bitwise_op): New.
>>>> 	(make_top_index_visit): New.
>>>> 	(make_top_index): New.
>>>> 	(paths_between): New.
>>>> 	(struct condcov): New.
>>>> 	(cov_length): New.
>>>> 	(cov_blocks): New.
>>>> 	(cov_masks): New.
>>>> 	(cov_maps): New.
>>>> 	(cov_free): New.
>>>> 	(find_conditions): New.
>>>> 	(struct counters): New.
>>>> 	(find_counters): New.
>>>> 	(resolve_counter): New.
>>>> 	(resolve_counters): New.
>>>> 	(instrument_decisions): New.
>>>> 	(tree_profiling): Check condition_coverage_flag.
>>>> 	(pass_ipa_tree_profile::gate): Likewise.
>>>> 	* tree.h (SET_EXPR_UID): New.
>>>> 	(EXPR_COND_UID): New.
>>>>
>>>> libgcc/ChangeLog:
>>>>
>>>> 	* libgcov-merge.c (__gcov_merge_ior): New.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 	* lib/gcov.exp: Add condition coverage test function.
>>>> 	* g++.dg/gcov/gcov-18.C: New test.
>>>> 	* gcc.misc-tests/gcov-19.c: New test.
>>>> 	* gcc.misc-tests/gcov-20.c: New test.
>>>> 	* gcc.misc-tests/gcov-21.c: New test.
>>>> 	* gcc.misc-tests/gcov-22.c: New test.
>>>> 	* gcc.misc-tests/gcov-23.c: New test.
>>>> ---
>>>>    gcc/builtins.cc                        |    2 +-
>>>>    gcc/collect2.cc                        |    7 +-
>>>>    gcc/common.opt                         |    9 +
>>>>    gcc/doc/gcov.texi                      |   38 +
>>>>    gcc/doc/invoke.texi                    |   21 +
>>>>    gcc/function.cc                        |    1 +
>>>>    gcc/function.h                         |    4 +
>>>>    gcc/gcc.cc                             |    4 +-
>>>>    gcc/gcov-counter.def                   |    3 +
>>>>    gcc/gcov-dump.cc                       |   24 +
>>>>    gcc/gcov-io.h                          |    3 +
>>>>    gcc/gcov.cc                            |  209 ++-
>>>>    gcc/gimplify.cc                        |  123 +-
>>>>    gcc/ipa-inline.cc                      |    2 +-
>>>>    gcc/ipa-split.cc                       |    2 +-
>>>>    gcc/passes.cc                          |    3 +-
>>>>    gcc/profile.cc                         |   76 +-
>>>>    gcc/testsuite/g++.dg/gcov/gcov-18.C    |  282 ++++
>>>>    gcc/testsuite/gcc.misc-tests/gcov-19.c | 1737 ++++++++++++++++++++++++
>>>>    gcc/testsuite/gcc.misc-tests/gcov-20.c |   22 +
>>>>    gcc/testsuite/gcc.misc-tests/gcov-21.c |   16 +
>>>>    gcc/testsuite/gcc.misc-tests/gcov-22.c |  103 ++
>>>>    gcc/testsuite/gcc.misc-tests/gcov-23.c |  361 +++++
>>>>    gcc/testsuite/lib/gcov.exp             |  259 +++-
>>>>    gcc/tree-core.h                        |    3 +
>>>>    gcc/tree-profile.cc                    | 1058 ++++++++++++++-
>>>>    gcc/tree.h                             |    4 +
>>>>    libgcc/libgcov-merge.c                 |    5 +
>>>>    28 files changed, 4339 insertions(+), 42 deletions(-)
>>>>    create mode 100644 gcc/testsuite/g++.dg/gcov/gcov-18.C
>>>>    create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-19.c
>>>>    create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-20.c
>>>>    create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-21.c
>>>>    create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-22.c
>>>>    create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-23.c
>>>>
>>>> ---
>>>> Changes since v9:
>>>>
>>>> * function->cond_uid is no longer in ggc memory
>>>> * function->cond_uid and condition annotation is only done when coverage
>>>>     is requested
>>>> * A few new test cases, notably interactions with C++ constexpr
>>>> * Updated comments based on review feedback
>>>> ---
>>>>
>>>
>>>> diff --git a/gcc/ipa-inline.cc b/gcc/ipa-inline.cc
>>>> index dc120e6da5a..9502a21c741 100644
>>>> --- a/gcc/ipa-inline.cc
>>>> +++ b/gcc/ipa-inline.cc
>>>> @@ -682,7 +682,7 @@ can_early_inline_edge_p (struct cgraph_edge *e)
>>>>        }
>>>>      gcc_assert (gimple_in_ssa_p (DECL_STRUCT_FUNCTION (e->caller->decl))
>>>>    	      && gimple_in_ssa_p (DECL_STRUCT_FUNCTION (callee->decl)));
>>>> -  if (profile_arc_flag
>>>> +  if ((profile_arc_flag || condition_coverage_flag)
>>>>          && ((lookup_attribute ("no_profile_instrument_function",
>>>>    			    DECL_ATTRIBUTES (caller->decl)) == NULL_TREE)
>>>>    	  != (lookup_attribute ("no_profile_instrument_function",
>>> tree-inline should also copy the cond tags, since always_inline
>>> functions are inlined even at -O0.  But I think this can be done
>>> incrementally.
>>> Patch is OK.
>>> Thanks a lot and sorry for taking so long on this one.
>>> Honza
>>
>> I guess you mean that since tree-inlining happen after gimplification (does
>> it?) that the basic condition -> condition expression association should
>> also fold in the now-inlined conditions?
>>
>> Thanks for the review - I'll do a final test run and install the patch.
> 
> the new gcc.misc-tests/gcov-22.c test loops on SPARC (both Solaris and
> Linux).  I've filed PR gcov-profile/114720 for this, but couldn't find
> any bugzilla account of yours to Cc:
> 
> 	Rainer
> 
> 

Thanks, I'll look into it right away.