This patch tries to remove all the direct use of DeclID except the real
low level reading and writing. All the use of DeclID is converted to
the use of LocalDeclID or GlobalDeclID. This is helpful to increase the
readability and type safety.
This patch tries to remove all the direct use of DeclID except the real
low level reading and writing. All the use of DeclID is converted to
the use of LocalDeclID or GlobalDeclID. This is helpful to increase the
readability and type safety.
Previously, the DeclID is defined in serialization/ASTBitCodes.h under
clang::serialization namespace. However, actually the DeclID is not
purely used in serialization part. The DeclID is already widely used in
AST and all around the clang project via classes like `LazyPtrDecl` or
calling `ExternalASTSource::getExernalDecl()`. All such uses are via the
raw underlying type of `DeclID` as `uint32_t`. This is not pretty good.
This patch moves the DeclID class family to a new header `AST/DeclID.h`
so that the whole project can use the wrapped class `DeclID`,
`GlobalDeclID` and `LocalDeclID` instead of the raw underlying type.
This can improve the readability and the type safety.
When writing out a PCM, we skip serializing headers' `HeaderFileInfo`
struct whenever this condition evaluates to `true`:
```c++
!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader)
```
However, when Clang parses a module map file, each textual header gets a
`HFI` with `isModuleHeader=false`, `isTextualModuleHeader=true` and
`isCompilingModuleHeader=false`. This means the condition evaluates to
`false` even if the header was never included and the module map did not
affect the compilation. Each PCM file that happened to parse such module
map then contains a copy of the `HeaderFileInfo` struct for all textual
headers, and considers the containing module map affecting.
This patch makes it so that we skip headers that have not been included,
essentially removing the virality of textual headers when it comes to
PCM serialization.
When in-place new-ing a local variable of an array of trivial type, the
generated code calls 'memset' with the correct size of the array,
earlier it was generating size (squared of the typedef array + size).
The cause: typedef TYPE TArray[8]; TArray x; The type of declarator is
Tarray[8] and in SemaExprCXX.cpp::BuildCXXNew we check if it's of
typedef and of constant size then we get the original type and it works
fine for non-dependent cases.
But in case of template we do TreeTransform.h:TransformCXXNEWExpr and
there we again check the allocated type which is TArray[8] and it stays
that way, so ArraySize=(Tarray[8] type, alloc Tarray[8*type]) so the
squared size allocation.
ArraySize gets calculated earlier in TreeTransform.h so that
if(!ArraySize) condition was failing.
fix: I changed that condition to if(ArraySize).
fixes https://github.com/llvm/llvm-project/issues/41441
---------
Co-authored-by: erichkeane <ekeane@nvidia.com>
This patch tries to use DeclID in the code bases to avoid use the raw
type 'uint32_t'. It is problematic to use the raw type 'uint32_t' if we
want to change the type of DeclID some day.
num_gangs takes an 'int-expr-list', for 'parallel', and an 'int-expr'
for 'kernels'. This patch changes the parsing to always parse it as an
'int-expr-list', then correct the expression count during Sema. It also
implements the rest of the semantic analysis changes for this clause.
Pruning non-affecting module maps is useful even when passing module
maps explicitly via `-fmodule-map-file=<path>`. For this situation, this
patch reinstates the behavior we had prior to #87849. For the situation
where the explicit module map file arguments were generated by the
dependency scanner (which already pruned the non-affecting ones), this
patch introduces new `-cc1` flag
`-fno-modules-prune-non-affecting-module-map-files` that avoids the
extra work.
The 'vector_length' clause is semantically identical to the
'num_workers' clause, in that it takes a mandatory single int-expr. This
is implemented identically to it.
module purview
For,
```
export module A;
static int impl() { ... }
export int func() { return impl(); }
```
Previously, even with reduced BMI, the function `impl` will be emitted
into the BMI. After the patch, the static entities in module purview
won't get emitted eagerly. Now the static entities may only be emitted
if required.
Note that, this restriction is actually more relaxed than the language
standard required. The language spec said, the program is ill-formed if
any TU-local entities get exposed. However, we can't do this since there
are many static entities in the headers of existing libraries.
Forbidding that will cause many existing program fail immediately.
Another note here is, we can't do this for non-static non-exported
entities, they can be used for other module units within the same
module.
ASTWrite would write some special records for the consumer of the AST
can get the information easily. This is fine. But currently all the
special records are written eagerly, which is conflicting with reduced
BMI.
In reduced BMI, we hope to write things as less as possible. It is not a
goal for reduced BMI to retain all the informations. So in this patch we
won't record the special declarations eagerly before we starting write.
But only writing the sepcial records after we writes decls and types,
then only the reached declarations will be collected.
ASTWriter::WriteASTCore
ASTWriter::WriteASTCore is a big function and slightly hard to read.
This patch extracts the logics to force emitting special declarations
and writing special records for these declarations into member
functions.
This is helpful to improve the readability and also helpful for the
following patch to write special declarations lazily.
Reduced BMI
Mitigate https://github.com/llvm/llvm-project/issues/61447
The root cause of the above problem is that when we write a declaration,
we need to lookup all the redeclarations in the imported modules. Then
it will be pretty slow if there are too many redeclarations in different
modules. This patch doesn't solve the porblem.
What the patchs mitigated is, when we writing a named module, we shouldn't
write the declarations from GMF if it is unreferenced **in current
module unit**. The difference here is that, if the declaration is used
in the imported modules, we used to emit it as an update. But we
definitely want to avoid that after this patch.
For that reproducer in
https://github.com/llvm/llvm-project/issues/61447, it used to take 2.5s
to compile and now it only takes 0.49s to compile, which is a big win.
`self` clauses on compute constructs take an optional condition
expression. We again limit the implementation to ONLY compute constructs
to ensure we get all the rules correct for others. However, this one
will be particularly complicated, as it takes a `var-list` for `update`,
so when we get to that construct/clause combination, we need to do that
as well.
This patch also furthers uses of the `OpenACCClauses.def` as it became
useful while implementing this (as well as some other minor refactors as
I went through).
Finally, `self` and `if` clauses have an interaction with each other, if
an `if` clause evaluates to `true`, the `self` clause has no effect.
While this is intended and can be used 'meaningfully', we are warning on
this with a very granular warning, so that this edge case will be
noticed by newer users, but can be disabled trivially.
Close https://github.com/llvm/llvm-project/issues/87609
We tried to profile the body of the lambda expressions in
https://reviews.llvm.org/D153957. But as the original comments show,
it is indeed dangerous. After we tried to skip calculating the ODR
hash values recently, we have fall into this trap twice.
So in this patch, I choose to not profile the body of the lambda
expression. The signature of the lambda is still profiled.
Like with the 'default' clause, this is being applied to only Compute
Constructs for now. The 'if' clause takes a condition expression which
is used as a runtime value.
This is not a particularly complex semantic implementation, as there
isn't much to this clause, other than its interactions with 'self',
which will be managed in the patch to implement that.
Following of https://github.com/llvm/llvm-project/pull/76930
This follows the idea of "only writes what we writes", which I think is
the most natural and efficient way to implement this optimization.
We start writing the BMI from the first declaration in module purview
instead of the global module fragment, so that everything in the GMF
untouched won't be written in the BMI naturally.
The exception is, as I said in
https://github.com/llvm/llvm-project/pull/76930, when we write a
declaration we need to write its decl context, and when we write the
decl context, we need to write everything from it. So when we see
`std::vector`, we basically need to write everything under namespace
std. This violates our intention. To fix this, this patch delays the
writing of namespace in the GMF.
From my local measurement, the size of the BMI decrease to 90M from 112M
for a local modules build. I think this is significant.
This feature will be covered under the experimental reduced BMI so that
it won't affect any existing users. So I'd like to land this when the CI
gets green.
Documents will be added seperately.
Clang uses the `HeaderFileInfo` struct to track bits of information on
header files, which gets used throughout the compiler. We also use this
to compute the set of affecting module maps in `ASTWriter` and in the
end serialize the information into the `HEADER_SEARCH_TABLE` record of a
PCM file, allowing clients to learn about headers from the module. In
doing so, Clang asks for existing `HeaderFileInfo` for all known
`FileEntries`. Note that this asks the loaded PCM files for the
information they have on each header file in question. This seems
unnecessary: we only want to serialize information on header files that
either belong to the current module or that got included textually.
Loaded PCM files can't provide us with any useful information.
For explicit modules with lazy loading (using `-fmodule-map-file=<path>`
with `-fmodule-file=<name>=<path>`) the compiler knows about header
files listed in the module map files on the command-line. This can be a
large number.
Asking for existing `HeaderFileInfo` can trigger deserialization of
`HEADER_SEARCH_TABLE` from loaded PCM files. Keys of the on-disk hash
table consist of the header file size and modification time. However,
with explicit modules Clang zeroes out the modification time. Moreover,
if you import lots of modules, some of their header files end up having
identical sizes. This means lots of hash collisions that can only be
resolved by running the serialized filename through `FileManager` and
comparing equality of the `FileEntry`. This ends up being super
expensive, essentially re-stating lots of the transitively loaded SDK
header files.
This patch cleans up the API for getting `HeaderFileInfo` and makes sure
`ASTWriter` uses the version that doesn't ask loaded PCM files for more
information. This removes the excessive stat traffic coming from
`ASTWriter` hopefully without changing observable behavior.
When writing out a PCM, we compute the set of module maps that did
affect the compilation and we strip the rest to make the output
independent of them. The most common way to read a module map that is
not affecting is with implicit module map search. The other option is to
pass a bunch of unnecessary `-fmodule-map-file=<path>` arguments on the
command-line, in which case the client should probably not give those to
Clang anyway.
This makes serialization of explicit modules faster, mostly due to
reduced file system traffic.
As a followup to my previous commits, this is an implementation of a
single clause, in this case the 'default' clause. This implements all
semantic analysis for it on compute clauses, and continues to leave it
rejected for all others (some as 'doesnt appertain', others as 'not
implemented' as appropriate).
This also implements and tests the TreeTransform as requested in the
previous patch.
As a first step in adding clause support for OpenACC to Semantic
Analysis, this patch adds the 'base' AST nodes required for clauses.
This patch has no functional effect at the moment, but followup patches
will add the semantic analysis of clauses (plus individual clauses).
This patch extract logics in ASTWriter::WriteASTCore about writing decls
and types into a standalone function. The WriteASTCore function is
pretty long and hard to read. It should be helpful for readability to extract the common
logics into a standalone function.
This is also helpful for further changes e.g., removing unreachable
declarations.
For pragma diagnostic mappings, we always write/read `SourceLocation`
with offset 0. This is equivalent to just writing a `FileID`, which is
exactly what this patch starts doing.
Originally reviewed here: https://reviews.llvm.org/D137213
HLSL constant sized array function parameters do not decay to pointers.
Instead constant sized array types are preserved as unique types for
overload resolution, template instantiation and name mangling.
This implements the change by adding a new `ArrayParameterType` which
represents a non-decaying `ConstantArrayType`. The new type behaves the
same as `ConstantArrayType` except that it does not decay to a pointer.
Values of `ConstantArrayType` in HLSL decay during overload resolution
via a new `HLSLArrayRValue` cast to `ArrayParameterType`.
`ArrayParamterType` values are passed indirectly by-value to functions
in IR generation resulting in callee generated memcpy instructions.
The behavior of HLSL function calls is documented in the [draft language
specification](https://microsoft.github.io/hlsl-specs/specs/hlsl.pdf)
under the Expr.Post.Call heading.
Additionally the design of this implementation approach is documented in
[Clang's
documentation](https://clang.llvm.org/docs/HLSL/FunctionCalls.html)
Resolves#70123
This patch reorder the lexical block for the translation unit, visible update block for the TU and
the viisble upaete block for the extern C context after the type decl
offsets block.
This should be a NFC patch.
This is helpful for later optimizations for eliding unreachable
declarations in the global module fragment. See the comments in
https://github.com/llvm/llvm-project/pull/76930.
Simply, if we want to get the reachable sets of declaratins during the
writing process, we need to write the file-level context later than the
process of writing declarations (which is the main process to determine
the reachable set).
by default
For reduced BMI, it is meaningless to record the local declarations in
functions if not required explicitly during the process of writing the
function bodies.
It wastes time for reduced BMI and may be problematic if we want to
avoid transiting unnecessary changes.
The `ASTWriter` algorithm for computing affecting module maps uses
`SourceManager::translateFile()` to get a `FileID` from a `FileEntry`.
This is slow (O(n)) since the function performs a linear walk over
`SLocEntries` until it finds one with a matching `FileEntry`.
This patch removes this use of `SourceManager::translateFile()` by
tracking `FileID` instead of `FileEntry` in couple of places in
`ModuleMap`, giving `ASTWriter` the desired `FileID` directly. There are
no changes required for clients that still want a `FileEntry` from
`ModuleMap`: the existing APIs internally use `SourceManager` to perform
the reverse `FileID` to `FileEntry` conversion in O(1).
In `-fbounds-safety`, bounds annotations are considered type attributes
rather than declaration attributes. Constructing them as type attributes
allows us to extend the attribute to apply nested pointers, which is
essential to annotate functions that involve out parameters: `void
foo(int *__counted_by(*out_count) *out_buf, int *out_count)`.
We introduce a new sugar type to support bounds annotated types,
`CountAttributedType`. In order to maintain extra data (the bounds
expression and the dependent declaration information) that is not
trackable in `AttributedType` we create a new type dedicate to this
functionality.
This patch also extends the parsing logic to parse the `counted_by`
argument as an expression, which will allow us to extend the model to
support arguments beyond an identifier, e.g., `__counted_by(n + m)` in
the future as specified by `-fbounds-safety`.
This also adjusts `__bdos` and array-bounds sanitizer code that already
uses `CountedByAttr` to check `CountAttributedType` instead to get the
field referred to by the attribute.
Previously we disabled to compute ODR hash for declarations from the
global module fragment. However, we missed the case that the functions
lives in the concept requiments (see the attached the test files for
example). And the mismatch causes the potential crashment.
Due to we will set the function body as lazy after we deserialize it and
we will only take its body when needed. However, we don't allow to take
the body during deserializing. So it is actually potentially problematic
if we set the body as lazy first and computing the hash value of the
function, which requires to deserialize its body. So we will meet a
crash here.
This patch tries to solve the issue by not taking the body of the
function from GMF. Note that we can't skip comparing the constraint
expression from the GMF directly since it is an key part of the
function selecting and it may be the reason why we can't return 0
directly for `FunctionDecl::getODRHash()` from the GMF.
Close https://github.com/llvm/llvm-project/issues/71034
See
https://discourse.llvm.org/t/rfc-c-20-modules-introduce-thin-bmi-and-decls-hash/74755
This patch introduces reduced BMI, which doesn't contain the definitions
of functions and variables if its definitions won't contribute to the
ABI.
Testing is a big part of the patch. We want to make sure the reduced BMI
contains the same behavior with the existing and relatively stable
fatBMI. This is pretty helpful for further reduction.
The user interfaces part it left to following patches to ease the
reviewing.
Close https://github.com/llvm/llvm-project/issues/80570.
In
a0b6747804,
we skipped ODR checks for decls in GMF. Then it should be natural to
skip storing the ODR values in BMI.
Generally it should be fine as long as the writer and the reader keep
consistent.
However, the use of preamble in clangd shows the tricky part.
For,
```
// test.cpp
module;
// any one off these is enough to crash clangd
// #include <iostream>
// #include <string_view>
// #include <cmath>
// #include <system_error>
// #include <new>
// #include <bit>
// probably many more
// only ok with libc++, not the system provided libstdc++ 13.2.1
// these are ok
export module test;
```
clangd will store the headers as preamble to speedup the parsing and the
preamble reuses the serialization techniques. (Generally we'd call the
preamble as PCH. However it is not true strictly. I've tested the PCH
wouldn't be problematic.) However, the tricky part is that the preamble
is not modules. It literally serialiaze and deserialize things. So
before clangd parsing the above test module, clangd will serialize the
headers into the preamble. Note that there is no concept like GMF now.
So the ODR bits are stored. However, when clangd parse the file
actually, the decls from preamble are thought as in GMF literally, then
hte ODR bits are skipped. Then mismatch happens.
To solve the problem, this patch adds another bit for decls to record
whether or not the ODR bits are skipped.
This patch expands notion of "interesting" in `IdentifierInto` it to
also cover ObjC keywords and builtins, which matches notion of
"interesting" in serialization layer. What was previously "interesting"
in `IdentifierInto` is now called "notable".
Beyond clearing confusion between serialization and the rest of the
compiler, it also resolved a naming problem: ObjC keywords, notable
identifiers, and builtin IDs are all stored in the same bit-field. Now
we can use "interesting" to name it and its corresponding type, instead
of `ObjCKeywordOrInterestingOrBuiltin` abomination.
This patch refactors how values are stored inside
`IdentifierInfo::ObjcOrBuiltinID` bit-field, and annotates it with
`preferred_type`. In order to make the value easier to interpret by
debuggers, a new `ObjCKeywordOrInterestingOrBuiltin` enum is added.
Previous "layout" of this fields couldn't be represented with this new enum,
because it skipped over some arbitrary enumerators, so a new "layout"
was invented, which is reflected in `ObjCKeywordOrInterestingOrBuiltin` enum. I
believe the new layout is simpler than the new one.
Close https://github.com/llvm/llvm-project/issues/79240
Cite the comment from @mizvekov in
//github.com/llvm/llvm-project/issues/79240:
> There are two kinds of bugs / issues relevant here:
>
> Clang bugs that this change hides
> Here we can add a Frontend flag that disables the GMF ODR check, just
> so
> we can keep tracking, testing and fixing these issues.
> The Driver would just always pass that flag.
> We could add that flag in this current issue.
> Bugs in user code:
> I don't think it's worth adding a corresponding Driver flag for
> controlling the above Frontend flag, since we intend it's behavior to
> become default as we fix the problems, and users interested in testing
> the more strict behavior can just use the Frontend flag directly.
This patch follows the suggestion:
- Introduce the CC1 flag `-fskip-odr-check-in-gmf` which is by default
off, so that the every existing test will still be tested with checking
ODR violations.
- Passing `-fskip-odr-check-in-gmf` in the driver to keep the behavior
we intended.
- Edit the document to tell the users who are still interested in more
strict checks can use `-Xclang -fno-skip-odr-check-in-gmf` to get the
existing behavior.
This is a support for " #pragma omp atomic compare weak". It has Parser
& AST support for now.
---------
Authored-by: Sunil Kuravinakop <kuravina@pe28vega.us.cray.com>
`-ivfsoverlay` files are unused when building most modules. Enable
removing them by,
* adding a way to visit the filesystem tree with extensible RTTI to
access each `RedirectingFileSystem`.
* Adding tracking to `RedirectingFileSystem` to record when it
actually redirects a file access.
* Storing this information in each PCM.
Usage tracking is only enabled when iterating over the source manager
and affecting modulemaps. Here each path is stated to cause an access.
During scanning these stats all hit the cache.
Close https://github.com/llvm/llvm-project/issues/79240.
See the linked issue for details. Given the frequency of issue reporting
about false positive ODR checks (I received private issue reports too),
I'd like to backport this to 18.x too.
Implements https://isocpp.org/files/papers/P2662R3.pdf
The feature is exposed as an extension in older language modes.
Mangling is not yet supported and that is something we will have to do before release.
Previously committed as 9e08e51a20d0d2b1c5724bb17e969d036fced4cd, and
reverted because a dependency commit was reverted, then committed again
as 4b574008aef5a7235c1f894ab065fe300d26e786 and reverted again because
"dependency commit" 5a391d38ac6c561ba908334d427f26124ed9132e was
reverted. But it doesn't seem that 5a391d38ac6c was a real dependency
for this.
This commit incorporates 4b574008aef5a7235c1f894ab065fe300d26e786 and
18e093faf726d15f210ab4917142beec51848258 by Richard Smith (@zygoloid),
with some minor fixes, most notably:
- `UncommonValue` renamed to `StructuralValue`
- `VK_PRValue` instead of `VK_RValue` as default kind in lvalue and
member pointer handling branch in
`BuildExpressionFromNonTypeTemplateArgumentValue`;
- handling of `StructuralValue` in `IsTypeDeclaredInsideVisitor`;
- filling in `SugaredConverted` along with `CanonicalConverted`
parameter in `Sema::CheckTemplateArgument`;
- minor cleanup in
`TemplateInstantiator::transformNonTypeTemplateParmRef`;
- `TemplateArgument` constructors refactored;
- `ODRHash` calculation for `UncommonValue`;
- USR generation for `UncommonValue`;
- more correct MS compatibility mangling algorithm (tested on MSVC ver.
19.35; toolset ver. 143);
- IR emitting fixed on using a subobject as a template argument when the
corresponding template parameter is used in an lvalue context;
- `noundef` attribute and opaque pointers in `template-arguments` test;
- analysis for C++17 mode is turned off for templates in
`warn-bool-conversion` test; in C++17 and C++20 mode, array reference
used as a template argument of pointer type produces template argument
of UncommonValue type, and
`BuildExpressionFromNonTypeTemplateArgumentValue` makes
`OpaqueValueExpr` for it, and `DiagnoseAlwaysNonNullPointer` cannot see
through it; despite of "These cases should not warn" comment, I'm not
sure about correct behavior; I'd expect a suggestion to replace `if` by
`if constexpr`;
- `temp.arg.nontype/p1.cpp` and `dr18xx.cpp` tests fixed.
Close https://github.com/llvm/llvm-project/issues/77995
The cause of the issue is that straight forward that we recorded the
'#pragma once' information in named modules, which is an overlook.
I tried to not record the header's information completely within named
modules. But the tests look not happy with some diagnosing problems,
which needs to be looked in details.
This patch tries to pack more bits into a value to reduce the size of
.pcm files. Also, after we introduced BitsPackers, it may slightly
better to adjust the way we use Abbrev.
After this patch, the size of the BMI for std module reduce from 28.94MB
to 28.1 MB.
This was reverted due to it broke the build of lldb. The reason that we
skip the serialization of a source location incorrectly. And this patch
now fixes that.
