[llvm-cov] Fix branch counts of template functions (second attempt) #135074
Conversation
|
|
|
Ping |
|
Hi! SOrry for the delay. Can you explain how you get around the non-deterministic behavior? The changes seem OK to me but I want to make sure I'm not overlooking the actual difference. |
stma247
force-pushed
the
users/stma247/llvm_cov_export_with_unified_function_instantiations2
branch
from
e334044 to
efbc47a
Compare
|
The key is to add an additional field "NestedCountedRegion::Position" which is used by function "sortNested" when ambiguous entries are contained in collections such as "Branches". Such entries may appear whenever a template function with more than one instantiation occurs (then all fields of NestedCountedRegion and it's base classes contain equal values). The field "Position" makes every entry unique and the shuffle of "llvm::sort" has no bad impact on the result. This problem existed even without my changes, but the new test "branch-export-lcov-unify-instances.test" brought it to light as it does more pedantic checks than the existing template tests. |
|
Thank you! Looks good to me. Let me know if you need assistance merging again. |
Added option "--unify-instantiations" to llvm-cov export to combine branch execution counts of C++ template instantiations. on-behalf-of: @e-solutions-GmbH <info@esolutions.de>
Fixed flaky test if EXPENSIVE_CHECKS are enabled, caused by an ambiguous comparator. Keep orginal position in NestedCountedRegion to sort distinct. on-behalf-of: @e-solutions-GmbH <info@esolutions.de>
stma247
force-pushed
the
users/stma247/llvm_cov_export_with_unified_function_instantiations2
branch
from
169d8fb to
82382d3
Compare
|
Yes please, I haven't requested write access so far. |
…lvm#135074) This PR is a second attempt for issue llvm#111743 to finish reverted PR llvm#113925. Added option "--unify-instantiations" to llvm-cov export to combine branch execution counts of C++ template instantiations. Fix non-deterministic behavior.
…lvm#135074) This PR is a second attempt for issue llvm#111743 to finish reverted PR llvm#113925. Added option "--unify-instantiations" to llvm-cov export to combine branch execution counts of C++ template instantiations. Fix non-deterministic behavior.
…lvm#135074) This PR is a second attempt for issue llvm#111743 to finish reverted PR llvm#113925. Added option "--unify-instantiations" to llvm-cov export to combine branch execution counts of C++ template instantiations. Fix non-deterministic behavior.
…lvm#135074) This PR is a second attempt for issue llvm#111743 to finish reverted PR llvm#113925. Added option "--unify-instantiations" to llvm-cov export to combine branch execution counts of C++ template instantiations. Fix non-deterministic behavior.
This PR is a second attempt for issue #111743 to finish reverted PR #113925.
Test run with EXPENSIVE_CHECKS showed a flaky test. It is related to "llvm::sort" that applies some kind of shuffle if expensive checks are on (see https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ADT/STLExtras.h#L1676).
The reason why this issue came up with my initial PR, is that no test for this template ambiguity existed before (see new llvm/test/tools/llvm-cov/branch-export-lcov-unify-instances.test). I can see this indeterministic behavior even on branch "main", if I add test code from "branch-export-lcov-unify-instances.test".