| Message ID | 20231004123921.634024-19-j@lambda.is |
|---|---|
| State | New |
| Headers | show |
| Series | [01/22] Add condition coverage profiling | expand |
On 04/10/2023 21:39, Jørgen Kvalsvik wrote: > A check was missing for is-single when contracting, so if a > multi-successor node that was not a condition node (e.g. a switch) a > random edge would be picked and contracted into. > --- > gcc/testsuite/gcc.misc-tests/gcov-23.c | 48 ++++++++++++++++++++++++++ > gcc/tree-profile.cc | 4 +++ > 2 files changed, 52 insertions(+) > > diff --git a/gcc/testsuite/gcc.misc-tests/gcov-23.c b/gcc/testsuite/gcc.misc-tests/gcov-23.c > index c4afc5e700d..856e97f5088 100644 > --- a/gcc/testsuite/gcc.misc-tests/gcov-23.c > +++ b/gcc/testsuite/gcc.misc-tests/gcov-23.c > @@ -3,6 +3,7 @@ > int id (int); > int idp (int *); > int err; > +char c; > > /* This becomes problematic only under optimization for the case when the > compiler cannot inline the function but have to generate a call. It is not > @@ -72,4 +73,51 @@ exit: > return a; > } > > +/* Adapted from icu4c-73.1 common/ucase.cpp ucase_getLocaleCase (). */ > +int > +mcdc003 (const char *locale) > +{ > + /* extern, so its effect won't be optimized out. */ > + c = *locale++; > + if (c == 'z') /* conditions(0/2) true(0) false(0) */ > + /* conditions(end) */ > + { > + return 1; > + } > + else if (c >= 'a') /* conditions(0/2) true(0) false(0) */ > + /* conditions(end) */ > + { > + if (id (c)) /* conditions(0/2) true(0) false(0) */ > + /* conditions(end) */ > + c = *locale++; > + } > + else > + { > + if (c == 'T') > + { > + if (id (c)) /* conditions(0/2) true(0) false(0) */ > + /* conditions(end) */ > + c = *locale++; > + if (id (c)) /* conditions(0/2) true(0) false(0) */ > + /* conditions(end) */ > + c = *locale++; > + } > + else if (c == 'L') > + c = *locale++; > + else if (c == 'E') > + c = *locale++; > + else if (c == 'N') > + c = *locale++; > + else if (c == 'H') > + { > + c = *locale++; > + if (id (c)) /* conditions(0/2) true(0) false(0) */ > + /* conditions(end) */ > + c = *locale++; > + } > + } > + > + return 1; > +} > + > /* { dg-final { run-gcov conditions { --conditions gcov-23.c } } } */ > diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc > index 26e1924d444..ce679130ca0 100644 > --- a/gcc/tree-profile.cc > +++ b/gcc/tree-profile.cc > @@ -297,6 +297,10 @@ contract_edge (edge e, sbitmap G = nullptr) > return source; > if (block_conditional_p (dest)) > return e; > + /* This happens for switches, and must be checked after the is-conditional > + (which is also not single). */ > + if (!single (dest->succs)) > + return source; > > if (G) > bitmap_set_bit (G, dest->index); This test seems to fail on aarch64 (thanks, linaro bot) FAIL: 24 regressions regressions.sum: === gcc tests === Running gcc:gcc.misc-tests/gcov.exp ... FAIL: gcc.misc-tests/gcov-23.c gcov: 0 failures in line counts, 0 in branch percentages, 13 in condition/decision, 0 in return percentages, 0 in intermediate format FAIL: gcc.misc-tests/gcov-23.c line 108: unexpected summary 0/2 FAIL: gcc.misc-tests/gcov-23.c line 108: unexpected uncovered term 0 (false) FAIL: gcc.misc-tests/gcov-23.c line 108: unexpected uncovered term 0 (true) FAIL: gcc.misc-tests/gcov-23.c line 110: unexpected summary 0/2 FAIL: gcc.misc-tests/gcov-23.c line 110: unexpected uncovered term 0 (false) FAIL: gcc.misc-tests/gcov-23.c line 110: unexpected uncovered term 0 (true) ... and 19 more entries Apparently the if-else do not get turned into a switch here. I have a look and see if I can reproduce the problem with an explicit switch rather than the implied one from if-else, as it is broken in its current state.
diff --git a/gcc/testsuite/gcc.misc-tests/gcov-23.c b/gcc/testsuite/gcc.misc-tests/gcov-23.c index c4afc5e700d..856e97f5088 100644 --- a/gcc/testsuite/gcc.misc-tests/gcov-23.c +++ b/gcc/testsuite/gcc.misc-tests/gcov-23.c @@ -3,6 +3,7 @@ int id (int); int idp (int *); int err; +char c; /* This becomes problematic only under optimization for the case when the compiler cannot inline the function but have to generate a call. It is not @@ -72,4 +73,51 @@ exit: return a; } +/* Adapted from icu4c-73.1 common/ucase.cpp ucase_getLocaleCase (). */ +int +mcdc003 (const char *locale) +{ + /* extern, so its effect won't be optimized out. */ + c = *locale++; + if (c == 'z') /* conditions(0/2) true(0) false(0) */ + /* conditions(end) */ + { + return 1; + } + else if (c >= 'a') /* conditions(0/2) true(0) false(0) */ + /* conditions(end) */ + { + if (id (c)) /* conditions(0/2) true(0) false(0) */ + /* conditions(end) */ + c = *locale++; + } + else + { + if (c == 'T') + { + if (id (c)) /* conditions(0/2) true(0) false(0) */ + /* conditions(end) */ + c = *locale++; + if (id (c)) /* conditions(0/2) true(0) false(0) */ + /* conditions(end) */ + c = *locale++; + } + else if (c == 'L') + c = *locale++; + else if (c == 'E') + c = *locale++; + else if (c == 'N') + c = *locale++; + else if (c == 'H') + { + c = *locale++; + if (id (c)) /* conditions(0/2) true(0) false(0) */ + /* conditions(end) */ + c = *locale++; + } + } + + return 1; +} + /* { dg-final { run-gcov conditions { --conditions gcov-23.c } } } */ diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc index 26e1924d444..ce679130ca0 100644 --- a/gcc/tree-profile.cc +++ b/gcc/tree-profile.cc @@ -297,6 +297,10 @@ contract_edge (edge e, sbitmap G = nullptr) return source; if (block_conditional_p (dest)) return e; + /* This happens for switches, and must be checked after the is-conditional + (which is also not single). */ + if (!single (dest->succs)) + return source; if (G) bitmap_set_bit (G, dest->index);