This is part of a patch series working towards the ability to make
SourceLocation into a 64-bit type to handle larger translation units.
NFC: this patch introduces typedefs for the integer type used by
SourceLocation and makes all the boring changes to use the typedefs
everywhere, but for the moment, they are unconditionally defined to
uint32_t.
Patch originally by Mikhail Maltsev.
Reviewed By: tmatheson
Differential Revision: https://reviews.llvm.org/D105492
Clang spends a decent amount of time in the LineOffsetMapping::get(...)
function. This function used to be vectorized (through SSE2) then the
optimization got dropped because the sequential version was on-par performance
wise.
This provides an optimization of the sequential version that works on a word at
a time, using (documented) bithacks to provide a portable vectorization.
When preprocessing the sqlite amalgamation, this yields a sweet 3% speedup.
This is a recommit of 6951b72334 with endianness
and unsigned long vs uint64_t issues fixed (hopefully).
Differential Revision: https://reviews.llvm.org/D99409
Clang spends a decent amount of time in the LineOffsetMapping::get(...)
function. This function used to be vectorized (through SSE2) then the
optimization got dropped because the sequential version was on-par performance
wise.
This provides an optimization of the sequential version that works on a word at
a time, using (documented) bithacks to provide a portable vectorization.
When preprocessing the sqlite amalgamation, this yields a sweet 3% speedup.
Differential Revision: https://reviews.llvm.org/D99409
Use the fact that the number of line break is lower than printable characters to
guide the optimization process. Also use a fuzzy test that catches both \n and
\r in a single check to speedup the computation.
Differential Revision: https://reviews.llvm.org/D97320
Change `SourceManager::getOrCreateContentCache` to take a `FileEntryRef`
and update call sites (mostly internal to SourceManager.cpp). In a
couple of cases this temporarily relies on `FileEntry::getLastRef`, but
those can be cleaned up once other APIs switch over.
The one change outside of SourceManager.cpp is in ASTReader.cpp, which
stops relying on the auto-degrade-to-`FileEntry*` behaviour from
`InputFile::getFile` since it now needs a `FileEntryRef`.
No functionality change here.
Differential Revision: https://reviews.llvm.org/D92983
Change `SourceManager::createFileID(const FileEntry*)` to defer to
`SourceManager::createFileID(FileEntryRef)`. This fixes an unexercised
bug where the latter gained support for named pipes and the former
didn't, but since we're trying to remove all calls to the former it
doesn't really make sense to test this explicitly now that the
implementation is hollowed out.
This is a belated follow-up to 245218bb35,
which sunk named pipe support into FileManager and SourceManager. The
original version of that patch was based on top of
https://reviews.llvm.org/D92984, which removed the `FileEntry` overload
of `createFileID()`, and I missed the subtle difference when it was
rebased.
Add support for stdin to SourceManager and FileManager. Adds
FileManager::getSTDIN, which adds a FileEntryRef for `<stdin>` and reads
the MemoryBuffer, which is stored as `FileEntry::Content`.
Eventually the other buffers in `ContentCache` will sink to here as well
-- we probably usually want to load/save a MemoryBuffer eagerly -- but
it's happening early for stdin to get rid of
CompilerInstance::InitializeSourceManager's final call to
`SourceManager::overrideFileContents`.
clang/test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.export/p1.cpp
relies on building a module from stdin; supporting that requires setting
ContentCache::BufferOverridden.
Differential Revision: https://reviews.llvm.org/D93148
Handle named pipes natively in SourceManager and FileManager, removing a
call to `SourceManager::overrideFileContents` in
`CompilerInstance::InitializeSourceManager` (removing a blocker for
sinking the content cache to FileManager (which will incidently sink
this new named pipe logic with it)).
SourceManager usually checks if the file entry's size matches the
eventually loaded buffer, but that's now skipped for named pipes since
the `stat` won't reflect the full size. Since we can't trust
`ContentsEntry->getSize()`, we also need shift the check for files that
are too large until after the buffer is loaded... and load the buffer
immediately in `createFileID` so that no client gets a bad value from
`ContentCache::getSize`. `FileManager::getBufferForFile` also needs to
treat these files as volatile when loading the buffer.
Native support in SourceManager / FileManager means that named pipes can
also be `#include`d, and clang/test/Misc/dev-fd-fs.c was expanded to
check for that.
This is a new version of 3b18a594c7, which
was reverted in b346322019 since it was
missing the `SourceManager` changes.
Differential Revision: https://reviews.llvm.org/D92531
Change the `InputFile` class to store `Optional<FileEntryRef>` instead
of `FileEntry*`. This paged in a few API changes:
- Added `FileManager::getVirtualFileRef`, and converted `getVirtualFile`
to a wrapper of it.
- Updated `SourceManager::bypassFileContentsOverride` to take
`FileEntryRef` and return `Optional<FileEntryRef>`
(`ASTReader::getInputFile` is the only caller).
Differential Revision: https://reviews.llvm.org/D90053
Shrink `FileEntryRef` to the size of a pointer, by having it directly
reference the `StringMapEntry` the same way that `DirectoryEntryRef`
does. This makes `FileEntryRef::FileEntryRef` private as a side effect
(`FileManager` is a friend!).
There are two helper types added within `FileEntryRef`:
- `FileEntryRef::MapValue` is the type stored in
`FileManager::SeenFileEntries`. It's a replacement for
`SeenFileEntryOrRedirect`, where the second pointer type has been
changed from `StringRef*` to `MapEntry*` (see next bullet).
- `FileEntryRef::MapEntry` is the instantiation of `StringMapEntry<>`
where `MapValue` is stored. This is what `FileEntryRef` has a pointer
to, in order to grab the name in addition to the value.
Differential Revision: https://reviews.llvm.org/D89488
Instead of putting a fake `SLocEntry` at `LoadedSLocEntryTable[Index]`
when it fails to load in `SourceManager::loadSLocEntry`, allocate a fake
one. Unless someone is sniffing the address of the returned `SLocEntry`
(doubtful), this won't be a functionality change. Note that
`SLocEntryLoaded[Index]` wasn't being set to `true` either before or
after this change so no accessor is every going to look at
`LoadedSLocEntryTable[Index]`.
As a side effect, drop the `mutable` from `LoadedSLocEntryTable`.
Differential Revision: https://reviews.llvm.org/D89748
4dc5573acc added `FileEntryRef` in order to
help enable sharing of a `FileManager` between `CompilerInstance`s.
It also added a `StringRef` with the filename on `FileInfo`. This
doubled `sizeof(FileInfo)`, bloating `sizeof(SLocEntry)`, of which we
have one for each (loaded and unloaded) file and macro expansion. This
causes a memory regression in modules builds.
Move the filename down into the `ContentCache`, which is a side data
structure for `FileInfo` that does not impact `sizeof(SLocEntry)`. Once
`FileEntryRef` is used for `ContentCache::OrigEntry` this can go away.
Differential Revision: https://reviews.llvm.org/D89580
Radar-Id: rdar://59908826
Update a few APIs to return non-const references instead of pointers,
and remove associated `const_cast`s and non-null assertions.
Differential Revision: https://reviews.llvm.org/D90067
Use `LineOffsetMapping:get` directly and remove/inline the helper
`ComputeLineNumbers`, simplifying the callers.
Differential Revision: https://reviews.llvm.org/D89922
Avoid some noisy `const_cast`s by making `ContentCache::SourceLineCache`
and `SourceManager::LastLineNoContentCache` both mutable.
Differential Revision: https://reviews.llvm.org/D89914
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
Simon Tatham