Currently, `NamespaceDecl` has a member `AnonOrFirstNamespaceAndFlags`
which stores a few pieces of data:
- a bit indicating whether the namespace was declared `inline`, and
- a bit indicating whether the namespace was declared as a
_nested-namespace-definition_, and
- a pointer a `NamespaceDecl` that either stores:
- a pointer to the first declaration of that namespace if the
declaration is no the first declaration, or
- a pointer to the unnamed namespace that inhabits the namespace
otherwise.
`Redeclarable` already stores a pointer to the first declaration of an
entity, so it's unnecessary to store this in `NamespaceDecl`.
`DeclContext` has 8 bytes in which various bitfields can be stored for a
declaration, so it's not necessary to store these in `NamespaceDecl`
either. We only need to store a pointer to the unnamed namespace that
inhabits the first declaration of a namespace. This patch moves the two
bits currently stored in `NamespaceDecl` to `DeclContext`, and only
stores a pointer to the unnamed namespace that inhabits a namespace in
the first declaration of that namespace. Since `getOriginalNamespace`
always returns the same `NamespaceDecl` as `getFirstDecl`, this function
is removed to avoid confusion.
This reverts commit 18f3bcbb13ca83d33223b00761d8cddf463e9ffb, 15bb02650e26875c48889053d6a9697444583721 and
99873b35da7ecb905143c8a6b8deca4d4416f1a9.
See the post commit message in
https://github.com/llvm/llvm-project/pull/75912 to see the reasons.
Now we can create a LocalDeclID directly with an integer without
verifying. It may be hard to refactor if we want to change the way we
serialize DeclIDs (See https://github.com/llvm/llvm-project/pull/95897).
Also it is hard for us to debug if someday someone construct a
LocalDeclID with an incorrect value.
So in this patch, I tried to unify the way we can construct a
LocalDeclID in ASTReader, where we will construct the LocalDeclID from
the serialized data. Also, now we can verify the constructed LocalDeclID
sooner in the new interface.
carefully
Close https://github.com/llvm/llvm-project/issues/93859
The direct pattern of the issue is that, in a reduced BMI, we're going
to wrtie a class but we didn't write the deduction guide. Although we
handled deduction guide, but we tried to record the found deduction
guide from `noload_lookup` directly.
It is slightly problematic if the found deduction guide is from AST.
e.g.,
```
module;
export module m;
import xxx; // Also contains the class and the deduction guide
...
```
Then when we writes the class in the current file, we tried to record
the deduction guide, but `noload_lookup` returns the deduction guide
from the AST file then we didn't record the local deduction guide. Then
mismatch happens.
To mitiagte the problem, we tried to record the canonical declaration
for the decution guide.
This is an enabler for https://github.com/llvm/llvm-project/pull/92855
This allows an NTTP default argument to be set as an arbitrary
TemplateArgument, not just an expression.
This allows template parameter packs to have default arguments in the
AST, even though the language proper doesn't support the syntax for it.
This allows NTTP default arguments to be other kinds of arguments, like
packs, integral constants, and such.
This is an enabler for a future patch.
This allows an type-parameter default argument to be set as an arbitrary
TemplateArgument, not just a type.
This allows template parameter packs to have default arguments in the
AST, even though the language proper doesn't support the syntax for it.
This will be used in a later patch which synthesizes template parameter
lists with arbitrary default arguments taken from template
specializations.
There are a few places we used SubsType, because we only had a type, now
we use SubstTemplateArgument.
SubstTemplateArgument was missing arguments for setting Instantiation
location and entity names.
Adding those is needed so we don't regress in diagnostics.
Our current method of storing the template arguments as written for
`(Class/Var)Template(Partial)SpecializationDecl` suffers from a number
of flaws:
- We use `TypeSourceInfo` to store `TemplateArgumentLocs` for class
template/variable template partial/explicit specializations. For
variable template specializations, this is a rather unintuitive hack (as
we store a non-type specialization as a type). Moreover, we don't ever
*need* the type as written -- in almost all cases, we only want the
template arguments (e.g. in tooling use-cases).
- The template arguments as written are stored in a number of redundant
data members. For example, `(Class/Var)TemplatePartialSpecialization`
have their own `ArgsAsWritten` member that stores an
`ASTTemplateArgumentListInfo` (the template arguments).
`VarTemplateSpecializationDecl` has yet _another_ redundant member
"`TemplateArgsInfo`" that also stores an `ASTTemplateArgumentListInfo`.
This patch eliminates all
`(Class/Var)Template(Partial)SpecializationDecl` members which store the
template arguments as written, and turns the `ExplicitInfo` member into
a `llvm::PointerUnion<const ASTTemplateArgumentListInfo*,
ExplicitInstantiationInfo*>` (to avoid unnecessary allocations when the
declaration isn't an explicit instantiation). The template arguments as
written are now accessed via `getTemplateArgsWritten` in all cases.
The "most breaking" change is to AST Matchers, insofar that `hasTypeLoc`
will no longer match class template specializations (since they no
longer store the type as written).
This relands 6c31104.
The patch was reverted due to incorrectly introduced alignment. And the
patch was re-commited after fixing the alignment issue.
Following off are the original message:
This is part of "no transitive change" patch series, "no transitive
source location change". I talked this with @Bigcheese in the tokyo's
WG21 meeting.
The idea comes from @jyknight posted on LLVM discourse. That for:
```
// A.cppm
export module A;
...
// B.cppm
export module B;
import A;
...
//--- C.cppm
export module C;
import C;
```
Almost every time A.cppm changes, we need to recompile `B`. Due to we
think the source location is significant to the semantics. But it may be
good if we can avoid recompiling `C` if the change from `A` wouldn't
change the BMI of B.
This patch only cares source locations. So let's focus on source
location's example. We can see the full example from the attached test.
```
//--- A.cppm
export module A;
export template <class T>
struct C {
T func() {
return T(43);
}
};
export int funcA() {
return 43;
}
//--- A.v1.cppm
export module A;
export template <class T>
struct C {
T func() {
return T(43);
}
};
export int funcA() {
return 43;
}
//--- B.cppm
export module B;
import A;
export int funcB() {
return funcA();
}
//--- C.cppm
export module C;
import A;
export void testD() {
C<int> c;
c.func();
}
```
Here the only difference between `A.cppm` and `A.v1.cppm` is that
`A.v1.cppm` has an additional blank line. Then the test shows that two
BMI of `B.cppm`, one specified `-fmodule-file=A=A.pcm` and the other
specified `-fmodule-file=A=A.v1.pcm`, should have the bit-wise same
contents.
However, it is a different story for C, since C instantiates templates
from A, and the instantiation records the source information from module
A, which is different from `A` and `A.v1`, so it is expected that the
BMI `C.pcm` and `C.v1.pcm` can and should differ.
To fully understand the patch, we need to understand how we encodes
source locations and how we serialize and deserialize them.
For source locations, we encoded them as:
```
|
|
| _____ base offset of an imported module
|
|
|
|_____ base offset of another imported module
|
|
|
|
| ___ 0
```
As the diagram shows, we encode the local (unloaded) source location
from 0 to higher bits. And we allocate the space for source locations
from the loaded modules from high bits to 0. Then the source locations
from the loaded modules will be mapped to our source location space
according to the allocated offset.
For example, for,
```
// a.cppm
export module a;
...
// b.cppm
export module b;
import a;
...
```
Assuming the offset of a source location (let's name the location as
`S`) in a.cppm is 45 and we will record the value `45` into the BMI
`a.pcm`. Then in b.cppm, when we import a, the source manager will
allocate a space for module 'a' (according to the recorded number of
source locations) as the base offset of module 'a' in the current source
location spaces. Let's assume the allocated base offset as 90 in this
example. Then when we want to get the location in the current source
location space for `S`, we can get it simply by adding `45` to `90` to
`135`. Finally we can get the source location for `S` in module B as
`135`.
And when we want to write module `b`, we would also write the source
location of `S` as `135` directly in the BMI. And to clarify the
location `S` comes from module `a`, we also need to record the base
offset of module `a`, 90 in the BMI of `b`.
Then the problem comes. Since the base offset of module 'a' is computed
by the number source locations in module 'a'. In module 'b', the
recorded base offset of module 'a' will change every time the number of
source locations in module 'a' increase or decrease. In other words, the
contents of BMI of B will change every time the number of locations in
module 'a' changes. This is pretty sensitive. Almost every change will
change the number of locations. So this is the problem this patch want
to solve.
Let's continue with the existing design to understand what's going on.
Another interesting case is:
```
// c.cppm
export module c;
import whatever;
import a;
import b;
...
```
In `c.cppm`, when we import `a`, we still need to allocate a base
location offset for it, let's say the value becomes to `200` somehow.
Then when we reach the location `S` recorded in module `b`, we need to
translate it into the current source location space. The solution is
quite simple, we can get it by `135 + (200 - 90) = 245`. In another
word, the offset of a source location in current module can be computed
as `Recorded Offset + Base Offset of the its module file - Recorded Base
Offset`.
Then we're almost done about how we handle the offset of source
locations in serializers.
From the abstract level, what we want to do is to remove the hardcoded
base offset of imported modules and remain the ability to calculate the
source location in a new module unit. To achieve this, we need to be
able to find the module file owning a source location from the encoding
of the source location.
So in this patch, for each source location, we will store the local
offset of the location and the module file index. For the above example,
in `b.pcm`, the source location of `S` will be recorded as `135`
directly. And in the new design, the source location of `S` will be
recorded as `<1, 45>`. Here `1` stands for the module file index of `a`
in module `b`. And `45` means the offset of `S` to the base offset of
module `a`.
So the trade-off here is that, to make the BMI more independent, we need
to record more abstract information. And I feel it is worthy. The
recompilation problem of modules is really annoying and there are still
people complaining this. But if we can make this (including stopping
other changes transitively), I think this may be a killer feature for
modules. And from @Bigcheese , this should be helpful for clang explicit
modules too.
And the benchmarking side, I tested this patch against
https://github.com/alibaba/async_simple/tree/CXX20Modules. No
significant change on compilation time. The size of .pcm files becomes
to 204M from 200M. I think the trade-off is pretty fair.
I didn't use another slot to record the module file index. I tried to
use the higher 32 bits of the existing source location encodings to
store that information. This design may be safe. Since we use `unsigned`
to store source locations but we use uint64_t in serialization. And
generally `unsigned` is 32 bit width in most platforms. So it might not
be a safe problem. Since all the bits we used to store the module file
index is not used before. So the new encodings may be:
```
|-----------------------|-----------------------|
| A | B | C |
* A: 32 bit. The index of the module file in the module manager + 1.
* The +1
here is necessary since we wish 0 stands for the current
module file.
* B: 31 bit. The offset of the source location to the module file
* containing it.
* C: The macro bit. We rotate it to the lowest bit so that we can save
* some
space in case the index of the module file is 0.
```
(The B and C is the existing raw encoding for source locations)
Another reason to reuse the same slot of the source location is to
reduce the impact of the patch. Since there are a lot of places assuming
we can store and get a source location from a slot. And if I tried to
add another slot, a lot of codes breaks. I don't feel it is worhty.
Another impact of this decision is that, the existing small
optimizations for encoding source location may be invalided. The key of
the optimization is that we can turn large values into small values then
we can use VBR6 format to reduce the size. But if we decided to put the
module file index into the higher bits, then maybe it simply doesn't
work. An example may be the `SourceLocationSequence` optimization.
This will only affect the size of on-disk .pcm files. I don't expect
this impact the speed and memory use of compilations. And seeing my
small experiments above, I feel this trade off is worthy.
The mental model for handling source location offsets is not so complex
and I believe we can solve it by adding module file index to each stored
source location.
For the practical side, since the source location is pretty sensitive,
and the patch can pass all the in-tree tests and a small scale projects,
I feel it should be correct.
I'll continue to work on no transitive decl change and no transitive
identifier change (if matters) to achieve the goal to stop the
propagation of unnecessary changes. But all of this depends on this
patch. Since, clearly, the source locations are the most sensitive
thing.
---
The release nots and documentation will be added seperately.
This is part of "no transitive change" patch series, "no transitive
source location change". I talked this with @Bigcheese in the tokyo's
WG21 meeting.
The idea comes from @jyknight posted on LLVM discourse. That for:
```
// A.cppm
export module A;
...
// B.cppm
export module B;
import A;
...
//--- C.cppm
export module C;
import C;
```
Almost every time A.cppm changes, we need to recompile `B`. Due to we
think the source location is significant to the semantics. But it may be
good if we can avoid recompiling `C` if the change from `A` wouldn't
change the BMI of B.
# Motivation Example
This patch only cares source locations. So let's focus on source
location's example. We can see the full example from the attached test.
```
//--- A.cppm
export module A;
export template <class T>
struct C {
T func() {
return T(43);
}
};
export int funcA() {
return 43;
}
//--- A.v1.cppm
export module A;
export template <class T>
struct C {
T func() {
return T(43);
}
};
export int funcA() {
return 43;
}
//--- B.cppm
export module B;
import A;
export int funcB() {
return funcA();
}
//--- C.cppm
export module C;
import A;
export void testD() {
C<int> c;
c.func();
}
```
Here the only difference between `A.cppm` and `A.v1.cppm` is that
`A.v1.cppm` has an additional blank line. Then the test shows that two
BMI of `B.cppm`, one specified `-fmodule-file=A=A.pcm` and the other
specified `-fmodule-file=A=A.v1.pcm`, should have the bit-wise same
contents.
However, it is a different story for C, since C instantiates templates
from A, and the instantiation records the source information from module
A, which is different from `A` and `A.v1`, so it is expected that the
BMI `C.pcm` and `C.v1.pcm` can and should differ.
# Internal perspective of status quo
To fully understand the patch, we need to understand how we encodes
source locations and how we serialize and deserialize them.
For source locations, we encoded them as:
```
|
|
| _____ base offset of an imported module
|
|
|
|_____ base offset of another imported module
|
|
|
|
| ___ 0
```
As the diagram shows, we encode the local (unloaded) source location
from 0 to higher bits. And we allocate the space for source locations
from the loaded modules from high bits to 0. Then the source locations
from the loaded modules will be mapped to our source location space
according to the allocated offset.
For example, for,
```
// a.cppm
export module a;
...
// b.cppm
export module b;
import a;
...
```
Assuming the offset of a source location (let's name the location as
`S`) in a.cppm is 45 and we will record the value `45` into the BMI
`a.pcm`. Then in b.cppm, when we import a, the source manager will
allocate a space for module 'a' (according to the recorded number of
source locations) as the base offset of module 'a' in the current source
location spaces. Let's assume the allocated base offset as 90 in this
example. Then when we want to get the location in the current source
location space for `S`, we can get it simply by adding `45` to `90` to
`135`. Finally we can get the source location for `S` in module B as
`135`.
And when we want to write module `b`, we would also write the source
location of `S` as `135` directly in the BMI. And to clarify the
location `S` comes from module `a`, we also need to record the base
offset of module `a`, 90 in the BMI of `b`.
Then the problem comes. Since the base offset of module 'a' is computed
by the number source locations in module 'a'. In module 'b', the
recorded base offset of module 'a' will change every time the number of
source locations in module 'a' increase or decrease. In other words, the
contents of BMI of B will change every time the number of locations in
module 'a' changes. This is pretty sensitive. Almost every change will
change the number of locations. So this is the problem this patch want
to solve.
Let's continue with the existing design to understand what's going on.
Another interesting case is:
```
// c.cppm
export module c;
import whatever;
import a;
import b;
...
```
In `c.cppm`, when we import `a`, we still need to allocate a base
location offset for it, let's say the value becomes to `200` somehow.
Then when we reach the location `S` recorded in module `b`, we need to
translate it into the current source location space. The solution is
quite simple, we can get it by `135 + (200 - 90) = 245`. In another
word, the offset of a source location in current module can be computed
as `Recorded Offset + Base Offset of the its module file - Recorded Base
Offset`.
Then we're almost done about how we handle the offset of source
locations in serializers.
# The high level design of current patch
From the abstract level, what we want to do is to remove the hardcoded
base offset of imported modules and remain the ability to calculate the
source location in a new module unit. To achieve this, we need to be
able to find the module file owning a source location from the encoding
of the source location.
So in this patch, for each source location, we will store the local
offset of the location and the module file index. For the above example,
in `b.pcm`, the source location of `S` will be recorded as `135`
directly. And in the new design, the source location of `S` will be
recorded as `<1, 45>`. Here `1` stands for the module file index of `a`
in module `b`. And `45` means the offset of `S` to the base offset of
module `a`.
So the trade-off here is that, to make the BMI more independent, we need
to record more abstract information. And I feel it is worthy. The
recompilation problem of modules is really annoying and there are still
people complaining this. But if we can make this (including stopping
other changes transitively), I think this may be a killer feature for
modules. And from @Bigcheese , this should be helpful for clang explicit
modules too.
And the benchmarking side, I tested this patch against
https://github.com/alibaba/async_simple/tree/CXX20Modules. No
significant change on compilation time. The size of .pcm files becomes
to 204M from 200M. I think the trade-off is pretty fair.
# Some low level details
I didn't use another slot to record the module file index. I tried to
use the higher 32 bits of the existing source location encodings to
store that information. This design may be safe. Since we use `unsigned`
to store source locations but we use uint64_t in serialization. And
generally `unsigned` is 32 bit width in most platforms. So it might not
be a safe problem. Since all the bits we used to store the module file
index is not used before. So the new encodings may be:
```
|-----------------------|-----------------------|
| A | B | C |
* A: 32 bit. The index of the module file in the module manager + 1. The +1
here is necessary since we wish 0 stands for the current module file.
* B: 31 bit. The offset of the source location to the module file containing it.
* C: The macro bit. We rotate it to the lowest bit so that we can save some
space in case the index of the module file is 0.
```
(The B and C is the existing raw encoding for source locations)
Another reason to reuse the same slot of the source location is to
reduce the impact of the patch. Since there are a lot of places assuming
we can store and get a source location from a slot. And if I tried to
add another slot, a lot of codes breaks. I don't feel it is worhty.
Another impact of this decision is that, the existing small
optimizations for encoding source location may be invalided. The key of
the optimization is that we can turn large values into small values then
we can use VBR6 format to reduce the size. But if we decided to put the
module file index into the higher bits, then maybe it simply doesn't
work. An example may be the `SourceLocationSequence` optimization.
This will only affect the size of on-disk .pcm files. I don't expect
this impact the speed and memory use of compilations. And seeing my
small experiments above, I feel this trade off is worthy.
# Correctness
The mental model for handling source location offsets is not so complex
and I believe we can solve it by adding module file index to each stored
source location.
For the practical side, since the source location is pretty sensitive,
and the patch can pass all the in-tree tests and a small scale projects,
I feel it should be correct.
# Future Plans
I'll continue to work on no transitive decl change and no transitive
identifier change (if matters) to achieve the goal to stop the
propagation of unnecessary changes. But all of this depends on this
patch. Since, clearly, the source locations are the most sensitive
thing.
---
The release nots and documentation will be added seperately.
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.
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.
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.
This patch adds a `Typename` bit-field to `TemplateTemplateParmDecl`
which stores whether the template template parameter was declared with
the `typename` keyword.
After this patch, we will generate the function body from implicitly
instantiated class. This is important for consumers with same
template arguments. Otherwise the consumers won't see the function body.
Since the consumers won't instantiate the templates again if they find an
instantiation.
Also we will generate the variable definition if the variable is
non-inline but known as constant. Such variables may not affect the
ABI, but they may get involved into the compile time constant computation
in the consumer's code. So we have to generate such definitions.
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.
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.
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.
Close https://github.com/llvm/llvm-project/issues/76443
Previously we assume the bits of the function scope index of ParmVarDecl
won't exceed 8. But this is a misreading. See the implementation of
`ParmVarDecl::getParameterIndex()`, which may
exceed the size of the normal bitfield. So it may be better to not
pack these bits.
bits and extract big bits from packed bits
Previously I tried to improve the size of .pcm files by introducing
packed bits. And I find we can improve it further by reordering the
bits.
The secret comes from the VBR format. We can find the formal definition
of VBR format in the doc of LLVM. The VBR format will be pretty
efficicent for small numbers.
For example, if we need to pack 8 bits into a value and the stored value
is 0xf0, the actual stored value will be 0b000111'110000, which takes 12
bits actually. However, if we changed the order to be 0x0f, then we
can store it as 0b001111, which takes 6 bits only now.
So we can improve the size by placing bits with lower probability to be
1 in the higher bits and extract bit bigs from the packed bits to make
it possible to be optimized by VBR.
After this patch, the size of std module becomes to 27.7MB from 28.1MB.
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.
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 patch tries to reduce the size of the BMIs by packing more bits
into an unsigned integer.
This patch was reverted due to buildbot failure report. But it should be
irrevelent after I took a double look. So I tried to recommit this NFC
change again.
This reverts commit 10951050b5f371eb3e7cacce1691c4eb2fe2eab5.
This should be part of 8c334627818437180176b16b1932 to revert
9406ea3fe32e59a7d2 completely.
Close https://github.com/llvm/llvm-project/issues/71347
Previously I misread the concept of module purview. I thought if a
declaration attached to a unnamed module, it can't be part of the module
purview. But after the issue report, I recognized that module purview is
more of a concept about locations instead of semantics.
Concretely, the things in the language linkage after module declarations
can be exported.
This patch refactors `Module::isModulePurview()` and introduces some
possible code cleanups.
This patch converts `ImplicitParamDecl::ImplicitParamKind` into a scoped enum at namespace scope, making it eligible for forward declaring. This is useful for `preferred_type` annotations on bit-fields.
Previously, the boolean values will occupy spaces that can contain
integers. It wastes the spaces especially if the boolean values are
serialized consecutively. The patch tries to pack such consecutive
boolean values (and enum values) so that we can save more spaces and so
the times.
Before the patch, we need 4.478s (in my machine) to build the std module
(https://libcxx.llvm.org/Modules.html) with 28712 bytes for size of the
BMI. After the patch, the time becomes to 4.374s and the size becomes to
27388 bytes for the size of the BMI.
This is intended to be a NFC patch.
This patch doesn't optimize all such cases. We can do it later after we
have consensus on this.
This patch introduces a new enumerator `Invalid = 0`, shifting other enumerators by +1. Contrary to how it might sound, this actually affirms status quo of how this enum is stored in `clang::Decl`:
```
/// If 0, we have not computed the linkage of this declaration.
/// Otherwise, it is the linkage + 1.
mutable unsigned CacheValidAndLinkage : 3;
```
This patch makes debuggers to not be mistaken about enumerator stored in this bit-field. It also converts `clang::Linkage` to a scoped enum.
This patch converts `LinkageSpecDecl::LanguageIDs` into scoped enum, and moves it to namespace scope, so that it can be forward-declared where required.
This patch moves `ObjCMethodDecl::ImplementationControl` to a DeclBase.h so that it's complete at the point where corresponsing bit-field is declared. This patch also converts it to a scoped enum `clang::ObjCImplementationControl`.
This patch moves `OMPDeclareReductionDecl::InitKind` to DeclBase.h, so that it's complete at the point where corresponding bit-field is declared. This patch also converts it to scoped enum named `OMPDeclareReductionInitKind`
This patch refactor said enums to hold total size of a bit-field, and not just non-inherited bits. This brings `Type` and `DeclContext` in line with `Comment` and `Stmt`. It also makes it unnecessary to list all transitive bases of a bit-field as unnamed bit-fields, which makes it more friendly towards debuggers.
