We need to keep the underlying profile reader alive as long as the
profile data, because the profile data may contain StringRefs referring
to strings in the reader's name table.
llvm-svn: 349600
Create a temporary file in the system temporary directory instead of creating a
file in the current directory, which may be not writable. (Fix for an issue
introduced in r342283.)
llvm-svn: 342386
Before this fix, multiple invocations of testRoundTrip will create multiple
writers which share the same file as output destination. That could introduce
filesystem race issue when multiple subtests are executed concurrently. This
patch assign writers with different files as their output destinations.
llvm-svn: 342301
The patch saves a function offset table which maps function name index to the
offset of its function profile to the start of the binary profile. By using
the function offset table, for those function profiles which will not be used
when compiling a module, the profile reader does't have to read them. For
profile size around 10~20M, it saves ~10% compile time.
Differential Revision: https://reviews.llvm.org/D51863
llvm-svn: 342283
Name table occupies a big chunk of size in current binary format sample profile.
In order to reduce its size, the patch changes the sample writer/reader to
save/restore MD5Hash of names in the name table. Sample annotation phase will
also use MD5Hash of name to query samples accordingly.
Experiment shows compact binary format can reduce the size of sample profile by
2/3 compared with binary format generally.
Differential Revision: https://reviews.llvm.org/D47955
llvm-svn: 334447
Summary:
Don't skip functions with the same name but from different files.
That change makes it possible to generate code coverage reports from
different binaries compiled from different sources even if there are functions
with non-unique names. Without that change, code coverage for such functions is
missing except of the first function processed.
Reviewers: vsk, morehouse
Reviewed By: vsk
Subscribers: llvm-commits, kcc
Differential Revision: https://reviews.llvm.org/D46478
llvm-svn: 331801
r327219 added wrappers to std::sort which randomly shuffle the container before
sorting. This will help in uncovering non-determinism caused due to undefined
sorting order of objects having the same key.
To make use of that infrastructure we need to invoke llvm::sort instead of
std::sort.
Note: This patch is one of a series of patches to replace *all* std::sort to
llvm::sort. Refer the comments section in D44363 for a list of all the
required patches.
llvm-svn: 329475
Summary:
This reverts commit 364eb09576a7667bc6d3ff80c52a83014ccac976 and separates out
the portion that was fixing binary reader error propagation - turns out, there
are production cases where that causes a regression.
Will re-introduce the error propagation fix separately.
The fix to the text reader error propagation is still "in".
Reviewers: bkramer
Reviewed By: bkramer
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D44807
llvm-svn: 328244
Summary:
This fixes a unittest failure introduced by D44717
D44717 introduced lazy sorting of the internal data structures of the
symbol table. The AddrToMD5Map getter was potentially exposing
inconsistent (unsorted) state. We could sort in the accessor, however,
a client may store the pointer and thus bypass the internal state
management of the symbol table. The alternative in this CL blocks
direct access to the state, thus ensuring consistent
externally-observable state.
Reviewers: davidxl, xur, eraman
Reviewed By: xur
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D44757
llvm-svn: 328163
Summary:
External functions appearing as indirect call targets could not be
found in the SymTab, and the value:counter record was represented,
in the text format, using an empty string for the name. This would
then cause a silent parsing error when reading.
This CL:
- adds explicit support for such functions
- fixes the places where we would not propagate errors when reading
- addresses a performance issue due to eager resorting of the SymTab.
Reviewers: xur, eraman, davidxl
Reviewed By: davidxl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D44717
llvm-svn: 328132
SampleProfReader assumes function names in the profile are all mangled names.
However, there are cases that few demangled names are somehow contained in
the profile (usually because of debug info problems), which may trigger parsing
error in SampleProfReader and cause the whole profile to be unusable. The patch
extends SampleProfReader to handle profiles with demangled names, so that those
profiles can still be useful.
Differential revision: https://reviews.llvm.org/D44161
llvm-svn: 326905
This extends r319391. It teaches the segment builder to emit the right
completed segment when more than one region ends at the same location.
Fixes PR35495.
llvm-svn: 319990
We currently use target_link_libraries without an explicit scope
specifier (INTERFACE, PRIVATE or PUBLIC) when linking executables.
Dependencies added in this way apply to both the target and its
dependencies, i.e. they become part of the executable's link interface
and are transitive.
Transitive dependencies generally don't make sense for executables,
since you wouldn't normally be linking against an executable. This also
causes issues for generating install export files when using
LLVM_DISTRIBUTION_COMPONENTS. For example, clang has a lot of LLVM
library dependencies, which are currently added as interface
dependencies. If clang is in the distribution components but the LLVM
libraries it depends on aren't (which is a perfectly legitimate use case
if the LLVM libraries are being built static and there are therefore no
run-time dependencies on them), CMake will complain about the LLVM
libraries not being in export set when attempting to generate the
install export file for clang. This is reasonable behavior on CMake's
part, and the right thing is for LLVM's build system to explicitly use
PRIVATE dependencies for executables.
Unfortunately, CMake doesn't allow you to mix and match the keyword and
non-keyword target_link_libraries signatures for a single target; i.e.,
if a single call to target_link_libraries for a particular target uses
one of the INTERFACE, PRIVATE, or PUBLIC keywords, all other calls must
also be updated to use those keywords. This means we must do this change
in a single shot. I also fully expect to have missed some instances; I
tested by enabling all the projects in the monorepo (except dragonegg),
and configuring both with and without shared libraries, on both Darwin
and Linux, but I'm planning to rely on the buildbots for other
configurations (since it should be pretty easy to fix those).
Even after this change, we still have a lot of target_link_libraries
calls that don't specify a scope keyword, mostly for shared libraries.
I'm thinking about addressing those in a follow-up, but that's a
separate change IMO.
Differential Revision: https://reviews.llvm.org/D40823
llvm-svn: 319840
This is a fix for the coverage segment builder.
If multiple regions must be popped off the active stack at once, and
more than one of them end at the same location, emit a segment using the
count from the most-recent completed region.
Fixes PR35437, rdar://35760630
Testing: invoked llvm-cov on a stage2 build of clang, additional unit
tests, check-profile
llvm-svn: 319391
We've worked around bugs in the frontend by ignoring the count from
wrapped segments when a line has at least one region entry segment.
Those frontend bugs are now fixed, so it's time to regenerate the
checked-in covmapping files and remove the workaround.
llvm-svn: 317761
LineCoverageIterator makes it easy for clients of coverage data to
determine line execution counts for a file or function. The coverage
iteration logic is tricky enough that it really pays not to have
multiple copies of it. Hopefully having just one implementation in LLVM
will make the iteration logic easier to test, reuse, and update.
This commit is NFC but I've added a unit test to go along with it just
because it's easy to do now.
llvm-svn: 316141
A coverage segment contains a starting line and column, an execution
count, and some other metadata. Clients of the coverage library use
segments to prepare line-oriented reports.
Users of the coverage library depend on segments being unique and sorted
in source order. Currently this is not guaranteed (this is why the clang
change which introduced deferred regions was reverted).
This commit documents the "unique and sorted" condition and asserts that
it holds. It also fixes the SegmentBuilder so that it produces correct
output in some edge cases.
Testing: I've added unit tests for some edge cases. I've also checked
that the new SegmentBuilder implementation is fully covered. Apart from
running check-profile and the llvm-cov tests, I've successfully used a
stage1 llvm-cov to prepare a coverage report for an instrumented clang
binary.
Differential Revision: https://reviews.llvm.org/D36813
llvm-svn: 312817
The CoverageMapping::getInstantiations() API retrieved all function
records corresponding to functions with more than one instantiation (e.g
template functions with multiple specializations). However, there was no
simple way to determine *which* function a given record was an
instantiation of. This was an oversight, since it's useful to aggregate
coverage information over all instantiations of a function.
llvm-cov works around this by building a mapping of source locations to
instantiation sets, but this duplicates logic that libCoverage already
has (see FunctionInstantiationSetCollector).
This change adds a new API, CoverageMapping::getInstantiationGroups(),
which returns a list of InstantiationGroups. A group contains records
for each instantiation of some particular function, and also provides
utilities to get the total execution count within the group, the source
location of the common definition, etc.
This lets removes some hacky logic in llvm-cov by reusing
FunctionInstantiationSetCollector and makes the CoverageMapping API
friendlier for other clients.
llvm-svn: 309904
This unit test constructed some profile records incorrectly. One of the
records had a duplicate name: adding that record into the writer caused
an error unrelated to what needed to be tested.
Reported by David Blaikie!
llvm-svn: 307596
Reduces llvm-profdata memory usage on a large profile from 7.8GB to 5.1GB.
The ProfData API now supports reporting all the errors/warnings rather
than only the first, though llvm-profdata ignores everything after the
first for now to preserve existing behavior. (if there's a desire for
other behavior, happy to implement that - but might be as well left for
a separate patch)
Reviewers: davidxl
Differential Revision: https://reviews.llvm.org/D35149
llvm-svn: 307516
The 'NoError' function was meant to be used as the input to
ASSERT/EXPECT_TRUE, but it is easy to forget this (it could be annotated
with nodiscard to help this) so many sites that look like they're checked
are not (& silently discard the failure). Only one site actually has an
Error sneaking out this way and I've replaced that one with a
FIXME+consumeError.
The rest of the code has been modified to use the EXPECT_THAT_ERROR
macros Zach introduced a while back. Between the options available this
seems OK/good/something to standardize on - though it's difficult to
build a matcher that could handle checking for a specific llvm::Error
result, so those remain using the custom ErrorEquals (& the nodiscard
added to ensure it is not misused as it was previous to this patch). It
could still be generalized a bit further (even not as far as a matcher,
but at least support multiple kinds of Error, etc) & added to the
general Error utility header.
llvm-svn: 307440
The InstrProfWriter already stores the name and hash of the record in
the nested maps it uses for lookup while merging - this data is
duplicated in the value within the maps.
Refactor the InstrProfRecord to use a nested struct for the counters
themselves so that InstrProfWriter can use this nested struct alone
without the name or hash duplicated there.
This work is incomplete, but enough to demonstrate the value (around a
50% decrease in memory usage for a large test case (10GB -> 5GB)).
Though most of that decrease is probably from removing the
SoftInstrProfError as well, but I haven't implemented a replacement for
it yet. (it needs to go with the counters, because the operations on the
counters - merging, etc, are where the failures are - unlike the
name/hash which are totally unused by those counter-related operations
and thus easy to split out)
Ongoing discussion about removing SoftInstrProfError as a field of the
InstrProfRecord is happening on the thread that added it - including
the possibility of moving back towards an earlier version of that
proposed patch that passed SoftInstrProfError through the various APIs,
rather than as a member of InstrProfRecord.
Reviewers: davidxl
Differential Revision: https://reviews.llvm.org/D34838
llvm-svn: 307298
These overloads are essentially dead, and pose a maintenance cost
without adding any benefit. This is coming up now because I'd like to
experiment with changing the way we store coverage mapping data, and
would rather not have to fix up the old overloads while doing so.
Testing: check-{llvm,profile}, build clang.
llvm-svn: 306776
With PR33517, it became apparent that symbol table creation can fail
when presented with malformed inputs. This patch makes that sort of
error detectable, so llvm-cov etc. can fail more gracefully.
Specifically, we now check that function records loaded from corrupted coverage
mapping data are rejected, e.g when the recorded function name is garbage.
Testing: check-{llvm,clang,profile}, some unit test updates.
llvm-svn: 305767
With PR33517, it became apparent that symbol table creation can fail
when presented with malformed inputs. This patch makes that sort of
error detectable, so llvm-cov etc. can fail more gracefully.
Specifically, we now check that function names within the symbol table
aren't empty.
Testing: check-{llvm,clang,profile}, some unit test updates.
llvm-svn: 305765
clang-format (https://reviews.llvm.org/D33932) to keep primary headers
at the top and handle new utility headers like 'gmock' consistently with
other utility headers.
No other change was made. I did no manual edits, all of this is
clang-format.
This should allow other changes to have more clear and focused diffs,
and is especially motivated by moving some headers into more focused
libraries.
llvm-svn: 304786
Add support for loading multiple coverage readers into a single
CoverageMapping instance. This should make it easier to prepare a
unified coverage report for multiple binaries.
Differential Revision: https://reviews.llvm.org/D25535
llvm-svn: 284251
Add a "-j" option to llvm-profdata to control the number of threads used.
Auto-detect NumThreads when it isn't specified, and avoid spawning threads when
they wouldn't be beneficial.
I tested this patch using a raw profile produced by clang (147MB). Here is the
time taken to merge 4 copies together on my laptop:
No thread pool: 112.87s user 5.92s system 97% cpu 2:01.08 total
With 2 threads: 134.99s user 26.54s system 164% cpu 1:33.31 total
Changes since the initial commit:
- When handling odd-length inputs, call ThreadPool::wait() before merging the
last profile. Should fix a race/off-by-one (see r275937).
Differential Revision: https://reviews.llvm.org/D22438
llvm-svn: 275938
Add a "-j" option to llvm-profdata to control the number of threads
used. Auto-detect NumThreads when it isn't specified, and avoid spawning
threads when they wouldn't be beneficial.
I tested this patch using a raw profile produced by clang (147MB). Here is the
time taken to merge 4 copies together on my laptop:
No thread pool: 112.87s user 5.92s system 97% cpu 2:01.08 total
With 2 threads: 134.99s user 26.54s system 164% cpu 1:33.31 total
Differential Revision: https://reviews.llvm.org/D22438
llvm-svn: 275921
This removes the subclasses of ProfileSummary, moves the members of the derived classes to the base class.
Differential Revision: http://reviews.llvm.org/D20390
llvm-svn: 270143
