On 3/28/22 16:40, Jørgen Kvalsvik via Gcc-patches wrote:
... And with another tiny change that fixes Martin's while (1); case.
Hello.
Back to this ;) Thank you for the updated version of the patch. I have a couple
of comments/requests:
1) I noticed the following cannot be linked:
$ cat errors.C
char trim_filename_name;
int r;
void trim_filename() {
if (trim_filename_name)
r = 123;
while (trim_filename_name)
;
}
int
main() {}
$ g++ errors.C -fprofile-conditions -O2
mold: error: undefined symbol: /tmp/ccmZANB5.o: __gcov8._Z13trim_filenamev
collect2: error: ld returned 1 exit status
Btw. how much have you tested the patch set so far? Have you tried building
something bigger
with -fprofile-conditions enabled?
2) As noticed by Sebastian, please support the new tag in gcov-dump:
$ gcov-dump -l a.gcno
...
a.gcno: 01450000: 28:LINES
a.gcno: block 7:`a.c':11
a.gcno: 01470000: 8:UNKNOWN
3) Then I have some top-level formatting comments:
a) please re-run check_GNU_style.py, I still see:
=== ERROR type #1: blocks of 8 spaces should be replaced with tabs (35
error(s)) ===
...
b) write comments for each newly added function (and separate it by one empty
line from
the function definition)
c) do not create visual alignment, we don't use it:
+ cond->set ("count", new json::integer_number (count));
+ cond->set ("covered", new json::integer_number (covered));
and similar examples
d) finish multiple comments after a dot on the same line:
+ /* Number of expressions found - this value is the number of entries in the
+ gcov output and the parameter to gcov_counter_alloc ().
+ */
should be ... gcov_counter_alloc (). */
e) for long lines with a function call like:
+ const int n = find_conditions_masked_by
+ (block, expr, flags + k, path, CONDITIONS_MAX_TERMS);
do rather
const int n
= find_conditions_masked_by (block, expr,
next_arg, ...);
f) for function params:
+int
+collect_reachable_conditionals (
+ basic_block pre,
+ basic_block post,
+ basic_block *out,
+ int maxsize,
+ sbitmap expr)
use rather:
int
collect_reachable_conditionals (basic_block pre,
second_arg,...
In the next round, I'm going to take a look at the CFG algorithm that identifies
and instruments the sub-expressions.
Thanks.
Cheers,
Martin
