This PR adds new `ModuleCache` interface to Clang's implicitly-built
modules machinery. The main motivation for this change is to create a
second implementation that uses a more efficient kind of
`llvm::AdvisoryLock` during dependency scanning.
In addition to the lock abstraction, the `ModuleCache` interface also
manages the existing `InMemoryModuleCache` instance. I found that
compared to keeping these separate/independent, the code is a bit
simpler now, since these are two tightly coupled concepts. I can
envision a more efficient implementation of the `InMemoryModuleCache`
for the single-process case too, which will be much easier to implement
with the current setup.
This is not intended to be a functional change.
The 'bind' clause allows the renaming of a function during code
generation. There are a few rules about when this can/cannot happen,
and it takes either a string or identifier (previously mis-implemetned
as ID-expression) argument.
Note there are additional rules to this in the implicit-function routine
case, but that isn't implemented in this patch, as implicit-function
routine is not yet implemented either.
'nohost' is only valid on routine, and states that the compiler
shouldn't compile this routine for the host. It has no arguments, so no
checking is required besides putting it in the AST.
The 'routine' construct has two forms, one which takes the name of a
function that it applies to, and another where it implicitly figures it
out based on the next declaration. This patch implements the former with
the required restrictions on the name and the function-static-variables
as specified.
What has not been implemented is any clauses for this, any of the A.3.4
warnings, or the other form.
The 'declare' construct is the first of two 'declaration' level
constructs, so it is legal in any place a declaration is, including as a
statement, which this accomplishes by wrapping it in a DeclStmt. All
clauses on this have a 'same scope' requirement, which this enforces as
declaration context instead, which makes it possible to implement these
as a template.
The 'link' and 'device_resident' clauses are also added, which have some
similar/small restrictions, but are otherwise pretty rote.
This patch implements all of the above.
This makes it significantly easier to add new builtin templates, since
you only have to modify two places instead of a dozen or so.
The `BuiltinTemplates.td` could also be extended to generate
documentation from it in the future.
This merges the functionality of ResolvedUnexpandedPackExpr into
FunctionParmPackExpr. I also added a test to show that
https://github.com/llvm/llvm-project/issues/125103 should be fixed with
this. I put the removal of ResolvedUnexpandedPackExpr in its own commit.
Let me know what you think.
Fixes#125103
The last use was removed in:
commit ee977933f7df9cef13cc06ac7fa3e4a22b72e41f
Author: Richard Smith <richard-llvm@metafoo.co.uk>
Date: Fri May 1 21:22:17 2015 +0000
Add initial parsing/sema support for new assumption clause so clause can
be specified. For now, it's ignored, just like the others.
Added support for 'no_openmp_construct' to release notes.
Testing
- Updated appropriate LIT tests.
- Testing: check-all
Note that PointerUnion::dyn_cast has been soft deprecated in
PointerUnion.h:
// FIXME: Replace the uses of is(), get() and dyn_cast() with
// isa<T>, cast<T> and the llvm::dyn_cast<T>
Literal migration would result in dyn_cast_if_present (see the
definition of PointerUnion::dyn_cast), but this patch uses dyn_cast
because we expect Subject to be nonnull.
This is an implementation of P1061 Structure Bindings Introduce a Pack
without the ability to use packs outside of templates. There is a couple
of ways the AST could have been sliced so let me know what you think.
The only part of this change that I am unsure of is the
serialization/deserialization stuff. I followed the implementation of
other Exprs, but I do not really know how it is tested. Thank you for
your time considering this.
---------
Co-authored-by: Yanzuo Liu <zwuis@outlook.com>
This is take two of #70976. This iteration of the patch makes sure that
custom
diagnostics without any warning group don't get promoted by `-Werror` or
`-Wfatal-errors`.
This implements parts of the extension proposed in
https://discourse.llvm.org/t/exposing-the-diagnostic-engine-to-c/73092/7.
Specifically, this makes it possible to specify a diagnostic group in an
optional third argument.
This reverts commit c3ba6f378ef80d750e2278560c6f95a300114412.
We are seeing performance regressions of up to 40% on some compilations
with this patch, we will investigate and reland after fixing performance
issues.
…ecord level.
This fixes the incorrect diagnostic emitted when compiling the following
snippet
```
// string_view.h
template<class _CharT>
class basic_string_view;
typedef basic_string_view<char> string_view;
template<class _CharT>
class
__attribute__((__preferred_name__(string_view)))
basic_string_view {
public:
basic_string_view()
{
}
};
inline basic_string_view<char> foo()
{
return basic_string_view<char>();
}
// A.cppm
module;
#include "string_view.h"
export module A;
// Use.cppm
module;
#include "string_view.h"
export module Use;
import A;
```
The diagnostic is
```
string_view.h:11:5: error: 'basic_string_view<char>::basic_string_view' from module 'A.<global>' is not present in definition of 'string_view' provided earlier
```
The underlying issue is that deserialization of the `preferred_name`
attribute triggers deserialization of `basic_string_view<char>`, which
triggers the deserialization of the `preferred_name` attribute again
(since it's attached to the `basic_string_view` template).
The deserialization logic is implemented in a way that prevents it from
going on a loop in a literal sense (it detects early on that it has
already seen the `string_view` typedef when trying to start its
deserialization for the second time), but leaves the typedef
deserialization in an unfinished state. Subsequently, the `string_view`
typedef from the deserialized module cannot be merged with the same
typedef from `string_view.h`, resulting in the above diagnostic.
This PR resolves the problem by delaying the deserialization of the
`preferred_name` attribute until the deserialization of the
`basic_string_view` template is completed. As a result of deferring, the
deserialization of the `preferred_name` attribute doesn't need to go on
a loop since the type of the `string_view` typedef is already known when
it's deserialized.
Close https://github.com/llvm/llvm-project/issues/90154
This patch is also an optimization to the lookup process to utilize the
information provided by `export` keyword.
Previously, in the lookup process, the `export` keyword only takes part
in the check part, it doesn't get involved in the lookup process. That
said, previously, in a name lookup for 'name', we would load all of
declarations with the name 'name' and check if these declarations are
valid or not. It works well. But it is inefficient since it may load
declarations that may not be wanted.
Note that this patch actually did a trick in the lookup process instead
of bring module information to DeclarationName or considering module
information when deciding if two declarations are the same. So it may
not be a surprise to me if there are missing cases. But it is not a
regression. It should be already the case. Issue reports are welcomed.
In this patch, I tried to split the big lookup table into a lookup table
as before and a module local lookup table, which takes a combination of
the ID of the DeclContext and hash value of the primary module name as
the key. And refactored `DeclContext::lookup()` method to take the
module information. So that a lookup in a DeclContext won't load
declarations that are local to **other** modules.
And also I think it is already beneficial to split the big lookup table
since it may reduce the conflicts during lookups in the hash table.
BTW, this patch introduced a **regression** for a reachability rule in
C++20 but it was false-negative. See
'clang/test/CXX/module/module.interface/p7.cpp' for details.
This patch is not expected to introduce any other
regressions for non-c++20-modules users since the module local lookup
table should be empty for them.
Close https://github.com/llvm/llvm-project/issues/90154
This patch is also an optimization to the lookup process to utilize the
information provided by `export` keyword.
Previously, in the lookup process, the `export` keyword only takes part
in the check part, it doesn't get involved in the lookup process. That
said, previously, in a name lookup for 'name', we would load all of
declarations with the name 'name' and check if these declarations are
valid or not. It works well. But it is inefficient since it may load
declarations that may not be wanted.
Note that this patch actually did a trick in the lookup process instead
of bring module information to DeclarationName or considering module
information when deciding if two declarations are the same. So it may
not be a surprise to me if there are missing cases. But it is not a
regression. It should be already the case. Issue reports are welcomed.
In this patch, I tried to split the big lookup table into a lookup table
as before and a module local lookup table, which takes a combination of
the ID of the DeclContext and hash value of the primary module name as
the key. And refactored `DeclContext::lookup()` method to take the
module information. So that a lookup in a DeclContext won't load
declarations that are local to **other** modules.
And also I think it is already beneficial to split the big lookup table
since it may reduce the conflicts during lookups in the hash table.
BTW, this patch introduced a **regression** for a reachability rule in
C++20 but it was false-negative. See
'clang/test/CXX/module/module.interface/p7.cpp' for details.
This patch is not expected to introduce any other
regressions for non-c++20-modules users since the module local lookup
table should be empty for them.
---
On the API side, this patch unfortunately add a maybe-confusing argument
`Module *NamedModule` to
`ExternalASTSource::FindExternalVisibleDeclsByName()`. People may think
we can get the information from the first argument `const DeclContext
*DC`. But sadly there are declarations (e.g., namespace) can appear in
multiple different modules as a single declaration. So we have to add
additional information to indicate this.
The 'align' modifier is now accepted in the 'allocate' clause. Added LIT
tests covering codegen, PCH, template handling, and serialization for
'align' modifier.
Added support for align-modifier to release notes.
Testing
- New allocate modifier LIT tests.
- OpenMP LIT tests.
- check-all
These two clauses just take a 'var-list' and specify where the variables
should be copied from/to. This patch implements the AST nodes for them
and ensures they properly take a var-list.
The 'self' clause is an unfortunately difficult one, as it has a
significantly different meaning between 'update' and the other
constructs. This patch introduces a way for the 'self' clause to work
as both. I considered making this two separate AST nodes (one for
'self' on 'update' and one for the others), however this makes the
automated macros/etc for supporting a clause break.
Instead, 'self' has the ability to act as either a condition or as a
var-list clause. As this is the only one of its kind, it is implemented
all within it. If in the future we have more that work like this, we
should consider rewriting a lot of the macros that we use to make
clauses work, and make them separate ast nodes.
A fairly simple one, only valid on the 'set' construct, this clause
takes an int expression. Most of the work was already done as a part of
parsing, so this patch ends up being a lot of infrastructure.
An instantiated templated function definition may not have a body due to
parsing errors inside the templated function. When serializing, an
assert is triggered inside `ASTRecordWriter::AddFunctionDefinition`.
The instantiation may happen on an intermediate module.
The test case was reduced from `mp-units`.
This is a very simple sema implementation, and just required AST node
plus the existing diagnostics. This patch adds tests and adds the AST
node required, plus enables it for 'init' and 'shutdown' (only!)
This is a clause that is only valid on 'host_data' constructs, and
identifies variables which it should use the current device address.
From a Sema perspective, the only thing novel here is mild changes to
how ActOnVar works for this clause, else this is very much like the rest
of the 'var-list' clauses.
'delete' is another clause that has very little compile-time
implication, but needs a full AST that takes a var list. This patch
ipmlements it fully, plus adds sufficient test coverage.
This is another new clause specific to 'exit data' that takes a pointer
argument. This patch implements this the same way we do a few other
clauses (like attach) that have the same restrictions.
The 'if_present' clause controls the replacement of addresses in the
var-list in current device memory. This clause can only go on
'host_device'. From a Sema perspective, there isn't anything to do
beyond add this to AST and pass it on.
This is a very simple clause as far as sema is concerned. It is only
valid on 'exit data', and doesn't have any rules involving it, so it is
simply applied and passed onto the MLIR.
Reland https://github.com/llvm/llvm-project/pull/83237
---
(Original comments)
Currently all the specializations of a template (including
instantiation, specialization and partial specializations) will be
loaded at once if we want to instantiate another instance for the
template, or find instantiation for the template, or just want to
complete the redecl chain.
This means basically we need to load every specializations for the
template once the template declaration got loaded. This is bad since
when we load a specialization, we need to load all of its template
arguments. Then we have to deserialize a lot of unnecessary
declarations.
For example,
```
// M.cppm
export module M;
export template <class T>
class A {};
export class ShouldNotBeLoaded {};
export class Temp {
A<ShouldNotBeLoaded> AS;
};
// use.cpp
import M;
A<int> a;
```
We have a specialization ` A<ShouldNotBeLoaded>` in `M.cppm` and we
instantiate the template `A` in `use.cpp`. Then we will deserialize
`ShouldNotBeLoaded` surprisingly when compiling `use.cpp`. And this
patch tries to avoid that.
Given that the templates are heavily used in C++, this is a pain point
for the performance.
This patch adds MultiOnDiskHashTable for specializations in the
ASTReader. Then we will only deserialize the specializations with the
same template arguments. We made that by using ODRHash for the template
arguments as the key of the hash table.
To review this patch, I think `ASTReaderDecl::AddLazySpecializations`
may be a good entry point.
Note that PointerUnion::{is,get} have been soft deprecated in
PointerUnion.h:
// FIXME: Replace the uses of is(), get() and dyn_cast() with
// isa<T>, cast<T> and the llvm::dyn_cast<T>
I'm not touching PointerUnion::dyn_cast for now because it's a bit
complicated; we could blindly migrate it to dyn_cast_if_present, but
we should probably use dyn_cast when the operand is known to be
non-null.
Currently all the specializations of a template (including
instantiation, specialization and partial specializations) will be
loaded at once if we want to instantiate another instance for the
template, or find instantiation for the template, or just want to
complete the redecl chain.
This means basically we need to load every specializations for the
template once the template declaration got loaded. This is bad since
when we load a specialization, we need to load all of its template
arguments. Then we have to deserialize a lot of unnecessary
declarations.
For example,
```
// M.cppm
export module M;
export template <class T>
class A {};
export class ShouldNotBeLoaded {};
export class Temp {
A<ShouldNotBeLoaded> AS;
};
// use.cpp
import M;
A<int> a;
```
We should a specialization ` A<ShouldNotBeLoaded>` in `M.cppm` and we
instantiate the template `A` in `use.cpp`. Then we will deserialize
`ShouldNotBeLoaded` surprisingly when compiling `use.cpp`. And this
patch tries to avoid that.
Given that the templates are heavily used in C++, this is a pain point
for the performance.
This patch adds MultiOnDiskHashTable for specializations in the
ASTReader. Then we will only deserialize the specializations with the
same template arguments. We made that by using ODRHash for the template
arguments as the key of the hash table.
To review this patch, I think `ASTReaderDecl::AddLazySpecializations`
may be a good entry point.
The patch was reviewed in
https://github.com/llvm/llvm-project/pull/83237 but that PR is a stacked
PR. But I feel the intention of the stacked PRs get lost during the
review process. So I feel it is better to merge the commits into a
single commit instead of merging them in the PR page. It is better for
us to cherry-pick and revert.
This is a follow up to #112015 and it reduces the unnecessary
duplication of source locations further.
We do not need to allocate source location space in the serialized PCMs
for module maps used only to find textual headers. Those module maps are
never referenced from anywhere in the serialized ASTs and are re-read in
other compilations.
This change should not affect correctness of Clang compilations or
clang-scan-deps in any way.
We do need the InputFile entry in the serialized AST because
clang-scan-deps relies on it. The previous patch introduced a mechanism
to do exactly that.
We have found that to finally remove any duplication of module maps we
use internally in our build system.
This PR changes a part of the PCM format to store string-like things in
the blob attached to a record instead of VBR6-encoding them into the
record itself. Applied to the `IMPORTS` section (which is very hot),
this speeds up dependency scanning by 2.8%.
This PR builds on top of
https://github.com/llvm/llvm-project/pull/115235 and makes it possible
to call `ASTWriter::WriteAST()` with `Preprocessor` only instead of full
`Sema` object. So far, there are no clients that leverage the new
capability - that will come in a follow-up commit.
Currently, any FileID that references a module map file that was
required for a compilation is considered as affecting. This misses an
important opportunity to reduce the source location space taken by the
resulting PCM.
In particular, consider the situation where the same module map file is
passed multiple times in the dependency chain:
```shell
$ clang -fmodule-map-file=foo.modulemap ... -o mod1.pcm
$ clang -fmodule-map-file=foo.modulemap -fmodule-file=mod1.pcm ... -o mod2.pcm
...
$ clang -fmodule-map-file=foo.modulemap -fmodule-file=mod$((N-1)).pcm ... -o mod$N.pcm
```
Because `foo.modulemap` is read before reading any of the `.pcm` files,
we have to create a unique `FileID` for it when creating each module.
However, when reading the `.pcm` files, we will reuse the `FileID`
loaded from it for the same module map file and the `FileID` we created
can never be used again, but we will still mark it as affecting and it
will take the source location space in the output PCM.
For a chain of N dependencies, this results in the file taking `N *
(size of file)` source location space, which could be significant. For
examples, we observer internally that some targets that run out of 2GB
of source location space end up wasting up to 20% of that space in
module maps as described above.
I take extra care to still write the InputFile entries for those files that occupied
source location space before. It is required for correctness of clang-scan-deps.
This patch removes `ASTWriter::Context` and starts passing `ASTContext
&` explicitly to functions that actually need it. This is a
non-functional change with the end-goal of being able to write
lightweight PCM files with no `ASTContext` at all.
The 'allocator' modifier is now accepted in the 'allocate' clause. Added
LIT tests covering codegen, PCH, template handling, and serialization
for 'allocator' modifier.
Added support for allocator-modifier to release notes.
Testing
- New allocate modifier LIT tests.
- OpenMP LIT tests.
- check-all
- relevant sollve_vv test cases
tests/5.2/scope/test_scope_allocate_construct.c
This PR removes the `HeaderFileInfo::Framework` member and reduces the
size of this data type from 32B to 16B. This should improve Clang's
memory usage in situations where it keeps track of lots of header files.
NFCI. Depends on #114459.
This PR removes the `-index-header-map` functionality from Clang. AFAIK
this was only used internally at Apple and is now dead code. The main
motivation behind this change is to enable the removal of
`HeaderFileInfo::Framework` member and reducing the size of that data
structure.
rdar://84036149
