On 10/5/21 12:04, Richard Biener wrote:
On Mon, Oct 4, 2021 at 1:32 PM Martin Liška <mli...@suse.cz> wrote:
On 10/4/21 13:16, Richard Biener wrote:
I meant in merge_one_data do not check ->stamp or ->checksum but instead rely
on the counter merging code to detect mismatches (there's read_mismatch and
read_error). There's multiple things we can do when we run into those:
- when we did not actually merged any counter yet we could issue the
warning as before and drop the old data on the floor
- when we_did_ merge some counters already we could hard-error
(I suppose we can't roll-back merging that took place already)
- we could do the merging two-stage, first see whether the data matches
and only if it did perform the merging
I've got your point, you are basically suggesting a fine grained merging
(function based). Huh, I don't like it much as it's typically a mistake
in the build setup that 2 objects (with a different checksum) want to emit
profile to the same .gcda file.
I agree, it's usually a mistake.
My patch handles the obvious situation where an object file is built exactly
the same way (so no e.g. -O0 and -O2).
Yeah, but then the two profiles may not be related at all ...
Well, it's quite common case that one object file is then linked into multiple
binaries (e.g. util.o in a project). We collect also sum_max:
Sum of individual run max values.
which helps handling such a situation.
Note that all of the changes (including yours) have user-visible effects and
the behavior is somewhat unobvious. Not merging when the object was
re-built is indeed the most obvious behavior so I'm not sure it's a good
idea. A new env variable to say whether to simply keep the_old_ data
when merging in new data isn't possible would be another "fix" I guess?
Even for a situation when checksum matches, but the timestamp is different?
Sure, we can provide env. variables that can tweak the behavior.
I suppose another distinguishing factor might be the name of the executable.
Well, at compile time, we don't know name of a final executable.
But yeah, in the end it's a fishy area ...
So I guess your originally posted patch might be the best way to go - can you
try to amend the documentation as for the behavior with respect to
re-compiling and profile merging? I suppose that if you re-compile just
a single .o you currently merge into all the other .o file counters but _not_
into the newly compiled old counters.
Yes, I can update the documentation.
That would make coverage off
as well for incremental re-compiling?
Yes.
I only can find
@item
Run the program on a representative workload to generate the arc profile
information. This may be repeated any number of times. You can run
concurrent instances of your program, and provided that the file system
supports locking, the data files will be correctly updated. Unless
a strict ISO C dialect option is in effect, @code{fork} calls are
detected and correctly handled without double counting.
but that's under -coverage, not sure if there's a better place to amend.
Note I see there's -fprofile-dir which eventually can be used to "fix"
the SPEC issue as well?
We would have to provide a different option value of -fprofile-dir for both
binaries. That's something we can't easily do in a SPEC config file.
Let me update the documentation bits.
Martin
Richard.
Cheers,
Martin
