Put the guts of `ComputeLineNumbers` into `LineOffsetMapping::get` and
`LineOffsetMapping::LineOffsetMapping`. As a drive-by, store the number
of lines directly in the bump-ptr-allocated array.
Differential Revision: https://reviews.llvm.org/D89913
It turns out that `FileInfo` *always* has a ContentCache. Clarify that
in the code:
- Update the private version of `SourceManager::createFileID` to take a
`ContentCache&` instead of `ContentCache*`, and rename it to
`createFileIDImpl` for clarity.
- Change `FileInfo::getContentCache` to return a reference.
Differential Revision: https://reviews.llvm.org/D89554
`SourceManager::getFileEntryRefForID`'s remaining callers just want the
filename component, which is coming from the `FileInfo`. Replace the API
with `getNonBuiltinFilenameForID`, which also removes another use of
`FileEntryRef::FileEntryRef` outside of `FileManager`.
Both callers are collecting file dependencies, and one of them relied on
this API to filter out built-ins (as exposed by
clang/test/ClangScanDeps/modules-full.cpp). It seems nice to continue
providing that service.
Differential Revision: https://reviews.llvm.org/D89508
`SourceManager::isMainFile` does not use the filename, so it doesn't
need the full `FileEntryRef`; in fact, it's misleading to take the name
because that makes it look relevant. Simplify the API, and in the
process remove some calls to `FileEntryRef::FileEntryRef` in the unit
tests (which were blocking making that private to `SourceManager`).
Differential Revision: https://reviews.llvm.org/D89507
Add helpers `getSLocEntryOrNull`, which handles the `Invalid` logic
around `getSLocEntry`, and `getSLocEntryForFile`, which also checks for
`SLocEntry::isFile`, and use them to reduce repeated code.
Differential Revision: https://reviews.llvm.org/D89503
Replace `ContentCache::getRawBuffer` with `getBufferDataIfLoaded` and
`getBufferIfLoaded`, excising another accessor for the underlying
`MemoryBuffer*` in favour of `StringRef` and `MemoryBufferRef`.
Differential Revision: https://reviews.llvm.org/D89445
This changes `ContentCache::Buffer` to use
`std::unique_ptr<MemoryBuffer>` instead of the `PointerIntPair`. It
drops the (mostly unused) `DoNotFree` bit, instead creating a (new)
non-owning `MemoryBuffer` instance when passed a `MemoryBufferRef`.
Differential Revision: https://reviews.llvm.org/D67030
Move a flag out of the `MemoryBuffer*` to unblock changing it to a
`unique_ptr`. There are plenty of bits available in the bitfield below.
Differential Revision: https://reviews.llvm.org/D89431
Inline `Source::getBufferPointer` into its only remaining caller,
`getBufferOrNone`. No functionality change.
Differential Revision: https://reviews.llvm.org/D89430
Replace `SourceManager::getMemoryBufferForFile`, which returned a
dereferenceable `MemoryBuffer*` and had a `bool*Invalid` out parameter,
with `getMemoryBufferForFileOrNone` (returning
`Optional<MemoryBufferRef>`) and `getMemoryBufferForFileOrFake`
(returning `MemoryBufferRef`).
Differential Revision: https://reviews.llvm.org/D89429
In order to drop the final callers to `SourceManager::getBuffer`, change
`FrontendInputFile` to use `Optional<MemoryBufferRef>`. Also updated
the "unowned" version of `SourceManager::createFileID` to take a
`MemoryBufferRef` (it now calls `MemoryBuffer::getMemBuffer`, which
creates a `MemoryBuffer` that does not own the buffer data).
Differential Revision: https://reviews.llvm.org/D89427
Update clang/lib/Basic to stop relying on a `MemoryBuffer*`, using the
`MemoryBufferRef` from `getBufferOrNone` or `getBufferOrFake` instead of
`getBuffer`.
Differential Revision: https://reviews.llvm.org/D89394
Remove `ContentCache::getBuffer`, which always returned a
dereferenceable `MemoryBuffer*` and had a `bool*Invalid` out parameter,
and replace it with:
- `ContentCache::getBufferOrNone`, which returns
`Optional<MemoryBufferRef>`. This is the new API that consumers should
use. Later it could be renamed to `getBuffer`, but intentionally using
a different name to root out any unexpected callers.
- `ContentCache::getBufferPointer`, which returns `MemoryBuffer*` with
"optional" semantics. This is `private` to avoid growing callers and
`SourceManager` has temporarily been made a `friend` to access it.
Later paches will update the transitive callers to not need a raw
pointer, and eventually this will be deleted.
No functionality change intended here.
Differential Revision: https://reviews.llvm.org/D89348
A seemingly innocuous Linux kernel change [0] seemingly blew up our
compile times by over 3x, as reported by @nathanchance in [1].
The code in question uses a doubly nested macro containing GNU C
statement expressions that are then passed to typeof(), which is then
used in a very important macro for atomic variable access throughout
most of the kernel. The inner most macro, is passed a GNU C statement
expression. In this case, we have macro arguments that are GNU C
statement expressions, which can contain a significant number of tokens.
The upstream kernel patch caused significant build time regressions for
both Clang and GCC. Since then, some of the nesting has been removed via
@melver, which helps gain back most of the lost compilation time. [2]
Profiles collected [3] from compilations of the slowest TU for us in the
kernel show:
* 51.4% time spent in clang::TokenLexer::updateLocForMacroArgTokens
* 48.7% time spent in clang::SourceManager::getFileIDLocal
* 35.5% time spent in clang::SourceManager::isOffsetInFileID
(mostly calls from the former through to the latter).
So it seems we have a pathological case for which properly tracking the
SourceLocation of macro arguments is significantly harming build
performance. This stands out in referenced flame graph.
In fact, this case was identified previously as being problematic in
commit 3339c568c4 ("[Lex] Speed up updateConsecutiveMacroArgTokens (NFC)")
Looking at the above call chain, there's 3 things we can do to speed up
this case.
1. TokenLexer::updateConsecutiveMacroArgTokens() calls
SourceManager::isWrittenInSameFile() which calls
SourceManager::getFileID(), which is both very hot and very expensive
to call. SourceManger has a one entry cache, member LastFileIDLookup.
If that isn't the FileID for a give source location offset, we fall
back to a linear probe, and then to a binary search for the FileID.
These fallbacks update the one entry cache, but noticeably they do
not for the case of macro expansions!
For the slowest TU to compile in the Linux kernel, it seems that we
miss about 78.67% of the 68 million queries we make to getFileIDLocal
that we could have had cache hits for, had we saved the macro
expansion source location's FileID in the one entry cache. [4]
I tried adding a separate cache item for macro expansions, and to
check that before the linear then binary search fallbacks, but did
not find it faster than simply allowing macro expansions into the one
item cache. This alone nets us back a lot of the performance loss.
That said, this is a modification of caching logic, which is playing
with a double edged sword. While it significantly improves the
pathological case, its hard to say that there's not an equal but
opposite pathological case that isn't regressed by this change.
Though non-pathological cases of builds of the Linux kernel before
[0] are only slightly improved (<1%) and builds of LLVM itself don't
change due to this patch.
Should future travelers find this change to significantly harm their
build times, I encourage them to feel empowered to revert this
change.
2. SourceManager::getFileIDLocal has a FIXME hinting that the call to
SourceManager::isOffsetInFileID could be made much faster since
isOffsetInFileID is generic in the sense that it tries to handle the
more generic case of "local" (as opposed to "loaded") files, though
the caller has already determined the file to be local. This patch
implements a new method that specialized for use when the caller
already knows the file is local, then use that in
TokenLexer::updateLocForMacroArgTokens. This should be less
controversial than 1, and is likely an across the board win. It's
much less significant for the pathological case, but still a
measurable win once we have fallen to the final case of binary
search. D82497
3. A bunch of methods in SourceManager take a default argument.
SourceManager::getLocalSLocEntry doesn't do anything with this
argument, yet many callers of getLocalSLocEntry setup, pass, then
check this argument. This is wasted work. D82498
With this patch applied, the above profile [5] for the same pathological
input looks like:
* 25.1% time spent in clang::TokenLexer::updateLocForMacroArgTokens
* 17.2% time spent in clang::SourceManager::getFileIDLocal
and clang::SourceManager::isOffsetInFileID is no longer called, and thus
falls out of the profile.
There may be further improvements to the general problem of "what
interval contains one number out of millions" than the current use of a
one item cache, followed by linear probing, followed by binary
searching. We might even be able to do something smarter in
TokenLexer::updateLocForMacroArgTokens.
[0] cdd28ad2d8
[1] https://github.com/ClangBuiltLinux/linux/issues/1032
[2] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=locking/kcsan&id=a5dead405f6be1fb80555bdcb77c406bf133fdc8
[3] https://github.com/ClangBuiltLinux/linux/issues/1032#issuecomment-633712667
[4] https://github.com/ClangBuiltLinux/linux/issues/1032#issuecomment-633741923
[5] https://github.com/ClangBuiltLinux/linux/issues/1032#issuecomment-634932736
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D80681
Forked from D80681.
getLocalSLocEntry() has an unused parameter used to satisfy an interface
of libclang (see getInclusions() in
clang/tools/libclang/CIndexInclusionStack.cpp). It's pointless for
callers to construct/pass/check this inout parameter that can never
signify that a FileID is invalid.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D82498
Summary:
A recent Linux kernel commit exposed a performance cliff in Clang. Calls
to SourceManager::getFileIDLocal() when there's a cache miss against
LastFileIDLookup can be relatively expensive, as getFileIDLocal() tries
a few linear probes, then falls back to binary search. The use of
SourceManager::isOffsetInFileID() is also relatively expensive (both
isOffsetInFileID and getFileIDLocal dominated a trace of the performance
cliff case).
As a FIXME notes (and as @kadircet helpfully noted in review of D80681),
there's a few optimizations we can do here since we've already
identified that an offset is local (as opposed to "loaded").
This patch was forked off of D80681, which additionally did this and
modified some caching behavior, as we expect this change to be less
controversial.
In terms of optimizations, we've already determined that the SLocOffset
parameter to SourceManager::getFileIDLocal() is local in the caller
SourceManager::getFileIDSlow(). Also, there's an early continue in the
binary search loop in getFileIDLocal() that are duplicated in
isOffsetInFileID() as pointed out by @kadircet.
Take advantage of these to optimize the binary search patch, and remove
the FIXME.
Reviewers: kadircet
Reviewed By: kadircet
Subscribers: cfe-commits, kadircet, srhines
Tags: #clang
Differential Revision: https://reviews.llvm.org/D82497
The commit 3c28a2dc6b introduced the check that checks if we're
trying to re-enter a main file when building a preamble. Unfortunately this slowed down the preamble
compilation by 80-90% in some test cases, as translateFile is really slow. This change checks
to see if the FileEntry is the main file without calling translateFile, but by using the new
isMainFile check instead. This speeds up preamble building by 1.5-2x for certain test cases that we have.
rdar://59361291
Differential Revision: https://reviews.llvm.org/D79834
Summary:
Macro argument expansion logic relies on skipping file IDs that created
as a result of an include. Unfortunately it fails to do that for
predefined buffer since it doesn't have a valid insertion location.
As a result of that any file ID created for an include inside the
predefined buffers breaks the traversal logic in
SourceManager::computeMacroArgsCache.
To fix this issue we first record number of created FIDs for predefined
buffer, and then skip them explicitly in source manager.
Another solution would be to just give predefined buffers a valid source
location, but it is unclear where that should be..
Reviewers: sammccall
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D78649
Most clients of SourceManager.h need to do things like turning source
locations into file & line number pairs, but this doesn't require
bringing in FileManager.h and LLVM's FS headers.
The main code change here is to sink SM::createFileID into the cpp file.
I reason that this is not performance critical because it doesn't happen
on the diagnostic path, it happens along the paths of macro expansion
(could be hot) and new includes (less hot).
Saves some includes:
309 - /usr/local/google/home/rnk/llvm-project/clang/include/clang/Basic/FileManager.h
272 - /usr/local/google/home/rnk/llvm-project/clang/include/clang/Basic/FileSystemOptions.h
271 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm/Support/VirtualFileSystem.h
267 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm/Support/FileSystem.h
266 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm/Support/Chrono.h
Differential Revision: https://reviews.llvm.org/D75406
Summary:
As discussed in http://lists.llvm.org/pipermail/cfe-dev/2019-October/063459.html
the overflow of the souce locations (limited to 2^31 chars) can generate all sorts of
weird things (bogus warnings, hangs, crashes, miscompilation and correct compilation).
In debug mode this assert would fail. So it might be a good start, as in PR42301,
to detect the failure and exit with a proper error message.
Reviewers: rsmith, thakis, miyuki
Reviewed By: miyuki
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D70183
Summary:
Review comments on {D68767} asked that this duplicated code in clang-format was moved to one central location that being SourceManager (where it had originally be copied from I assume)
Moved function into static function ContentCache::getInvalidBOM(...) - (closest class to where it was defined before)
Updated clang-format to call this static function
Added unit tests for said new function in BasicTests
Sorry not my normal code area so may have the wrong reviewers. (but your names were on the recent history)
Reviewers: bruno, arphaman, klimek, owenpan, mitchell-stellar, dexonsmith
Reviewed By: owenpan
Subscribers: cfe-commits
Tags: #clang, #clang-format, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D68914
If contents of a file that is part of a PCM are overridden when reading
it, but weren't overridden when the PCM was being built, the ASTReader
will emit an error. Now it creates a separate FileEntry for recovery,
bypassing the overridden content instead of discarding it. The
pre-existing testcase clang/test/PCH/remap-file-from-pch.cpp confirms
that the new recovery method works correctly.
This resolves a long-standing FIXME to avoid hypothetically invalidating
another precompiled module that's already using the overridden contents.
This also removes ContentCache-related API that would be unsafe to use
across `CompilerInstance`s in an implicit modules build. This helps to
unblock us sinking it from SourceManager into FileManager in the future,
which would allow us to delete `InMemoryModuleCache`.
https://reviews.llvm.org/D66710
llvm-svn: 370546
Refactor ContentCache::IsSystemFile to IsFileVolatile, checking
SourceManager::userFilesAreVolatile at construction time. This is a
step toward lowering ContentCache down from SourceManager to
FileManager.
No functionality change intended.
https://reviews.llvm.org/D66713
llvm-svn: 369958
when the FileManager is reused across invocations
This commit introduces a parallel API to FileManager's getFile: getFileEntryRef, which returns
a reference to the FileEntry, and the name that was used to access the file. In the case of
a VFS with 'use-external-names', the FileEntyRef contains the external name of the file,
not the filename that was used to access it.
The new API is adopted only in the HeaderSearch and Preprocessor for include file lookup, so that the
accessed path can be propagated to SourceManager's FileInfo. SourceManager's FileInfo now can report this accessed path, using
the new getName method. This API is then adopted in the dependency collector, which now correctly reports dependencies when a file
is included both using a symlink and a real path in the case when the FileManager is reused across multiple Preprocessor invocations.
Note that this patch does not fix all dependency collector issues, as the same problem is still present in other cases when dependencies
are obtained using FileSkipped, InclusionDirective, and HasInclude. This will be fixed in follow-up commits.
Differential Revision: https://reviews.llvm.org/D65907
llvm-svn: 369680
I noticed that SourceManager::translateFile has code that doesn't really make sense.
In particular, if it fails to find a FileID by comparing FileEntry * values, it tries to
look through files that have the same filename, to see if they have a matching inode to try to
find the right FileID. However, the inode comparison seem redundant, as Clang's FileManager
already deduplicates FileEntry * values by inode.
Thus the comparisons between inodes should never actually succeed, and the comparison between FileEntry * values should be sufficient here.
Differential Revision: https://reviews.llvm.org/D65481
llvm-svn: 369585
Now that we've moved to C++14, we no longer need the llvm::make_unique
implementation from STLExtras.h. This patch is a mechanical replacement
of (hopefully) all the llvm::make_unique instances across the monorepo.
Differential revision: https://reviews.llvm.org/D66259
llvm-svn: 368942
Update the callers of FileManager::getFile and FileManager::getDirectory to handle the new llvm::ErrorOr-returning methods.
Signed-off-by: Harlan Haskins <harlan@apple.com>
llvm-svn: 367616
crashing.
Ideally we wouldn't care about the size of a file so long as it fits in
memory, but in practice we have lots of hardocded assumptions that
unsigned can be used to index files, string literals, and so on.
llvm-svn: 364103
If a header file was processed for the second time, we could end up with a
wrong conditional stack and skipped ranges:
In the particular example, if the header guard is evaluated the second time and
it is decided to skip the conditional block, the corresponding "#endif" is
never seen since the preamble does not include it and we end up in the
Tok.is(tok::eof) case with a wrong conditional stack.
Detect the circular inclusion, emit a diagnostic and stop processing the
inclusion.
llvm-svn: 360418
Requires making the llvm::MemoryBuffer* stored by SourceManager const,
which in turn requires making the accessors for that return const
llvm::MemoryBuffer*s and updating all call sites.
The original motivation for this was to use it and fix the TODO in
CodeGenAction.cpp's ConvertBackendLocation() by using the UnownedTag
version of createFileID, and since llvm::SourceMgr* hands out a const
llvm::MemoryBuffer* this is required. I'm not sure if fixing the TODO
this way actually works, but this seems like a good change on its own
anyways.
No intended behavior change.
Differential Revision: https://reviews.llvm.org/D60247
llvm-svn: 357724
to reflect the new license.
We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.
Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.
llvm-svn: 351636
Summary:
SSE2 vectorization was added in 2012, but it is 2018 now and I can't
observe any performance boost (testing clang -E [all Sema/* CodeGen/* with proper -I options]) with the existing _mm_movemask_epi8+countTrailingZeros or the following SSE4.2 (compiling with -msse4.2):
__m128i C = _mm_setr_epi8('\r','\n',0,0,0,0,0,0,0,0,0,0,0,0,0,0);
_mm_cmpestri(C, 2, Chunk, 16, _SIDD_UBYTE_OPS | _SIDD_CMP_EQUAL_ANY | _SIDD_POSITIVE_POLARITY | _SIDD_LEAST_SIGNIFICANT)
Delete the vectorization to simplify the code.
Also simplify the code a bit and don't check the line ending sequence \n\r
Reviewers: bkramer, #clang
Reviewed By: bkramer
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D55484
llvm-svn: 348777
This patch moves the virtual file system form clang to llvm so it can be
used by more projects.
Concretely the patch:
- Moves VirtualFileSystem.{h|cpp} from clang/Basic to llvm/Support.
- Moves the corresponding unit test from clang to llvm.
- Moves the vfs namespace from clang::vfs to llvm::vfs.
- Formats the lines affected by this change, mostly this is the result of
the added llvm namespace.
RFC on the mailing list:
http://lists.llvm.org/pipermail/llvm-dev/2018-October/126657.html
Differential revision: https://reviews.llvm.org/D52783
llvm-svn: 344140
Summary: This can be used to create a virtual environment (incl. VFS, source manager) for code snippets.
Reviewers: sammccall, klimek
Reviewed By: sammccall
Subscribers: klimek, mgorny, cfe-commits
Differential Revision: https://reviews.llvm.org/D46176
llvm-svn: 331923
This is similar to the LLVM change https://reviews.llvm.org/D46290.
We've been running doxygen with the autobrief option for a couple of
years now. This makes the \brief markers into our comments
redundant. Since they are a visual distraction and we don't want to
encourage more \brief markers in new code either, this patch removes
them all.
Patch produced by
for i in $(git grep -l '\@brief'); do perl -pi -e 's/\@brief //g' $i & done
for i in $(git grep -l '\\brief'); do perl -pi -e 's/\\brief //g' $i & done
Differential Revision: https://reviews.llvm.org/D46320
llvm-svn: 331834
Duncan P. N. Exon Smith