[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
ChuanqiXu9 wrote:
It looks like the qualified related problems in ODRHash (at least some of them)
are fixed in https://reviews.llvm.org/D156210
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
ChuanqiXu9 wrote:
I had this
https://github.com/llvm/llvm-project/commit/9b808a4beb8e6c8255b412fdd6f5a3e20cbcf270#diff-e88c8434346144fdf335c14c17a112759af7a4e81957efcaca635a08b9f13a29R110-R116.
Do you mean I need to put that into a namespace?
I tried to create alias arguments for template template arguments. But the
tests can't pass before the current patch.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix crash with modules and constexpr destructor (PR #69076)
ChuanqiXu9 wrote: > Well, this patch is up since almost three months now (!). Sure, we can keep > carrying a similar fix downstream, but ideally I would really like to get rid > of as many local changes as possible. That's not possible without proper > review, but the current situation is quite unsatisfactory... Yeah, fully understood. I have a lot of similar experiences... 1~2 weeks is not long in comparing with 3 months. https://github.com/llvm/llvm-project/pull/69076 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix crash with modules and constexpr destructor (PR #69076)
ChuanqiXu9 wrote: > Ping, is this ok to be accepted and landed? If it is not hurry, I prefer to wait @cor3ntin to have a look. But given the scale of the patch, it should be good too to land it in 2 weeks if there is no other comments. https://github.com/llvm/llvm-project/pull/69076 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix crash with modules and constexpr destructor (PR #69076)
@@ -15754,10 +15754,18 @@ bool Expr::EvaluateAsInitializer(APValue ,
const ASTContext ,
LValue LVal;
LVal.set(VD);
-if (!EvaluateInPlace(Value, Info, LVal, this,
- /*AllowNonLiteralTypes=*/true) ||
-EStatus.HasSideEffects)
- return false;
+{
+ // C++23 [intro.execution]/p5
+ // A full-expression is ... an init-declarator ([dcl.decl]) or a
+ // mem-initializer.
+ // So we need to make sure temporary objects are destroyed after having
+ // evaluated the expression (per C++23 [class.temporary]/p4).
ChuanqiXu9 wrote:
```suggestion
// evaluated the expression (per C++23 [class.temporary]/p4).
//
// FIXME: Otherwise this may break test/Modules/pr68702.cpp. because the
serialization code
// calls ParmVarDecl::getDefaultArg() which strips the outermost
FullExpr, such as ExprWithCleanups.
```
I mean the reason why this is related to modules. I feel the analysis is
valuable.
https://github.com/llvm/llvm-project/pull/69076
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
ChuanqiXu9 wrote:
The secret why ODRHash can handle this may live in
https://github.com/llvm/llvm-project/blob/fe1364f1e7ac0c4d0f9a4b15189485782241190d/clang/lib/AST/ODRHash.cpp#L868-L910.
We've already done the work to strip the types.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
ChuanqiXu9 wrote:
I tried to add a test case to show the problem in
https://github.com/llvm/llvm-project/commit/9b808a4beb8e6c8255b412fdd6f5a3e20cbcf270.
But the current patch works well for that. While I agree the ODRHash may be
too aggressive for the problem we're solving, I don't want to write things that
can't be well tested. I am wondering if we can proceed by leaving a FIXME here
if we can't find good test in time? Or maybe we can add an option
`-fload-specialization-lazily`, then we can regress smoothly if there are any
problems.
@hahnjo @vgvassilev
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 9b808a4 - [NFC] [Modules] Add a test case for selecting specializations with aliased template args
Author: Chuanqi Xu
Date: 2024-01-08T14:46:11+08:00
New Revision: 9b808a4beb8e6c8255b412fdd6f5a3e20cbcf270
URL:
https://github.com/llvm/llvm-project/commit/9b808a4beb8e6c8255b412fdd6f5a3e20cbcf270
DIFF:
https://github.com/llvm/llvm-project/commit/9b808a4beb8e6c8255b412fdd6f5a3e20cbcf270.diff
LOG: [NFC] [Modules] Add a test case for selecting specializations with aliased
template args
This a test for https://github.com/llvm/llvm-project/pull/76774. In the
review comments, we're concerning about the case that ODRHash may
produce the different hash values for semantical same template
arguments. For example, if the template argument in a specialization is
not qualified and the semantical same template argument in the instantiation
point is qualified, we should be able to select that template
specialization. And this patch tests this behavior: we should be able to select
the correct specialization with semantical same template arguments.
Added:
clang/test/Modules/explicit-specializations.cppm
Modified:
Removed:
diff --git a/clang/test/Modules/explicit-specializations.cppm
b/clang/test/Modules/explicit-specializations.cppm
new file mode 100644
index 00..914144018e8808
--- /dev/null
+++ b/clang/test/Modules/explicit-specializations.cppm
@@ -0,0 +1,133 @@
+// Testing that the compiler can select the correct template specialization
+// from
diff erent template aliasing.
+//
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/b.cpp -fprebuilt-module-path=%t \
+// RUN: -fsyntax-only -verify
+
+//--- a.cppm
+
+// For template type parameters
+export module a;
+export template
+struct S {
+static constexpr bool selected = false;
+};
+
+export struct A {};
+
+export template <>
+struct S {
+static constexpr bool selected = true;
+};
+
+export using B = A;
+
+// For template template parameters
+
+export template typename C>
+struct V {
+static constexpr bool selected = false;
+};
+
+export template <>
+struct V {
+static constexpr bool selected = true;
+};
+
+// For template non type parameters
+export template
+struct Numbers {
+static constexpr bool selected = false;
+static constexpr int value = X;
+};
+
+export template<>
+struct Numbers<43> {
+static constexpr bool selected = true;
+static constexpr int value = 43;
+};
+
+export template
+struct Pointers {
+static constexpr bool selected = false;
+};
+
+export int IntegralValue = 0;
+export template<>
+struct Pointers<> {
+static constexpr bool selected = true;
+};
+
+export template
+struct NullPointers {
+static constexpr bool selected = false;
+};
+
+export template<>
+struct NullPointers {
+static constexpr bool selected = true;
+};
+
+export template
+struct Array {
+static constexpr bool selected = false;
+};
+
+export int array[5];
+export template<>
+struct Array {
+static constexpr bool selected = true;
+};
+
+//--- b.cpp
+// expected-no-diagnostics
+import a;
+
+// Testing for
diff erent qualifiers
+static_assert(S::selected);
+static_assert(S<::B>::selected);
+static_assert(::S::selected);
+static_assert(::S<::B>::selected);
+typedef A C;
+static_assert(S::selected);
+static_assert(S<::C>::selected);
+static_assert(::S::selected);
+static_assert(::S<::C>::selected);
+
+namespace D {
+C getAType();
+typedef C E;
+}
+
+static_assert(S::selected);
+static_assert(S::selected);
+
+// Testing we can select the correct specialization for
diff erent
+// template template argument alising.
+
+static_assert(V::selected);
+static_assert(V<::S>::selected);
+static_assert(::V::selected);
+static_assert(::V<::S>::selected);
+
+// Testing for template non type parameters
+static_assert(Numbers<43>::selected);
+static_assert(Numbers<21 * 2 + 1>::selected);
+static_assert(Numbers<42 + 1>::selected);
+static_assert(Numbers<44 - 1>::selected);
+static_assert(Numbers::value>::selected);
+static_assert(!Numbers<44>::selected);
+
+static_assert(Pointers<>::selected);
+static_assert(!Pointers::selected);
+static_assert(NullPointers::selected);
+static_assert(!NullPointers<(void*)>::selected);
+
+static_assert(Array::selected);
+int another_array[5];
+static_assert(!Array::selected);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
@@ -0,0 +1,50 @@
+// REQUIRES: !system-windows
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/layer1.cppm -triple %itanium_abi_triple \
+// RUN: -emit-module-interface -o %t/foo-layer1.pcm
+// RUN: %clang_cc1 -std=c++20 %t/layer2.cppm -triple %itanium_abi_triple \
+// RUN: -emit-module-interface -fmodule-file=foo:layer1=%t/foo-layer1.pcm \
+// RUN: -o %t/foo-layer2.pcm
+// RUN: %clang_cc1 -std=c++20 %t/foo-layer1.pcm -S -emit-llvm -o - | FileCheck
%t/layer1.cppm
+// RUN: %clang_cc1 -std=c++20 %t/foo-layer2.pcm -S -emit-llvm -o - \
+// RUN: -fmodule-file=foo:layer1=%t/foo-layer1.pcm | FileCheck
%t/layer2.cppm
+
+//--- layer1.cppm
+export module foo:layer1;
+struct Fruit {
+virtual ~Fruit() = default;
+virtual void eval() = 0;
+};
+struct Banana : public Fruit {
+Banana() {}
+void eval() override;
+};
+
ChuanqiXu9 wrote:
Great suggestion. I caught another bug after I removed `Banana`: previously we
would only generate vtable if it is used. And my draft to fix that is
https://github.com/ChuanqiXu9/llvm-project/commit/f6a22849bb410532c60dca4453fd2b0b71da3994.
It touches Sema and Serializer. So I feel it is a little bit complex even if
it is small. Personally, I prefer to separate that into 2 patches later instead
of combining everything in the current single patch. How do you feel about the
direction? @dwblaikie @rjmccall
https://github.com/llvm/llvm-project/pull/75912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [coroutines] Detect lifetime issues with coroutine lambda captures (PR #77066)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/77066 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [coroutines] Detect lifetime issues with coroutine lambda captures (PR #77066)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/77066 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [coroutines] Detect lifetime issues with coroutine lambda captures (PR #77066)
@@ -7575,15 +7577,27 @@ static void
visitLifetimeBoundArguments(IndirectLocalPath , Expr *Call,
Path.pop_back();
};
- if (ObjectArg && implicitObjectParamIsLifetimeBound(Callee))
-VisitLifetimeBoundArg(Callee, ObjectArg);
-
bool CheckCoroCall = false;
if (const auto *RD = Callee->getReturnType()->getAsRecordDecl()) {
CheckCoroCall = RD->hasAttr() &&
RD->hasAttr() &&
!Callee->hasAttr();
}
+
+ if (ObjectArg) {
+bool CheckCoroObjArg = CheckCoroCall;
+// Ignore `__promise.get_return_object()` as it is not lifetimebound.
+if (CheckCoroObjArg && Callee->getDeclName().isIdentifier() &&
+Callee->getName() == "get_return_object")
+ CheckCoroObjArg = false;
+// Coroutine lambda objects with empty capture list are not lifetimebound.
+if (auto *LE = dyn_cast(ObjectArg->IgnoreImplicit());
+LE && LE->captures().empty())
+ CheckCoroObjArg = false;
ChuanqiXu9 wrote:
I am a little bit surprised that we need to handle this specially. Is this an
optimization? Or it is necessary and why?
https://github.com/llvm/llvm-project/pull/77066
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [coroutines] Detect lifetime issues with coroutine lambda captures (PR #77066)
@@ -7575,15 +7577,27 @@ static void
visitLifetimeBoundArguments(IndirectLocalPath , Expr *Call,
Path.pop_back();
};
- if (ObjectArg && implicitObjectParamIsLifetimeBound(Callee))
-VisitLifetimeBoundArg(Callee, ObjectArg);
-
bool CheckCoroCall = false;
if (const auto *RD = Callee->getReturnType()->getAsRecordDecl()) {
CheckCoroCall = RD->hasAttr() &&
RD->hasAttr() &&
!Callee->hasAttr();
}
+
+ if (ObjectArg) {
+bool CheckCoroObjArg = CheckCoroCall;
+// Ignore `__promise.get_return_object()` as it is not lifetimebound.
+if (CheckCoroObjArg && Callee->getDeclName().isIdentifier() &&
+Callee->getName() == "get_return_object")
+ CheckCoroObjArg = false;
ChuanqiXu9 wrote:
The hardcoded name are not good. What's wrong with it? And if necessary, we'd
better to check it with the built `CoroutineBodyStmt::getReturnStmt` (and maybe
`CoroutineBodyStmt::getReturnStmtOnAllocFailure`?) If that is not built this
time, can we perform that in `Sema::CheckCompletedCoroutineBody`? Maybe that
will require a little bit refactor work, but I guess it may be worthy.
https://github.com/llvm/llvm-project/pull/77066
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [coroutines] Detect lifetime issues with coroutine lambda captures (PR #77066)
https://github.com/ChuanqiXu9 commented: Maybe it'll be better to say this is related to `[[coro_lifetimebound]]`. My instinct reaction to this is that "no, this is not strictly correct". But I feel good after I know it is an extension of `[[coro_lifetimebound]]` only. https://github.com/llvm/llvm-project/pull/77066 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
ChuanqiXu9 wrote:
Great analysis. Fair enough, let's find a method to proceed.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [coroutines] Introduce [[clang::coro_disable_lifetimebound]] (PR #76818)
https://github.com/ChuanqiXu9 approved this pull request. I feel good with this. We can do other improvements in other patches. https://github.com/llvm/llvm-project/pull/76818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
ChuanqiXu9 wrote: > I'm Sam's colleague and wanted to mention that Sam won't be available until > January 15. It probably does not make much sense for someone else to take > over at this point as the change is large and waiting for Sam is more > efficient than ramping up someone else at this point. > > Sorry about the delays. Got it. Yes, there is no many other alternatives to progress. We can only be patience in such cases. https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -1249,3 +1249,5 @@ void ODRHash::AddQualType(QualType T) {
void ODRHash::AddBoolean(bool Value) {
Bools.push_back(Value);
}
+
+void ODRHash::AddInteger(unsigned Value) { ID.AddInteger(Value); }
ChuanqiXu9 wrote:
Interesting. I didn't recognize this. If this is true, we need to decide if we
can leave a FIXME here or we must fix it to proceed.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -2431,10 +2434,14 @@ void
ASTDeclReader::VisitClassTemplateDecl(ClassTemplateDecl *D) {
mergeRedeclarableTemplate(D, Redecl);
if (ThisDeclID == Redecl.getFirstID()) {
-// This ClassTemplateDecl owns a CommonPtr; read it to keep track of all of
-// the specializations.
+// This ClassTemplateDecl owns a CommonPtr; read it to keep track of all
+// of the specializations.
SmallVector SpecIDs;
readDeclIDList(SpecIDs);
+
+if (Record.readInt())
+ ReadDeclsSpecs(*Loc.F, D, Loc.F->DeclsCursor);
ChuanqiXu9 wrote:
Then it won't fall here. It is the job of the latter patch
(https://github.com/ChuanqiXu9/llvm-project/commit/7f027f0b6551a8e421034e96bd0a4c953c473df6)
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -527,6 +527,10 @@ class ASTWriter : public ASTDeserializationListener, bool isLookupResultExternal(StoredDeclsList , DeclContext *DC); bool isLookupResultEntirelyExternal(StoredDeclsList , DeclContext *DC); + uint64_t + WriteSpecsLookupTable(NamedDecl *D, ChuanqiXu9 wrote: Got it. Will do in the next circle. https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
@@ -150,6 +150,11 @@ class ExternalASTSource : public
RefCountedBase {
virtual bool
FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name);
+ /// Load all the external specialzations for the Decl and the corresponding
+ /// template arguments.
+ virtual void LoadExternalSpecs(const Decl *D,
ChuanqiXu9 wrote:
I feel `Load` may be a better name. Since from the signature it doesn't find
anything. And if we want consistency, I suggest to rename
`FindExternalVisibleDeclsByName ` to `LoadExternalVisibleDeclsByName`.
https://github.com/llvm/llvm-project/pull/76774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
ChuanqiXu9 wrote: > This is a great way to start a new year ;) > > The phab link is https://reviews.llvm.org/D41416. > > In general I was wondering could we simplify the implementation by loading > the specialization hash table upon module load. That should be relatively > cheap as we will read 2 integers per specialization. > > Perhaps we should put both patches together and that'd allow us to test them > if they are on par with https://reviews.llvm.org/D41416 which we use > downstream. > > Thanks for working on this! Hi Vassilev, for testing purpose I sent https://github.com/ChuanqiXu9/llvm-project/tree/LoadSpecializationUpdatesLazily. I didn't create stacked review since I feel a standalone branch may be sufficient. > In general I was wondering could we simplify the implementation by loading > the specialization hash table upon module load. That should be relatively > cheap as we will read 2 integers per specialization. IIUC, it looks like what I do in https://github.com/ChuanqiXu9/llvm-project/commit/7f027f0b6551a8e421034e96bd0a4c953c473df6#diff-c61a3cce4bfa099b5af032fa83cbf1563f0af4bf58dc112b39571d74b6b681c1R3487-R3499. But I don't want to do that with this patch. Since we can avoid load the hash table if the template decl is not loaded. https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] f5fd183 - [NFC] [C++20] [Modules] Remove pr60085.cppm with deprecated practice
Author: Chuanqi Xu
Date: 2024-01-05T11:31:22+08:00
New Revision: f5fd1836836e0d37dea61cc842199713cc0e2fc4
URL:
https://github.com/llvm/llvm-project/commit/f5fd1836836e0d37dea61cc842199713cc0e2fc4
DIFF:
https://github.com/llvm/llvm-project/commit/f5fd1836836e0d37dea61cc842199713cc0e2fc4.diff
LOG: [NFC] [C++20] [Modules] Remove pr60085.cppm with deprecated practice
See https://github.com/llvm/llvm-project/issues/60085 for the complete
story.
Previously I thought the problem got fixed surprisingly. But it is not
true. I just tested it with a deprecated method. My bad. Then the
deprecated style should be removed and the proper style can't work. So
I'll remove the test and reopen that issue to look into it.
Added:
Modified:
Removed:
clang/test/Modules/pr60085.cppm
diff --git a/clang/test/Modules/pr60085.cppm b/clang/test/Modules/pr60085.cppm
deleted file mode 100644
index fba60120640471..00
--- a/clang/test/Modules/pr60085.cppm
+++ /dev/null
@@ -1,98 +0,0 @@
-// RUN: rm -rf %t
-// RUN: mkdir %t
-// RUN: split-file %s %t
-//
-// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/d.cppm \
-// RUN: -emit-module-interface -o %t/d.pcm
-// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/c.cppm \
-// RUN: -emit-module-interface -o %t/c.pcm -fmodule-file=%t/d.pcm
-// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cppm \
-// RUN: -emit-module-interface -o %t/b.pcm -fmodule-file=%t/d.pcm
-// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.cppm \
-// RUN: -emit-module-interface -o %t/a.pcm -fmodule-file=%t/d.pcm \
-// RUN: -fmodule-file=%t/c.pcm -fmodule-file=%t/b.pcm
-// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.pcm \
-// RUN: -S -emit-llvm -disable-llvm-passes -o - | FileCheck %t/a.cppm
-//
-// Use -fmodule-file==
-// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/d.cppm \
-// RUN: -emit-module-interface -o %t/d.pcm
-// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/c.cppm \
-// RUN: -emit-module-interface -o %t/c.pcm -fmodule-file=%t/d.pcm
-// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cppm \
-// RUN: -emit-module-interface -o %t/b.pcm -fmodule-file=%t/d.pcm
-// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.cppm \
-// RUN: -emit-module-interface -o %t/a.pcm -fmodule-file=%t/d.pcm \
-// RUN: -fmodule-file=%t/c.pcm -fmodule-file=%t/b.pcm
-// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/a.pcm \
-// RUN: -S -emit-llvm -disable-llvm-passes -o - | FileCheck %t/a.cppm
-
-//--- d.cppm
-export module d;
-
-export template
-struct integer {
- using type = int;
-
- static constexpr auto value() {
- return 0;
- }
-
- friend constexpr void f(integer const x) {
- x.value();
- }
-};
-
-export constexpr void ddd(auto const value) {
- f(value);
-}
-
-
-template
-constexpr auto dd = T();
-
-export template
-constexpr auto d() {
- dd;
-}
-
-//--- c.cppm
-export module c;
-
-import d;
-
-template
-auto cc = T();
-
-auto c() {
- cc>;
- integer().value();
-}
-
-//--- b.cppm
-export module b;
-
-import d;
-
-auto b() {
- integer::type;
-}
-
-//--- a.cppm
-export module a;
-
-import b;
-import c;
-import d;
-
-constexpr void aa() {
- d>();
- ddd(integer());
-}
-
-export extern "C" void a() {
- aa();
-}
-
-// Checks that we emit the IR successfully.
-// CHECK: define{{.*}}@a(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce reduced BMI (PR #75894)
ChuanqiXu9 wrote: > > Like I said in the commit message, this patch itself doesn't involve > > anything relevant to user interfaces. I left it to the latter patches. > > Are you in a position to post the next patch (at least as a draft)? That > would help me see the direction. I post it here https://github.com/ChuanqiXu9/llvm-project/commit/efcc7e888a96e9935a354759de320dea55e93105 since I didn't get how to send stacked pr in github yet. But I guess it might be sufficient since it is really in an early phase. > > > > * I was concerned from earlier conversations that this design might > > > require a codegen back end to be instantiated to allow the reduced BMI > > > (which would be bad for --precompile/-fmodule-only type jobs). Any > > > comments? > > > > > > I am not sure if I understand this. What does it mean "require a codegen > > back end to be instantiated to allow the reduced BMI "? Do you mean to not > > touch CodeGen part or to not touch the CodeGen action? My local patch > > touched the code gen action without touching any real CodeGen related > > things. > > > > the reduced BMI (which would be bad for --precompile/-fmodule-only type > > > jobs) > > > > > > For `--precompile/-fmodule-only` type jobs, I'll create another action to > > make it (Similar to existing `GenerateModuleInterfaceAction`). Then both of > > the actions will try to reuse the same consumer `ReducedBMIGenerator` to > > avoid repeated works. > > OK, that answers my concern (which was that we might have to add the code-gen > backend to a --precompile if that was the mechanism used to do the BMI > reduction). > > > > * It would be better to avoid introducing more layering violations but, > > > as we discussed in face-to-face meetings, I have less concern on the > > > output side. It still seems to me that the best model is one where we > > > have AST transforms (that very likely need Sema to be correct) and then > > > the serializer is a simple as possible. > > > > > > Yeah, it should be less concerned. BTW, currently the simpler > > serializer/deserializer should be ASTRecordWriter/ASTRecordReader. And the > > current ASTReader/ASTWriter takes some semantical job. > > ... and, on the reader side, that already gives us some big problems (as I > say, I am less concerned on the writer side, but who can see the whole > future?). I guess you're referring the process how we decide a declaration is visible? > > > > so something like > > > ``` > > > raw AST + > codegen (when required) > > > | > > > + => AST transforms > BMI output. > > > ``` > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > As I understand the patch you are combining the transform with the output? > > > > > > On the one hand, the **current** patch doesn't do that. The current patch > > is almost a NFC patch. It belongs to the following patch. On the other > > hand, the answer may be yes. Probably I did the `AST transforms` you said > > in the AST Writer. I don't feel it is so awful. > > Maybe not for the short-term relatively simple tasks - but we should also > take a view on the medium and longer term (for example, GMF decl elision is > likely to be helpful to users in reducing both the size of the BMI and the > number of decls that need merging on input). For GMF decl elision, I posted a patch to implement it in ASTWriter and I reposted it in https://github.com/llvm/llvm-project/pull/76930. The big problem is that this is not formal. (Just for sharing, I am not proposing this now) > > We need the AST in this path to be mutable (including removal of decls); that > way each transform can be maintained as a separate entity - I think that if > we end up doing "many transforms" as part of the output, it will become very > confusing. While the model sounds good, I am pessimistic for making it correctly, completely, and efficiently. > > _(although, to be clear, i**n the short-term we might agree to do the work in > the output** - I really do think it would be bad to make that the long term > mechanism)._ Not against, I just think it is not so bad. There already many optimizations in the current serializations. > > > Given all of us loves reduced BMI, I suggest we can fosus on current patch > > then discuss user interfaces related things in the next patch after this > > got landed. > > We do all want to produce the reduced BMI, I agree; but we also always have > limited resources to do the work, so that it would be good to try and pick an > implementation that will be smooth for the future work too. Understood. I just think it won't be too bad. Or it is not easy for us to get a much better solution in the resources we have. I prefer the style that don't make perfect the enemy of better. > > I understand the purpose of the current patch better now - and will try to > take a more detailed look over the
[clang] [C++20] [Modules] Implementing Eliding Unreachable Decls of GMF in ASTWriter (PR #76930)
https://github.com/ChuanqiXu9 converted_to_draft https://github.com/llvm/llvm-project/pull/76930 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Implementing Eliding Unreachable Decls of GMF in ASTWriter (PR #76930)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/76930
This was a patch to try to implement eliding unreachable decls in GMF in
ASTWriter. It was developed a half year ago and I just rebased it but I did't
fix the failing test. It ran well.
The core idea of the patch is that we can implement the idea **reachable** in
ASTWriter naturally.
The secret is that we skip writing GMF initially (generally we will write decls
from the top to the bottom) and we start to write the declarations from module
purview. Then we will only write the declarations in GMF if it is mentioned
during the writing process. So the unreachable decls won't be written natually.
The experience in implementing this patch is pretty smooth and the tests from
the spec can be passed. I felt this should be the natural way to implement this
feature.
The only one and big problem is that we didn't implement the formal semantics
in the spec in this way : |
>From b9a03912276d25ff819a755bef4ee72d64ce1480 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Thu, 4 Jan 2024 17:24:23 +0800
Subject: [PATCH] [C++20] [Modules] Implementing Eliding Unreachable Decls of
GMF in ASTWriter
---
clang/include/clang/AST/DeclBase.h| 16 +-
clang/include/clang/Basic/LangOptions.def | 1 +
clang/include/clang/Driver/Options.td | 7 +
clang/include/clang/Serialization/ASTWriter.h | 24 +++
clang/lib/Sema/SemaModule.cpp | 6 +-
clang/lib/Serialization/ASTReaderDecl.cpp | 3 +
clang/lib/Serialization/ASTWriter.cpp | 160 +-
clang/lib/Serialization/ASTWriterDecl.cpp | 2 +
.../basic.scope/basic.scope.namespace/p2.cpp | 6 +-
.../module.glob.frag/cxx20-10-4-ex2.cppm | 72
clang/test/CXX/module/module.import/p2.cpp| 7 +-
.../test/CodeGenCXX/module-intializer-pmf.cpp | 6 +-
clang/test/CodeGenCXX/module-intializer.cpp | 39 ++---
clang/test/Modules/abi-tag.cppm | 69
clang/test/Modules/concept.cppm | 2 +
.../explicitly-specialized-template.cpp | 2 +-
.../inconsistent-deduction-guide-linkage.cppm | 6 +
clang/test/Modules/named-modules-adl-2.cppm | 2 +
clang/test/Modules/named-modules-adl.cppm | 2 +
clang/test/Modules/polluted-operator.cppm | 11 +-
clang/test/Modules/pr58716.cppm | 2 +
clang/test/Modules/pr60775.cppm | 4 +
clang/test/Modules/pr62589.cppm | 3 +
clang/test/Modules/preferred_name.cppm| 2 +
.../redundant-template-default-arg3.cpp | 9 +
.../template-function-specialization.cpp | 6 +-
26 files changed, 416 insertions(+), 53 deletions(-)
create mode 100644 clang/test/CXX/module/module.glob.frag/cxx20-10-4-ex2.cppm
create mode 100644 clang/test/Modules/abi-tag.cppm
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 10dcbdb262d84e..523b930d59645a 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -233,9 +233,12 @@ class alignas(8) Decl {
/// This declaration has an owning module, but is only visible to
/// lookups that occur within that module.
-/// The discarded declarations in global module fragment belongs
-/// to this group too.
-ModulePrivate
+ModulePrivate,
+
+/// This declaration is part of a Global Module Fragment, it is permitted
+/// to discard it and therefore it is not reachable or visible to importers
+/// of the named module of which the GMF is part.
+ModuleDiscardable
};
protected:
@@ -658,9 +661,10 @@ class alignas(8) Decl {
/// Whether this declaration comes from another module unit.
bool isInAnotherModuleUnit() const;
- /// FIXME: Implement discarding declarations actually in global module
- /// fragment. See [module.global.frag]p3,4 for details.
- bool isDiscardedInGlobalModuleFragment() const { return false; }
+ /// See [module.global.frag]p3,4 for details.
+ bool isDiscardedInGlobalModuleFragment() const {
+return getModuleOwnershipKind() == ModuleOwnershipKind::ModuleDiscardable;
+ }
/// Return true if this declaration has an attribute which acts as
/// definition of the entity, such as 'alias' or 'ifunc'.
diff --git a/clang/include/clang/Basic/LangOptions.def
b/clang/include/clang/Basic/LangOptions.def
index 21abc346cf17ac..97ba8147bdfbb5 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -178,6 +178,7 @@ LANGOPT(BuiltinHeadersInSystemModules, 1, 0, "builtin
headers belong to system m
BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 3, CMK_None,
"compiling a module interface")
BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
+LANGOPT(DiscardGMFDecls , 1, 1, "Discard unreachable decls in GMF")
BENIGN_LANGOPT(BuildingPCHWithObjectFile, 1, 0, "building a pch which has a
corresponding
[clang] [C++20] [Modules] Introduce reduced BMI (PR #75894)
ChuanqiXu9 wrote: > @ChuanqiXu9 very sorry for the slow review. It would help me if the design > was described in the commit message instead of trying to deduce it from the > patch (maybe it's in a thread somewhere - so a cross-reference would help). hi @iains , sorry for the confusion. It may be hard to balance how detail the message it is. So it is no problem at all to ask questions as long as it need. This should be the point of the review process too. For this patch itself, the design is to introduce a CC1 command line `-emit-reduced-module-interface` and the corrresponding action `GenerateReducedModuleInterfaceAction`. Both of them are aimed to help testing reduced BMI instead of being used by end users actually. Then it might be straightforward to get it by seeing the implementation of `GenerateReducedModuleInterfaceAction`. Like I said in the commit message, this patch itself doesn't involve anything relevant to user interfaces. I left it to the latter patches. > > two immediate questions and one observation: > > * I see you are using a multiplex consumer (actually, for some reason, I > thought you were objecting to that part of my design); In fact, I am not objecting your design due to multiplex consumer. I am objecting your design in implementing it in `FrontendAction::CreateWrappedASTConsumer()`. I think this is not the correct place to introduce language or module specific things. > does this mean that your proposed solution can emit both the object and the > reduced BMI from a single compilation job? Yes, this is the goal. I've already had a patch in the downstream to do that. But I am still wondering if I can implement it better. > * I was concerned from earlier conversations that this design might require a > codegen back end to be instantiated to allow the reduced BMI (which would be > bad for --precompile/-fmodule-only type jobs). Any comments? I am not sure if I understand this. What does it mean "require a codegen back end to be instantiated to allow the reduced BMI "? Do you mean to not touch CodeGen part or to not touch the CodeGen action? My local patch touched the code gen action without touching any real CodeGen related things. > the reduced BMI (which would be bad for --precompile/-fmodule-only type jobs) For `--precompile/-fmodule-only` type jobs, I'll create another action to make it (Similar to existing `GenerateModuleInterfaceAction`). Then both of the actions will try to reuse the same consumer `ReducedBMIGenerator` to avoid repeated works. > * It would be better to avoid introducing more layering violations but, as we > discussed in face-to-face meetings, I have less concern on the output side. > It still seems to me that the best model is one where we have AST transforms > (that very likely need Sema to be correct) and then the serializer is a > simple as possible. Yeah, it should be less concerned. BTW, currently the simpler serializer/deserializer should be ASTRecordWriter/ASTRecordReader. And the current ASTReader/ASTWriter takes some semantical job. > > so something like > > ``` > raw AST + > codegen (when required) > | > + => AST transforms > BMI output. > ``` > > As I understand the patch you are combining the transform with the output? On the one hand, the **current** patch doesn't do that. The current patch is almost a NFC patch. It belongs to the following patch. On the other hand, the answer may be yes. Probably I did the `AST transforms` you said in the AST Writer. I don't feel it is so awful. --- Given all of us loves reduced BMI, I suggest we can focus on current patch then discuss user interfaces related things in the next patch after this got landed. https://github.com/llvm/llvm-project/pull/75894 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/76774
>From 79cefc9f0f006acd788b6ac4e240c17d9deadf13 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 3 Jan 2024 11:33:17 +0800
Subject: [PATCH] Load Specializations Lazily
---
clang/include/clang/AST/DeclTemplate.h| 35 ++--
clang/include/clang/AST/ExternalASTSource.h | 5 +
clang/include/clang/AST/ODRHash.h | 3 +
.../clang/Sema/MultiplexExternalSemaSource.h | 6 +
.../include/clang/Serialization/ASTBitCodes.h | 3 +
clang/include/clang/Serialization/ASTReader.h | 19 +++
clang/include/clang/Serialization/ASTWriter.h | 6 +
clang/lib/AST/DeclTemplate.cpp| 66 +---
clang/lib/AST/ExternalASTSource.cpp | 5 +
clang/lib/AST/ODRHash.cpp | 2 +
.../lib/Sema/MultiplexExternalSemaSource.cpp | 6 +
clang/lib/Serialization/ASTReader.cpp | 109 +++-
clang/lib/Serialization/ASTReaderDecl.cpp | 34 +++-
clang/lib/Serialization/ASTReaderInternals.h | 80 +
clang/lib/Serialization/ASTWriter.cpp | 149 +++-
clang/lib/Serialization/ASTWriterDecl.cpp | 75 ++---
clang/test/Modules/odr_hash.cpp | 4 +-
clang/unittests/Serialization/CMakeLists.txt | 1 +
.../Serialization/LoadSpecLazily.cpp | 159 ++
19 files changed, 702 insertions(+), 65 deletions(-)
create mode 100644 clang/unittests/Serialization/LoadSpecLazily.cpp
diff --git a/clang/include/clang/AST/DeclTemplate.h
b/clang/include/clang/AST/DeclTemplate.h
index 832ad2de6b08a8..4699dd17bc182c 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -525,8 +525,11 @@ class FunctionTemplateSpecializationInfo final
return Function.getInt();
}
+ void loadExternalRedecls();
+
public:
friend TrailingObjects;
+ friend class ASTReader;
static FunctionTemplateSpecializationInfo *
Create(ASTContext , FunctionDecl *FD, FunctionTemplateDecl *Template,
@@ -789,13 +792,15 @@ class RedeclarableTemplateDecl : public TemplateDecl,
return SpecIterator(isEnd ? Specs.end() : Specs.begin());
}
- void loadLazySpecializationsImpl() const;
+ void loadExternalSpecializations() const;
template
typename SpecEntryTraits::DeclType*
findSpecializationImpl(llvm::FoldingSetVector ,
void *, ProfileArguments &&...ProfileArgs);
+ void loadLazySpecializationsWithArgs(ArrayRef
TemplateArgs);
+
template
void addSpecializationImpl(llvm::FoldingSetVector ,
EntryType *Entry, void *InsertPos);
@@ -814,9 +819,13 @@ class RedeclarableTemplateDecl : public TemplateDecl,
/// If non-null, points to an array of specializations (including
/// partial specializations) known only by their external declaration IDs.
///
+/// These specializations needs to be loaded at once in
+/// loadExternalSpecializations to complete the redecl chain or be
preparing
+/// for template resolution.
+///
/// The first value in the array is the number of specializations/partial
/// specializations that follow.
-uint32_t *LazySpecializations = nullptr;
+uint32_t *ExternalSpecializations = nullptr;
/// The set of "injected" template arguments used within this
/// template.
@@ -850,6 +859,8 @@ class RedeclarableTemplateDecl : public TemplateDecl,
friend class ASTDeclWriter;
friend class ASTReader;
template friend class RedeclarableTemplate;
+ friend class ClassTemplateSpecializationDecl;
+ friend class VarTemplateSpecializationDecl;
/// Retrieves the canonical declaration of this template.
RedeclarableTemplateDecl *getCanonicalDecl() override {
@@ -977,6 +988,7 @@ SpecEntryTraits {
class FunctionTemplateDecl : public RedeclarableTemplateDecl {
protected:
friend class FunctionDecl;
+ friend class FunctionTemplateSpecializationInfo;
/// Data that is common to all of the declarations of a given
/// function template.
@@ -1012,13 +1024,13 @@ class FunctionTemplateDecl : public
RedeclarableTemplateDecl {
void addSpecialization(FunctionTemplateSpecializationInfo* Info,
void *InsertPos);
+ /// Load any lazily-loaded specializations from the external source.
+ void LoadLazySpecializations() const;
+
public:
friend class ASTDeclReader;
friend class ASTDeclWriter;
- /// Load any lazily-loaded specializations from the external source.
- void LoadLazySpecializations() const;
-
/// Get the underlying function declaration of the template.
FunctionDecl *getTemplatedDecl() const {
return static_cast(TemplatedDecl);
@@ -1839,6 +1851,8 @@ class ClassTemplateSpecializationDecl
LLVM_PREFERRED_TYPE(TemplateSpecializationKind)
unsigned SpecializationKind : 3;
+ void loadExternalRedecls();
+
protected:
ClassTemplateSpecializationDecl(ASTContext , Kind DK, TagKind
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/76774
>From af6f8ca9b739c532a489881245fac1413ec84a07 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 3 Jan 2024 11:33:17 +0800
Subject: [PATCH] Load Specializations Lazily
---
clang/include/clang/AST/DeclTemplate.h| 37 ++--
clang/include/clang/AST/ExternalASTSource.h | 5 +
clang/include/clang/AST/ODRHash.h | 3 +
.../clang/Sema/MultiplexExternalSemaSource.h | 6 +
.../include/clang/Serialization/ASTBitCodes.h | 3 +
clang/include/clang/Serialization/ASTReader.h | 19 +++
clang/include/clang/Serialization/ASTWriter.h | 6 +
clang/lib/AST/DeclTemplate.cpp| 66 +---
clang/lib/AST/ExternalASTSource.cpp | 5 +
clang/lib/AST/ODRHash.cpp | 2 +
.../lib/Sema/MultiplexExternalSemaSource.cpp | 6 +
clang/lib/Serialization/ASTReader.cpp | 109 +++-
clang/lib/Serialization/ASTReaderDecl.cpp | 34 +++-
clang/lib/Serialization/ASTReaderInternals.h | 80 +
clang/lib/Serialization/ASTWriter.cpp | 149 +++-
clang/lib/Serialization/ASTWriterDecl.cpp | 75 ++---
clang/test/Modules/odr_hash.cpp | 4 +-
clang/unittests/Serialization/CMakeLists.txt | 1 +
.../Serialization/LoadSpecLazily.cpp | 159 ++
19 files changed, 704 insertions(+), 65 deletions(-)
create mode 100644 clang/unittests/Serialization/LoadSpecLazily.cpp
diff --git a/clang/include/clang/AST/DeclTemplate.h
b/clang/include/clang/AST/DeclTemplate.h
index 832ad2de6b08a8..3e0a89a92b020d 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -525,8 +525,11 @@ class FunctionTemplateSpecializationInfo final
return Function.getInt();
}
+ void loadExternalRedecls();
+
public:
friend TrailingObjects;
+ friend class ASTReader;
static FunctionTemplateSpecializationInfo *
Create(ASTContext , FunctionDecl *FD, FunctionTemplateDecl *Template,
@@ -789,13 +792,15 @@ class RedeclarableTemplateDecl : public TemplateDecl,
return SpecIterator(isEnd ? Specs.end() : Specs.begin());
}
- void loadLazySpecializationsImpl() const;
+ void loadExternalSpecializations() const;
template
typename SpecEntryTraits::DeclType*
findSpecializationImpl(llvm::FoldingSetVector ,
void *, ProfileArguments &&...ProfileArgs);
+ void loadLazySpecializationsWithArgs(ArrayRef
TemplateArgs);
+
template
void addSpecializationImpl(llvm::FoldingSetVector ,
EntryType *Entry, void *InsertPos);
@@ -814,9 +819,13 @@ class RedeclarableTemplateDecl : public TemplateDecl,
/// If non-null, points to an array of specializations (including
/// partial specializations) known only by their external declaration IDs.
///
+/// These specializations needs to be loaded at once in
+/// loadExternalSpecializations to complete the redecl chain or be
preparing
+/// for template resolution.
+///
/// The first value in the array is the number of specializations/partial
/// specializations that follow.
-uint32_t *LazySpecializations = nullptr;
+uint32_t *ExternalSpecializations = nullptr;
/// The set of "injected" template arguments used within this
/// template.
@@ -850,6 +859,8 @@ class RedeclarableTemplateDecl : public TemplateDecl,
friend class ASTDeclWriter;
friend class ASTReader;
template friend class RedeclarableTemplate;
+ friend class ClassTemplateSpecializationDecl;
+ friend class VarTemplateSpecializationDecl;
/// Retrieves the canonical declaration of this template.
RedeclarableTemplateDecl *getCanonicalDecl() override {
@@ -977,6 +988,7 @@ SpecEntryTraits {
class FunctionTemplateDecl : public RedeclarableTemplateDecl {
protected:
friend class FunctionDecl;
+ friend class FunctionTemplateSpecializationInfo;
/// Data that is common to all of the declarations of a given
/// function template.
@@ -1012,13 +1024,13 @@ class FunctionTemplateDecl : public
RedeclarableTemplateDecl {
void addSpecialization(FunctionTemplateSpecializationInfo* Info,
void *InsertPos);
+ /// Load any lazily-loaded specializations from the external source.
+ void LoadLazySpecializations() const;
+
public:
friend class ASTDeclReader;
friend class ASTDeclWriter;
- /// Load any lazily-loaded specializations from the external source.
- void LoadLazySpecializations() const;
-
/// Get the underlying function declaration of the template.
FunctionDecl *getTemplatedDecl() const {
return static_cast(TemplatedDecl);
@@ -1839,6 +1851,8 @@ class ClassTemplateSpecializationDecl
LLVM_PREFERRED_TYPE(TemplateSpecializationKind)
unsigned SpecializationKind : 3;
+ void loadExternalRedecls();
+
protected:
ClassTemplateSpecializationDecl(ASTContext , Kind DK, TagKind
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/76774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Load Specializations Lazily (1/2) (PR #76774)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/76774
The idea comes from @vgvassilev and @vgvassilev had patch for it on phab.
Unfortunately phab is closed and I forgot the Dxxx number of that patch. But I
remember the last comment from @vgvassilev is that we should use
MultiOnDiskHashTable for it. So I followed that and rewrite the whole from the
scratch in the new year.
### Background
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 A {};
export class ShouldNotBeLoaded {};
export class Temp {
A AS;
};
// use.cpp
import M;
A a;
```
We should a specialization ` A` 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.
### What this patch did
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.
The partial specializations are not added to the MultiOnDiskHashTable. Since we
can't know if a partial specialization is needed before deciding the template
declaration for a instantiation request. There may be space for further
optimizations, but let's do that in the future.
To review this patch, I think `ASTReaderDecl::AddLazySpecializations` may be a
good entry point.
### What this patch not did
This patch doesn't solve the problem completely. Since we will add `update`
specializations if there are new specializations in a different module:
https://github.com/llvm/llvm-project/blob/8ae73fea3a2cbb072bf3e577dc49deb25b56e760/clang/lib/Serialization/ASTWriterDecl.cpp#L251-L269
That said, we can't handle this case now:
```
// M.cppm
export module M;
export template
class A {};
// N.cppm
export module N;
export import A;
export class ShouldNotBeLoaded {};
export class Temp {
A AS;
};
// use.cpp
import N;
A a;
```
Now `ShouldNotBeLoaded` will still be loaded.
But the current patch is already relatively big. So I want to split it in the
next patch. I think the current patch is already self contained.
>From f4191661961428a0f534f527774ac3d5159c5103 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Tue, 2 Jan 2024 10:43:03 +0800
Subject: [PATCH] Load Specializations Lazily
---
clang/include/clang/AST/DeclTemplate.h| 51 --
clang/include/clang/AST/ExternalASTSource.h | 5 +
clang/include/clang/AST/ODRHash.h | 3 +
.../clang/Sema/MultiplexExternalSemaSource.h | 6 +
.../include/clang/Serialization/ASTBitCodes.h | 3 +
clang/include/clang/Serialization/ASTReader.h | 19 +++
clang/include/clang/Serialization/ASTWriter.h | 6 +
clang/lib/AST/DeclTemplate.cpp| 66 +---
clang/lib/AST/ExternalASTSource.cpp | 5 +
clang/lib/AST/ODRHash.cpp | 2 +
.../lib/Sema/MultiplexExternalSemaSource.cpp | 6 +
clang/lib/Serialization/ASTReader.cpp | 109 +++-
clang/lib/Serialization/ASTReaderDecl.cpp | 33 +++-
clang/lib/Serialization/ASTReaderInternals.h | 80 +
clang/lib/Serialization/ASTWriter.cpp | 149 +++-
clang/lib/Serialization/ASTWriterDecl.cpp | 75 ++---
clang/test/Modules/odr_hash.cpp | 4 +-
.../Modules/static-member-in-templates.cppm | 52 ++
clang/unittests/Serialization/CMakeLists.txt | 1 +
.../Serialization/LoadSpecLazily.cpp | 159 ++
20 files changed, 769 insertions(+), 65 deletions(-)
create mode 100644 clang/test/Modules/static-member-in-templates.cppm
create mode 100644 clang/unittests/Serialization/LoadSpecLazily.cpp
diff --git a/clang/include/clang/AST/DeclTemplate.h
b/clang/include/clang/AST/DeclTemplate.h
index 832ad2de6b08a8..ab380f55c038ee 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -30,6 +30,7 @@
#include "llvm/ADT/FoldingSet.h"
#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/iterator.h"
#include "llvm/ADT/iterator_range.h"
#include "llvm/Support/Casting.h"
@@ -525,8 +526,11 @@ class
[clang] [C++20] [Modules] Introduce reduced BMI (PR #75894)
ChuanqiXu9 wrote: @iains @dwblaikie ping~ https://github.com/llvm/llvm-project/pull/75894 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
ChuanqiXu9 wrote: @dwblaikie @rjmccall ping~ https://github.com/llvm/llvm-project/pull/75912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
ChuanqiXu9 wrote: @sam-mccall ping~ what's the fate of the patch? https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Print library module manifest path. (PR #76451)
@@ -2164,6 +2164,12 @@ bool Driver::HandleImmediateArgs(const Compilation ) {
return false;
}
+ if (C.getArgs().hasArg(options::OPT_print_library_module_manifest_path)) {
+llvm::outs() << "module: ="
ChuanqiXu9 wrote:
But "module: =" doesn't look like an error value too. And I feel it just adds
the work the tool vendors need to do. @mathstuf @boris-kolpackov do you have an
opinion here?
https://github.com/llvm/llvm-project/pull/76451
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Print library module manifest path. (PR #76451)
@@ -2164,6 +2164,12 @@ bool Driver::HandleImmediateArgs(const Compilation ) {
return false;
}
+ if (C.getArgs().hasArg(options::OPT_print_library_module_manifest_path)) {
+llvm::outs() << "module: ="
ChuanqiXu9 wrote:
I don't get why we can't just return empty string if we can't find it.
https://github.com/llvm/llvm-project/pull/76451
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Print library module manifest path. (PR #76451)
@@ -0,0 +1,15 @@ +// Test that -print-library-module-manifest-path finds the correct file. ChuanqiXu9 wrote: Let's change the suffix of this file to `.cpp` https://github.com/llvm/llvm-project/pull/76451 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Print library module manifest path. (PR #76451)
@@ -0,0 +1,9 @@ +// Test that -print-library-module-manifest-path finds the correct file. ChuanqiXu9 wrote: Let's change the suffix. https://github.com/llvm/llvm-project/pull/76451 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Print library module manifest path. (PR #76451)
@@ -602,6 +602,16 @@ class Driver {
// FIXME: This should be in CompilationInfo.
std::string GetProgramPath(StringRef Name, const ToolChain ) const;
+ /// GetModuleManifestPath - Lookup the name of the Standard library manifest.
+ ///
+ /// \param C - The compilation.
+ /// \param TC - The tool chain for additional information on
+ /// directories to search.
+ //
+ // FIXME: This should be in CompilationInfo.
ChuanqiXu9 wrote:
Got it. I didn't look into the context.
https://github.com/llvm/llvm-project/pull/76451
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 52770d8 - [Serialization] Don't pack bits for the function scope index of ParmVarDecl
Author: Chuanqi Xu
Date: 2023-12-28T11:04:11+08:00
New Revision: 52770d83bf00fc56e9496e32f083f0f940bf7315
URL:
https://github.com/llvm/llvm-project/commit/52770d83bf00fc56e9496e32f083f0f940bf7315
DIFF:
https://github.com/llvm/llvm-project/commit/52770d83bf00fc56e9496e32f083f0f940bf7315.diff
LOG: [Serialization] Don't pack bits for the function scope index of ParmVarDecl
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.
Added:
clang/test/PCH/pr76443.cpp
Modified:
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
Removed:
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index d989707d557524..547eb77930b4ee 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1706,10 +1706,10 @@ void
ASTDeclReader::VisitImplicitParamDecl(ImplicitParamDecl *PD) {
void ASTDeclReader::VisitParmVarDecl(ParmVarDecl *PD) {
VisitVarDecl(PD);
+ unsigned scopeIndex = Record.readInt();
BitsUnpacker ParmVarDeclBits(Record.readInt());
unsigned isObjCMethodParam = ParmVarDeclBits.getNextBit();
unsigned scopeDepth = ParmVarDeclBits.getNextBits(/*Width=*/7);
- unsigned scopeIndex = ParmVarDeclBits.getNextBits(/*Width=*/8);
unsigned declQualifier = ParmVarDeclBits.getNextBits(/*Width=*/7);
if (isObjCMethodParam) {
assert(scopeDepth == 0);
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp
b/clang/lib/Serialization/ASTWriterDecl.cpp
index 2554abc682a1d6..9e3299f0491848 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1163,10 +1163,14 @@ void
ASTDeclWriter::VisitImplicitParamDecl(ImplicitParamDecl *D) {
void ASTDeclWriter::VisitParmVarDecl(ParmVarDecl *D) {
VisitVarDecl(D);
+ // 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.
+ Record.push_back(D->getFunctionScopeIndex());
+
BitsPacker ParmVarDeclBits;
ParmVarDeclBits.addBit(D->isObjCMethodParameter());
ParmVarDeclBits.addBits(D->getFunctionScopeDepth(), /*BitsWidth=*/7);
- ParmVarDeclBits.addBits(D->getFunctionScopeIndex(), /*BitsWidth=*/8);
// FIXME: stable encoding
ParmVarDeclBits.addBits(D->getObjCDeclQualifier(), /*BitsWidth=*/7);
ParmVarDeclBits.addBit(D->isKNRPromoted());
@@ -2350,10 +2354,11 @@ void ASTWriter::WriteDeclAbbrevs() {
// isARCPseudoStrong, Linkage, ModulesCodegen
Abv->Add(BitCodeAbbrevOp(0)); // VarKind (local
enum)
// ParmVarDecl
+ Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // ScopeIndex
Abv->Add(BitCodeAbbrevOp(
BitCodeAbbrevOp::Fixed,
- 27)); // Packed Parm Var Decl bits: IsObjCMethodParameter, ScopeDepth,
-// ScopeIndex, ObjCDeclQualifier, KNRPromoted,
+ 19)); // Packed Parm Var Decl bits: IsObjCMethodParameter, ScopeDepth,
+// ObjCDeclQualifier, KNRPromoted,
// HasInheritedDefaultArg, HasUninstantiatedDefaultArg
// Type Source Info
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
diff --git a/clang/test/PCH/pr76443.cpp b/clang/test/PCH/pr76443.cpp
new file mode 100644
index 00..5b3e23de7da0d0
--- /dev/null
+++ b/clang/test/PCH/pr76443.cpp
@@ -0,0 +1,24 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+//
+// RUN: %clang_cc1 -std=c++17 -emit-pch %s -o %t/h.pcm
+
+//--- header.h
+template int stringData(const char (&...x)[Nx]) {
+ return 0;
+}
+int qt_meta_stringdata_CLASSQStyleENDCLASS = stringData(
+"", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "",
+"", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "",
+"", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "",
+"", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "",
+"", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "",
+"", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "",
+"", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "",
+"", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "",
+"", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "",
+"", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "",
+"", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "",
+"", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "",
+"", "",
[libcxx] [clang] [libc++] Re-enable the clang_modules_include test for Objective-C++ (PR #66801)
ChuanqiXu9 wrote: I tried to fix this in https://github.com/llvm/llvm-project/commit/c2c840bd92cfac155f6205ff7505b109b301d389. Sorry for disturbing. https://github.com/llvm/llvm-project/pull/66801 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] c2c840b - [Modules] Don't prevent @import from ObjectiveC
Author: Chuanqi Xu
Date: 2023-12-28T10:45:47+08:00
New Revision: c2c840bd92cfac155f6205ff7505b109b301d389
URL:
https://github.com/llvm/llvm-project/commit/c2c840bd92cfac155f6205ff7505b109b301d389
DIFF:
https://github.com/llvm/llvm-project/commit/c2c840bd92cfac155f6205ff7505b109b301d389.diff
LOG: [Modules] Don't prevent @import from ObjectiveC
Previously we forbiden the users to import named modules from clang header
modules. However, due to an oversight, the @import form of Objective C
got involved. This is not want and we fix that in this patch.
Added:
Modified:
clang/lib/Sema/SemaModule.cpp
clang/test/Modules/pr64755.cppm
Removed:
diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index db0cbd5ec6d6ca..ed7f626971f345 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -529,7 +529,8 @@ DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
if (!Mod)
return true;
- if (!Mod->isInterfaceOrPartition() && !ModuleName.empty()) {
+ if (!Mod->isInterfaceOrPartition() && !ModuleName.empty() &&
+ !getLangOpts().ObjC) {
Diag(ImportLoc, diag::err_module_import_non_interface_nor_parition)
<< ModuleName;
return true;
diff --git a/clang/test/Modules/pr64755.cppm b/clang/test/Modules/pr64755.cppm
index 75ef843154610d..2d656868eb60bb 100644
--- a/clang/test/Modules/pr64755.cppm
+++ b/clang/test/Modules/pr64755.cppm
@@ -7,6 +7,11 @@
// RUN: %clang_cc1 -std=c++20 %t/use.cpp -fmodule-file=a0=%t/a0.pcm -verify
-fsyntax-only
// RUN: %clang_cc1 -std=c++20 %t/use.cpp -fprebuilt-module-path=%t -verify
-fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -fmodules -fimplicit-module-maps
-fmodules-cache-path=%t -I%t -fmodule-name=a0 -x objective-c++ -emit-module
%t/module.modulemap -o %t/a0.pcm
+// RUN: %clang_cc1 -std=c++20 -x objective-c++ %t/use_obj.cpp
-fmodule-file=%t/a0.pcm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -x objective-c++ %t/use_obj.cpp
-fmodule-file=a0=%t/a0.pcm -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 -x objective-c++ %t/use_obj.cpp
-fprebuilt-module-path=%t -verify -fsyntax-only
+
//--- module.modulemap
module a0 { header "a0.h" export * }
@@ -15,3 +20,7 @@ void a0() {}
//--- use.cpp
import a0; // expected-error {{import of module 'a0' imported non C++20
importable modules}}
+
+//--- use_obj.cpp
+// expected-no-diagnostics
+@import a0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Print library module manifest path. (PR #76451)
@@ -602,6 +602,16 @@ class Driver {
// FIXME: This should be in CompilationInfo.
std::string GetProgramPath(StringRef Name, const ToolChain ) const;
+ /// GetModuleManifestPath - Lookup the name of the Standard library manifest.
+ ///
+ /// \param C - The compilation.
+ /// \param TC - The tool chain for additional information on
+ /// directories to search.
+ //
+ // FIXME: This should be in CompilationInfo.
ChuanqiXu9 wrote:
If possible, let's explain the reason why we choose to put it here.
https://github.com/llvm/llvm-project/pull/76451
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Print library module manifest path. (PR #76451)
@@ -2164,6 +2164,12 @@ bool Driver::HandleImmediateArgs(const Compilation ) {
return false;
}
+ if (C.getArgs().hasArg(options::OPT_print_library_module_manifest_path)) {
+llvm::outs() << "module: ="
ChuanqiXu9 wrote:
Do we need the prefix? I am wondering if the plain path are good enough.
https://github.com/llvm/llvm-project/pull/76451
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Print library module manifest path. (PR #76451)
@@ -0,0 +1,15 @@
+// Test that -print-library-module-manifest-path finds the correct file.
+
+// RUN: %clang -print-library-module-manifest-path \
+// RUN: -stdlib=libc++ \
+// RUN: --sysroot=%S/Inputs/cxx23_modules \
+// RUN: --target=x86_64-linux-gnu 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-LIBCXX %s
+// CHECK-LIBCXX: module:
={{.*}}/Inputs/cxx23_modules/usr/lib/x86_64-linux-gnu/modules.json
+
ChuanqiXu9 wrote:
Is there a path for libc++ like `--gcc-toolchain=` so that we can choose the
different locations for libstdc++?
https://github.com/llvm/llvm-project/pull/76451
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Print library module manifest path. (PR #76451)
@@ -602,6 +602,16 @@ class Driver {
// FIXME: This should be in CompilationInfo.
std::string GetProgramPath(StringRef Name, const ToolChain ) const;
+ /// GetModuleManifestPath - Lookup the name of the Standard library manifest.
+ ///
+ /// \param C - The compilation.
+ /// \param TC - The tool chain for additional information on
+ /// directories to search.
+ //
+ // FIXME: This should be in CompilationInfo.
+ std::string GetModuleManifestPath(const Compilation ,
ChuanqiXu9 wrote:
```suggestion
std::string GetStdModuleManifestPath(const Compilation ,
```
https://github.com/llvm/llvm-project/pull/76451
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Print library module manifest path. (PR #76451)
@@ -5280,6 +5280,9 @@ def print_resource_dir : Flag<["-", "--"], "print-resource-dir">, def print_search_dirs : Flag<["-", "--"], "print-search-dirs">, HelpText<"Print the paths used for finding libraries and programs">, Visibility<[ClangOption, CLOption]>; +def print_library_module_manifest_path : Flag<["-", "--"], "print-library-module-manifest-path">, ChuanqiXu9 wrote: ```suggestion def print_std_module_manifest_path : Flag<["-", "--"], "print-std-module-manifest-path">, ``` https://github.com/llvm/llvm-project/pull/76451 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Print library module manifest path. (PR #76451)
@@ -602,6 +602,16 @@ class Driver {
// FIXME: This should be in CompilationInfo.
std::string GetProgramPath(StringRef Name, const ToolChain ) const;
+ /// GetModuleManifestPath - Lookup the name of the Standard library manifest.
ChuanqiXu9 wrote:
```suggestion
/// GetStdModuleManifestPath - Lookup the path to the module of the Standard
library manifest.
```
https://github.com/llvm/llvm-project/pull/76451
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix crash with modules and constexpr destructor (PR #69076)
ChuanqiXu9 wrote: > I finally had time to debug this: The reason for modules being involved here > is because the serialization code calls `ParmVarDecl::getDefaultArg()` which > strips the outermost `FullExpr`, such as `ExprWithCleanups`. A potential fix > is in #76473 though I'm not really convinced by this asymmetry between > `getInit()` but calling `setDefaultArg()`. However, removing the handling of > `FullExpr` in `setDefaultArg()` causes a total 29 test failures, so that's > not an (easy) option... > > Personally, I would argue that adding `FullExpressionRAII` makes the code > more robust against the absence of `ExprWithCleanups` so that's maybe a good > thing to have regardless of fixing the serialization code. Thanks for looking into this. Good enough for me. Yeah the asymmetry between `getInit()` but calling `setDefaultArg()` is a concerning. Also it may be not easy to understand and fix the underlying problem (why getDefaultArg() will strip the outmost `FullExpr`) **properly**. So personally I am fine with the current workaround with a `FIXME`. Let's wait for the opinion from @cor3ntin and @shafik https://github.com/llvm/llvm-project/pull/69076 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] [clang] [libc++] Re-enable the clang_modules_include test for Objective-C++ (PR #66801)
ChuanqiXu9 wrote: > https://reviews.llvm.org/D158694 The motivation of my change is that I find multiple people are trying to fake `import std` from clang modules. This is not wanted. So I tried to ban it. I'll try to fix it or revert it later today if I didn't. https://github.com/llvm/llvm-project/pull/66801 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 2203a4e - [NFC] [Serialization] Improve AST serialization by reordering packed
Author: Chuanqi Xu
Date: 2023-12-21T16:35:20+08:00
New Revision: 2203a4e6e01ce6bfd69505420d304a81daf23dc9
URL:
https://github.com/llvm/llvm-project/commit/2203a4e6e01ce6bfd69505420d304a81daf23dc9
DIFF:
https://github.com/llvm/llvm-project/commit/2203a4e6e01ce6bfd69505420d304a81daf23dc9.diff
LOG: [NFC] [Serialization] Improve AST serialization by reordering packed
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'11, which takes 12
bits actually. However, if we changed the order to be 0x0f, then we
can store it as 0b00, 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.
Added:
Modified:
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
Removed:
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 209fb043420881..d989707d557524 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -584,7 +584,18 @@ void ASTDeclReader::Visit(Decl *D) {
void ASTDeclReader::VisitDecl(Decl *D) {
BitsUnpacker DeclBits(Record.readInt());
+ auto ModuleOwnership =
+ (Decl::ModuleOwnershipKind)DeclBits.getNextBits(/*Width=*/3);
+ D->setReferenced(DeclBits.getNextBit());
+ D->Used = DeclBits.getNextBit();
+ IsDeclMarkedUsed |= D->Used;
+ D->setAccess((AccessSpecifier)DeclBits.getNextBits(/*Width=*/2));
+ D->setImplicit(DeclBits.getNextBit());
bool HasStandaloneLexicalDC = DeclBits.getNextBit();
+ bool HasAttrs = DeclBits.getNextBit();
+ D->setTopLevelDeclInObjCContainer(DeclBits.getNextBit());
+ D->InvalidDecl = DeclBits.getNextBit();
+ D->FromASTFile = true;
if (D->isTemplateParameter() || D->isTemplateParameterPack() ||
isa(D)) {
@@ -623,20 +634,6 @@ void ASTDeclReader::VisitDecl(Decl *D) {
}
D->setLocation(ThisDeclLoc);
- D->InvalidDecl = DeclBits.getNextBit();
- bool HasAttrs = DeclBits.getNextBit();
- D->setImplicit(DeclBits.getNextBit());
- D->Used = DeclBits.getNextBit();
- IsDeclMarkedUsed |= D->Used;
- D->setReferenced(DeclBits.getNextBit());
- D->setTopLevelDeclInObjCContainer(DeclBits.getNextBit());
- D->setAccess((AccessSpecifier)DeclBits.getNextBits(/*Width=*/2));
- D->FromASTFile = true;
- auto ModuleOwnership =
- (Decl::ModuleOwnershipKind)DeclBits.getNextBits(/*Width=*/3);
- bool ModulePrivate =
- (ModuleOwnership == Decl::ModuleOwnershipKind::ModulePrivate);
-
if (HasAttrs) {
AttrVec Attrs;
Record.readAttributes(Attrs);
@@ -647,8 +644,9 @@ void ASTDeclReader::VisitDecl(Decl *D) {
// Determine whether this declaration is part of a (sub)module. If so, it
// may not yet be visible.
+ bool ModulePrivate =
+ (ModuleOwnership == Decl::ModuleOwnershipKind::ModulePrivate);
if (unsigned SubmoduleID = readSubmoduleID()) {
-
switch (ModuleOwnership) {
case Decl::ModuleOwnershipKind::Visible:
ModuleOwnership = Decl::ModuleOwnershipKind::VisibleWhenImported;
@@ -1065,9 +1063,11 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
// after everything else is read.
BitsUnpacker FunctionDeclBits(Record.readInt());
+ FD->setCachedLinkage((Linkage)FunctionDeclBits.getNextBits(/*Width=*/3));
FD->setStorageClass((StorageClass)FunctionDeclBits.getNextBits(/*Width=*/3));
FD->setInlineSpecified(FunctionDeclBits.getNextBit());
FD->setImplicitlyInline(FunctionDeclBits.getNextBit());
+ FD->setHasSkippedBody(FunctionDeclBits.getNextBit());
FD->setVirtualAsWritten(FunctionDeclBits.getNextBit());
// We defer calling `FunctionDecl::setPure()` here as for methods of
// `CXXTemplateSpecializationDecl`s, we may not have connected up the
@@ -1081,16 +1081,14 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD)
{
FD->setDefaulted(FunctionDeclBits.getNextBit());
FD->setExplicitlyDefaulted(FunctionDeclBits.getNextBit());
FD->setIneligibleOrNotSelected(FunctionDeclBits.getNextBit());
- FD->setHasImplicitReturnZero(FunctionDeclBits.getNextBit());
FD->setConstexprKind(
(ConstexprSpecKind)FunctionDeclBits.getNextBits(/*Width=*/2));
- FD->setUsesSEHTry(FunctionDeclBits.getNextBit());
-
[clang] [Modules] [HeaderSearch] Don't reenter headers if it is pragma once (PR #76119)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/76119 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Modules] [HeaderSearch] Don't reenter headers if it is pragma once (PR #76119)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/76119 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Modules] [HeaderSearch] Don't reenter headers if it is pragma once o… (PR #76119)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/76119
…f #import
Close https://github.com/llvm/llvm-project/issues/73023
The direct issue of https://github.com/llvm/llvm-project/issues/73023 is that
we entered a header which is marked as pragma once since the compiler think it
is OK if there is controlling macro.
It doesn't make sense. I feel like it should be sufficient to skip it after we
see the '#pragma once'.
>From the context, it looks like the workaround is primarily for ObjectiveC. So
>we might need reviewers from OC.
>From 6610a2599318f1e7ae6be6295f2d4e8371ac44f1 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Thu, 21 Dec 2023 11:24:14 +0800
Subject: [PATCH] [Modules] [HeaderSearch] Don't reenter headers if it is
pragma once of #import
Close https://github.com/llvm/llvm-project/issues/73023
The direct issue of https://github.com/llvm/llvm-project/issues/73023 is
that we entered a header which is marked as pragma once since the
compiler think it is OK if there is controlling macro.
It doesn't make sense. I feel like it should be sufficient to skip it
after we see the '#pragma once'.
>From the context, it looks like the workaround is primarily for
ObjectiveC. So we might need reviewers from OC.
---
clang/lib/Lex/HeaderSearch.cpp | 2 +-
clang/test/Modules/pr73023.cpp | 20
2 files changed, 21 insertions(+), 1 deletion(-)
create mode 100644 clang/test/Modules/pr73023.cpp
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 0f1090187734ff..9020a661321faf 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1458,7 +1458,7 @@ bool HeaderSearch::ShouldEnterIncludeFile(Preprocessor
,
} else {
// Otherwise, if this is a #include of a file that was previously #import'd
// or if this is the second #include of a #pragma once file, ignore it.
-if ((FileInfo.isPragmaOnce || FileInfo.isImport) && !TryEnterImported())
+if (FileInfo.isPragmaOnce || FileInfo.isImport)
return false;
}
diff --git a/clang/test/Modules/pr73023.cpp b/clang/test/Modules/pr73023.cpp
new file mode 100644
index 00..d80306d39ef48c
--- /dev/null
+++ b/clang/test/Modules/pr73023.cpp
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/foo.cpp -I%t -fsyntax-only -verify
+
+//--- i.h
+#ifndef FOO_H
+#pragma once
+struct S{};
+#endif
+
+//--- foo.cpp
+// expected-no-diagnostics
+#include "i.h"
+#include "i.h"
+
+int foo() {
+return sizeof(S);
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 565e5e8 - Recommit [NFC] [Serialization] Packing more bits and refactor AbbrevToUse
Author: Chuanqi Xu
Date: 2023-12-21T10:30:12+08:00
New Revision: 565e5e861f64f455ab789bc50840e0be2d33c417
URL:
https://github.com/llvm/llvm-project/commit/565e5e861f64f455ab789bc50840e0be2d33c417
DIFF:
https://github.com/llvm/llvm-project/commit/565e5e861f64f455ab789bc50840e0be2d33c417.diff
LOG: Recommit [NFC] [Serialization] Packing more bits and refactor AbbrevToUse
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.
Added:
Modified:
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
Removed:
diff --git a/clang/include/clang/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index 59358e77edb072..21d791f5cd89a2 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2422,6 +2422,8 @@ class BitsUnpacker {
CurrentBitsIndex = 0;
}
+ void advance(uint32_t BitsWidth) { CurrentBitsIndex += BitsWidth; }
+
bool getNextBit() {
assert(isValid());
return Value & (1 << CurrentBitsIndex++);
diff --git a/clang/include/clang/Serialization/ASTWriter.h
b/clang/include/clang/Serialization/ASTWriter.h
index a56929ef0245ee..de69f99003d827 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -564,11 +564,25 @@ class ASTWriter : public ASTDeserializationListener,
unsigned DeclEnumAbbrev = 0;
unsigned DeclObjCIvarAbbrev = 0;
unsigned DeclCXXMethodAbbrev = 0;
+ unsigned DeclDependentNonTemplateCXXMethodAbbrev = 0;
+ unsigned DeclTemplateCXXMethodAbbrev = 0;
+ unsigned DeclMemberSpecializedCXXMethodAbbrev = 0;
+ unsigned DeclTemplateSpecializedCXXMethodAbbrev = 0;
+ unsigned DeclDependentSpecializationCXXMethodAbbrev = 0;
+ unsigned DeclTemplateTypeParmAbbrev = 0;
+ unsigned DeclUsingShadowAbbrev = 0;
unsigned DeclRefExprAbbrev = 0;
unsigned CharacterLiteralAbbrev = 0;
unsigned IntegerLiteralAbbrev = 0;
unsigned ExprImplicitCastAbbrev = 0;
+ unsigned BinaryOperatorAbbrev = 0;
+ unsigned CompoundAssignOperatorAbbrev = 0;
+ unsigned CallExprAbbrev = 0;
+ unsigned CXXOperatorCallExprAbbrev = 0;
+ unsigned CXXMemberCallExprAbbrev = 0;
+
+ unsigned CompoundStmtAbbrev = 0;
void WriteDeclAbbrevs();
void WriteDecl(ASTContext , Decl *D);
@@ -735,12 +749,41 @@ class ASTWriter : public ASTDeserializationListener,
unsigned getDeclFieldAbbrev() const { return DeclFieldAbbrev; }
unsigned getDeclEnumAbbrev() const { return DeclEnumAbbrev; }
unsigned getDeclObjCIvarAbbrev() const { return DeclObjCIvarAbbrev; }
- unsigned getDeclCXXMethodAbbrev() const { return DeclCXXMethodAbbrev; }
+ unsigned getDeclCXXMethodAbbrev(FunctionDecl::TemplatedKind Kind) const {
+switch (Kind) {
+case FunctionDecl::TK_NonTemplate:
+ return DeclCXXMethodAbbrev;
+case FunctionDecl::TK_FunctionTemplate:
+ return DeclTemplateCXXMethodAbbrev;
+case FunctionDecl::TK_MemberSpecialization:
+ return DeclMemberSpecializedCXXMethodAbbrev;
+case FunctionDecl::TK_FunctionTemplateSpecialization:
+ return DeclTemplateSpecializedCXXMethodAbbrev;
+case FunctionDecl::TK_DependentNonTemplate:
+ return DeclDependentNonTemplateCXXMethodAbbrev;
+case FunctionDecl::TK_DependentFunctionTemplateSpecialization:
+ return DeclDependentSpecializationCXXMethodAbbrev;
+}
+llvm_unreachable("Unknwon Template Kind!");
+ }
+ unsigned getDeclTemplateTypeParmAbbrev() const {
+return DeclTemplateTypeParmAbbrev;
+ }
+ unsigned getDeclUsingShadowAbbrev() const { return DeclUsingShadowAbbrev; }
unsigned getDeclRefExprAbbrev() const { return DeclRefExprAbbrev; }
unsigned getCharacterLiteralAbbrev() const { return CharacterLiteralAbbrev; }
unsigned getIntegerLiteralAbbrev() const { return IntegerLiteralAbbrev; }
unsigned getExprImplicitCastAbbrev() const { return ExprImplicitCastAbbrev; }
+ unsigned getBinaryOperatorAbbrev() const { return BinaryOperatorAbbrev; }
+ unsigned getCompoundAssignOperatorAbbrev() const {
+return CompoundAssignOperatorAbbrev;
+ }
+ unsigned getCallExprAbbrev() const { return CallExprAbbrev; }
+ unsigned getCXXOperatorCallExprAbbrev() { return CXXOperatorCallExprAbbrev; }
+ unsigned getCXXMemberCallExprAbbrev() { return
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
@@ -0,0 +1,50 @@
+// REQUIRES: !system-windows
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/layer1.cppm -triple %itanium_abi_triple \
+// RUN: -emit-module-interface -o %t/foo-layer1.pcm
+// RUN: %clang_cc1 -std=c++20 %t/layer2.cppm -triple %itanium_abi_triple \
+// RUN: -emit-module-interface -fmodule-file=foo:layer1=%t/foo-layer1.pcm \
+// RUN: -o %t/foo-layer2.pcm
+// RUN: %clang_cc1 -std=c++20 %t/foo-layer1.pcm -S -emit-llvm -o - | FileCheck
%t/layer1.cppm
+// RUN: %clang_cc1 -std=c++20 %t/foo-layer2.pcm -S -emit-llvm -o - \
+// RUN: -fmodule-file=foo:layer1=%t/foo-layer1.pcm | FileCheck
%t/layer2.cppm
+
+//--- layer1.cppm
+export module foo:layer1;
+struct Fruit {
+virtual ~Fruit() = default;
+virtual void eval() = 0;
+};
+struct Banana : public Fruit {
+Banana() {}
+void eval() override;
+};
+
+// CHECK-DAG: @_ZTVW3foo6Banana = unnamed_addr constant
+// CHECK-DAG: @_ZTSW3foo6Banana = constant
+// CHECK-DAG: @_ZTIW3foo6Banana = constant
+//
+// CHECK-DAG: @_ZTVW3foo5Fruit = unnamed_addr constant
+// CHECK-DAG: @_ZTSW3foo5Fruit = constant
+// CHECK-DAG: @_ZTIW3foo5Fruit = constant
+
+//--- layer2.cppm
+export module foo:layer2;
+import :layer1;
+export void layer2_fun() {
+Banana *b = new Banana();
+b->eval();
+}
+void Banana::eval() {
+}
+
ChuanqiXu9 wrote:
Done
https://github.com/llvm/llvm-project/pull/75912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/75912
>From 908a0287e092ce7ac1865de32370ec3114b104ad Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Tue, 19 Dec 2023 17:00:59 +0800
Subject: [PATCH] [C++20] [Modules] [Itanium ABI] Generate the vtable in the
module unit of dynamic classes
Close https://github.com/llvm/llvm-project/issues/70585 and reflect
https://github.com/itanium-cxx-abi/cxx-abi/issues/170.
The significant change of the patch is: for dynamic classes attached to
module units, we generate the vtable to the attached module units
directly and the key functions for such classes is meaningless.
---
clang/lib/CodeGen/CGVTables.cpp | 32 +++---
clang/lib/CodeGen/ItaniumCXXABI.cpp | 6 +++
clang/test/CodeGenCXX/modules-vtable.cppm | 27 +++-
clang/test/CodeGenCXX/pr70585.cppm| 54 +++
4 files changed, 102 insertions(+), 17 deletions(-)
create mode 100644 clang/test/CodeGenCXX/pr70585.cppm
diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index 27a2cab4f75319..8324c38ff94feb 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -1046,6 +1046,15 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD)
{
if (!RD->isExternallyVisible())
return llvm::GlobalVariable::InternalLinkage;
+ // Previously we'll decide the linkage of the vtable by the linkage
+ // of the key function. But within modules, the concept of key functions
+ // becomes meaningless. So the linkage of the vtable should always be
+ // external if the class is externally visible.
+ //
+ // TODO: How about the case of AppleKext, DLLExportAttr and DLLImportAttr.
+ if (Module *M = RD->getOwningModule(); M && M->isNamedModule())
+return llvm::GlobalVariable::ExternalLinkage;
+
// We're at the end of the translation unit, so the current key
// function is fully correct.
const CXXMethodDecl *keyFunction = Context.getCurrentKeyFunction(RD);
@@ -1180,6 +1189,21 @@ bool CodeGenVTables::isVTableExternal(const
CXXRecordDecl *RD) {
TSK == TSK_ExplicitInstantiationDefinition)
return false;
+ // Itanium C++ ABI [5.2.3]:
+ // Virtual tables for dynamic classes are emitted as follows:
+ //
+ // - If the class is templated, the tables are emitted in every object that
+ // references any of them.
+ // - Otherwise, if the class is attached to a module, the tables are uniquely
+ // emitted in the object for the module unit in which it is defined.
+ // - Otherwise, if the class has a key function (see below), the tables are
+ // emitted in the object for the translation unit containing the definition
of
+ // the key function. This is unique if the key function is not inline.
+ // - Otherwise, the tables are emitted in every object that references any of
+ // them.
+ if (Module *M = RD->getOwningModule(); M && M->isNamedModule())
+return M != CGM.getContext().getCurrentNamedModule();
+
// Otherwise, if the class doesn't have a key function (possibly
// anymore), the vtable must be defined here.
const CXXMethodDecl *keyFunction =
CGM.getContext().getCurrentKeyFunction(RD);
@@ -1189,13 +1213,7 @@ bool CodeGenVTables::isVTableExternal(const
CXXRecordDecl *RD) {
const FunctionDecl *Def;
// Otherwise, if we don't have a definition of the key function, the
// vtable must be defined somewhere else.
- if (!keyFunction->hasBody(Def))
-return true;
-
- assert(Def && "The body of the key function is not assigned to Def?");
- // If the non-inline key function comes from another module unit, the vtable
- // must be defined there.
- return Def->isInAnotherModuleUnit() && !Def->isInlineSpecified();
+ return !keyFunction->hasBody(Def);
}
/// Given that we're currently at the end of the translation unit, and
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index d173806ec8ce53..e80270c231e19f 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1801,6 +1801,12 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables
,
if (VTable->hasInitializer())
return;
+ // If the class are attached to a C++ named module other than the one
+ // we're currently compiling, the vtable should be defined there.
+ if (Module *M = RD->getOwningModule();
+ M && M->isNamedModule() && M != CGM.getContext().getCurrentNamedModule())
+return;
+
ItaniumVTableContext = CGM.getItaniumVTableContext();
const VTableLayout = VTContext.getVTableLayout(RD);
llvm::GlobalVariable::LinkageTypes Linkage = CGM.getVTableLinkage(RD);
diff --git a/clang/test/CodeGenCXX/modules-vtable.cppm
b/clang/test/CodeGenCXX/modules-vtable.cppm
index fb179b1de4880b..abbbc403ace243 100644
--- a/clang/test/CodeGenCXX/modules-vtable.cppm
+++ b/clang/test/CodeGenCXX/modules-vtable.cppm
@@ -24,6 +24,8 @@
// RUN: %t/M-A.cppm -o %t/M-A.pcm
// RUN:
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
@@ -0,0 +1,50 @@
+// REQUIRES: !system-windows
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/layer1.cppm -triple %itanium_abi_triple \
+// RUN: -emit-module-interface -o %t/foo-layer1.pcm
+// RUN: %clang_cc1 -std=c++20 %t/layer2.cppm -triple %itanium_abi_triple \
+// RUN: -emit-module-interface -fmodule-file=foo:layer1=%t/foo-layer1.pcm \
+// RUN: -o %t/foo-layer2.pcm
+// RUN: %clang_cc1 -std=c++20 %t/foo-layer1.pcm -S -emit-llvm -o - | FileCheck
%t/layer1.cppm
+// RUN: %clang_cc1 -std=c++20 %t/foo-layer2.pcm -S -emit-llvm -o - \
+// RUN: -fmodule-file=foo:layer1=%t/foo-layer1.pcm | FileCheck
%t/layer2.cppm
+
+//--- layer1.cppm
+export module foo:layer1;
+struct Fruit {
+virtual ~Fruit() = default;
+virtual void eval() = 0;
+};
+struct Banana : public Fruit {
+Banana() {}
+void eval() override;
+};
+
+// CHECK-DAG: @_ZTVW3foo6Banana = unnamed_addr constant
+// CHECK-DAG: @_ZTSW3foo6Banana = constant
+// CHECK-DAG: @_ZTIW3foo6Banana = constant
+//
+// CHECK-DAG: @_ZTVW3foo5Fruit = unnamed_addr constant
+// CHECK-DAG: @_ZTSW3foo5Fruit = constant
+// CHECK-DAG: @_ZTIW3foo5Fruit = constant
+
+//--- layer2.cppm
+export module foo:layer2;
+import :layer1;
+export void layer2_fun() {
+Banana *b = new Banana();
+b->eval();
+}
+void Banana::eval() {
+}
+
ChuanqiXu9 wrote:
We're testing:
(1) The use of virtual functions won't produce the vtable.
(2) The definition of key functions won't produce the vtable.
I'll try to add a comment here.
https://github.com/llvm/llvm-project/pull/75912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
@@ -0,0 +1,50 @@
+// REQUIRES: !system-windows
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/layer1.cppm -triple %itanium_abi_triple \
+// RUN: -emit-module-interface -o %t/foo-layer1.pcm
+// RUN: %clang_cc1 -std=c++20 %t/layer2.cppm -triple %itanium_abi_triple \
+// RUN: -emit-module-interface -fmodule-file=foo:layer1=%t/foo-layer1.pcm \
+// RUN: -o %t/foo-layer2.pcm
+// RUN: %clang_cc1 -std=c++20 %t/foo-layer1.pcm -S -emit-llvm -o - | FileCheck
%t/layer1.cppm
+// RUN: %clang_cc1 -std=c++20 %t/foo-layer2.pcm -S -emit-llvm -o - \
+// RUN: -fmodule-file=foo:layer1=%t/foo-layer1.pcm | FileCheck
%t/layer2.cppm
+
+//--- layer1.cppm
+export module foo:layer1;
+struct Fruit {
+virtual ~Fruit() = default;
+virtual void eval() = 0;
+};
+struct Banana : public Fruit {
+Banana() {}
+void eval() override;
+};
+
+// CHECK-DAG: @_ZTVW3foo6Banana = unnamed_addr constant
+// CHECK-DAG: @_ZTSW3foo6Banana = constant
+// CHECK-DAG: @_ZTIW3foo6Banana = constant
+//
+// CHECK-DAG: @_ZTVW3foo5Fruit = unnamed_addr constant
+// CHECK-DAG: @_ZTSW3foo5Fruit = constant
+// CHECK-DAG: @_ZTIW3foo5Fruit = constant
+
+//--- layer2.cppm
+export module foo:layer2;
+import :layer1;
+export void layer2_fun() {
+Banana *b = new Banana();
+b->eval();
+}
+void Banana::eval() {
+}
+
+// CHECK-NOT: @_ZTVW3foo6Banana
+// CHECK-NOT: @_ZTSW3foo6Banana
+// CHECK-NOT: @_ZTIW3foo6Banana
+// CHECK-NOT: @_ZTVW3foo5Fruit
+// CHECK-NOT: @_ZTSW3foo5Fruit
+// CHECK-NOT: @_ZTIW3foo5Fruit
ChuanqiXu9 wrote:
I prefer the current pattern so that other developers can understand what we're
testing by `llvm-cxxfilt`.
https://github.com/llvm/llvm-project/pull/75912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
@@ -0,0 +1,50 @@
+// REQUIRES: !system-windows
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/layer1.cppm -triple %itanium_abi_triple \
+// RUN: -emit-module-interface -o %t/foo-layer1.pcm
+// RUN: %clang_cc1 -std=c++20 %t/layer2.cppm -triple %itanium_abi_triple \
+// RUN: -emit-module-interface -fmodule-file=foo:layer1=%t/foo-layer1.pcm \
+// RUN: -o %t/foo-layer2.pcm
+// RUN: %clang_cc1 -std=c++20 %t/foo-layer1.pcm -S -emit-llvm -o - | FileCheck
%t/layer1.cppm
+// RUN: %clang_cc1 -std=c++20 %t/foo-layer2.pcm -S -emit-llvm -o - \
+// RUN: -fmodule-file=foo:layer1=%t/foo-layer1.pcm | FileCheck
%t/layer2.cppm
+
+//--- layer1.cppm
+export module foo:layer1;
+struct Fruit {
+virtual ~Fruit() = default;
+virtual void eval() = 0;
+};
+struct Banana : public Fruit {
+Banana() {}
+void eval() override;
+};
+
+// CHECK-DAG: @_ZTVW3foo6Banana = unnamed_addr constant
+// CHECK-DAG: @_ZTSW3foo6Banana = constant
+// CHECK-DAG: @_ZTIW3foo6Banana = constant
+//
+// CHECK-DAG: @_ZTVW3foo5Fruit = unnamed_addr constant
+// CHECK-DAG: @_ZTSW3foo5Fruit = constant
+// CHECK-DAG: @_ZTIW3foo5Fruit = constant
+
ChuanqiXu9 wrote:
Yeah, the linkage is already tested. Currently we're checking the linkage is
`external` (default linkage). For example, if the linkage is `linkonce_odr`,
the output will be something like: `// CHECK-DAG: @_ZTSW3foo5Fruit =
linkonce_odr constant`
https://github.com/llvm/llvm-project/pull/75912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
@@ -0,0 +1,50 @@
+// REQUIRES: !system-windows
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/layer1.cppm -triple %itanium_abi_triple \
+// RUN: -emit-module-interface -o %t/foo-layer1.pcm
+// RUN: %clang_cc1 -std=c++20 %t/layer2.cppm -triple %itanium_abi_triple \
+// RUN: -emit-module-interface -fmodule-file=foo:layer1=%t/foo-layer1.pcm \
+// RUN: -o %t/foo-layer2.pcm
+// RUN: %clang_cc1 -std=c++20 %t/foo-layer1.pcm -S -emit-llvm -o - | FileCheck
%t/layer1.cppm
+// RUN: %clang_cc1 -std=c++20 %t/foo-layer2.pcm -S -emit-llvm -o - \
+// RUN: -fmodule-file=foo:layer1=%t/foo-layer1.pcm | FileCheck
%t/layer2.cppm
+
+//--- layer1.cppm
+export module foo:layer1;
+struct Fruit {
+virtual ~Fruit() = default;
+virtual void eval() = 0;
+};
+struct Banana : public Fruit {
+Banana() {}
+void eval() override;
+};
+
ChuanqiXu9 wrote:
This comes from the original reproducer while the base class is a pure virtual
class that can't be instantiated. While it may be sufficient to reduce them to
a single class, I feel slightly better to keep the form since it is not complex.
https://github.com/llvm/llvm-project/pull/75912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce thin BMI (PR #71622)
ChuanqiXu9 wrote: Yeah, it'll be much better if we had that feature. https://github.com/llvm/llvm-project/pull/71622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce thin BMI (PR #71622)
ChuanqiXu9 wrote: > > Since there is no meaningful review opinions here, let's continue it in a > > new and clean PR: #75894 > > Note that this leaves behind all who have hit "Subscribe" on this PR to keep > tabs on things (I wish Github provided a way to mark-as-duplicate and merge > the subscriber list). All of the references to this PR are now confused as > well. What benefit does a "clean" PR provide? >From my experience, a cleaner PR will be easier for new people to get the >context or we look back after times. The more the comments are, the more >confused readers get. Generally a cleaner PR is not suggested since it lost >the context. But for this specific example, I think it makes sense since the >context here is not so meaning full. > Note that this leaves behind all who have hit "Subscribe" on this PR to keep > tabs on things Sorry for that. But I think it may not be a big deal since people who still want to take an eye this can made it by clicking two times simply. https://github.com/llvm/llvm-project/pull/71622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/75912
>From 64296827cbba26fba0b5c0d2a6edfd966394cabc Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Tue, 19 Dec 2023 17:00:59 +0800
Subject: [PATCH] [C++20] [Modules] [Itanium ABI] Generate the vtable in the
module unit of dynamic classes
Close https://github.com/llvm/llvm-project/issues/70585 and reflect
https://github.com/itanium-cxx-abi/cxx-abi/issues/170.
The significant change of the patch is: for dynamic classes attached to
module units, we generate the vtable to the attached module units
directly and the key functions for such classes is meaningless.
---
clang/lib/CodeGen/CGVTables.cpp | 32 +++
clang/lib/CodeGen/ItaniumCXXABI.cpp | 6 +++
clang/test/CodeGenCXX/modules-vtable.cppm | 27 +++-
clang/test/CodeGenCXX/pr70585.cppm| 50 +++
4 files changed, 98 insertions(+), 17 deletions(-)
create mode 100644 clang/test/CodeGenCXX/pr70585.cppm
diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index 27a2cab4f75319..8324c38ff94feb 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -1046,6 +1046,15 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD)
{
if (!RD->isExternallyVisible())
return llvm::GlobalVariable::InternalLinkage;
+ // Previously we'll decide the linkage of the vtable by the linkage
+ // of the key function. But within modules, the concept of key functions
+ // becomes meaningless. So the linkage of the vtable should always be
+ // external if the class is externally visible.
+ //
+ // TODO: How about the case of AppleKext, DLLExportAttr and DLLImportAttr.
+ if (Module *M = RD->getOwningModule(); M && M->isNamedModule())
+return llvm::GlobalVariable::ExternalLinkage;
+
// We're at the end of the translation unit, so the current key
// function is fully correct.
const CXXMethodDecl *keyFunction = Context.getCurrentKeyFunction(RD);
@@ -1180,6 +1189,21 @@ bool CodeGenVTables::isVTableExternal(const
CXXRecordDecl *RD) {
TSK == TSK_ExplicitInstantiationDefinition)
return false;
+ // Itanium C++ ABI [5.2.3]:
+ // Virtual tables for dynamic classes are emitted as follows:
+ //
+ // - If the class is templated, the tables are emitted in every object that
+ // references any of them.
+ // - Otherwise, if the class is attached to a module, the tables are uniquely
+ // emitted in the object for the module unit in which it is defined.
+ // - Otherwise, if the class has a key function (see below), the tables are
+ // emitted in the object for the translation unit containing the definition
of
+ // the key function. This is unique if the key function is not inline.
+ // - Otherwise, the tables are emitted in every object that references any of
+ // them.
+ if (Module *M = RD->getOwningModule(); M && M->isNamedModule())
+return M != CGM.getContext().getCurrentNamedModule();
+
// Otherwise, if the class doesn't have a key function (possibly
// anymore), the vtable must be defined here.
const CXXMethodDecl *keyFunction =
CGM.getContext().getCurrentKeyFunction(RD);
@@ -1189,13 +1213,7 @@ bool CodeGenVTables::isVTableExternal(const
CXXRecordDecl *RD) {
const FunctionDecl *Def;
// Otherwise, if we don't have a definition of the key function, the
// vtable must be defined somewhere else.
- if (!keyFunction->hasBody(Def))
-return true;
-
- assert(Def && "The body of the key function is not assigned to Def?");
- // If the non-inline key function comes from another module unit, the vtable
- // must be defined there.
- return Def->isInAnotherModuleUnit() && !Def->isInlineSpecified();
+ return !keyFunction->hasBody(Def);
}
/// Given that we're currently at the end of the translation unit, and
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index d173806ec8ce53..e80270c231e19f 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1801,6 +1801,12 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables
,
if (VTable->hasInitializer())
return;
+ // If the class are attached to a C++ named module other than the one
+ // we're currently compiling, the vtable should be defined there.
+ if (Module *M = RD->getOwningModule();
+ M && M->isNamedModule() && M != CGM.getContext().getCurrentNamedModule())
+return;
+
ItaniumVTableContext = CGM.getItaniumVTableContext();
const VTableLayout = VTContext.getVTableLayout(RD);
llvm::GlobalVariable::LinkageTypes Linkage = CGM.getVTableLinkage(RD);
diff --git a/clang/test/CodeGenCXX/modules-vtable.cppm
b/clang/test/CodeGenCXX/modules-vtable.cppm
index fb179b1de4880b..abbbc403ace243 100644
--- a/clang/test/CodeGenCXX/modules-vtable.cppm
+++ b/clang/test/CodeGenCXX/modules-vtable.cppm
@@ -24,6 +24,8 @@
// RUN: %t/M-A.cppm -o %t/M-A.pcm
// RUN:
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/75912
Close https://github.com/llvm/llvm-project/issues/70585 and reflect
https://github.com/itanium-cxx-abi/cxx-abi/issues/170.
The significant change of the patch is: for dynamic classes attached to module
units, we generate the vtable to the attached module units directly and the key
functions for such classes is meaningless.
>From 38b3178137b5bfafd8cd0a3194111029b2398298 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Tue, 19 Dec 2023 17:00:59 +0800
Subject: [PATCH] [C++20] [Modules] [Itanium ABI] Generate the vtable in the
module unit of dynamic classes
Close https://github.com/llvm/llvm-project/issues/70585 and reflect
https://github.com/itanium-cxx-abi/cxx-abi/issues/170.
The significant change of the patch is: for dynamic classes attached to
module units, we generate the vtable to the attached module units
directly and the key functions for such classes is meaningless.
---
clang/lib/CodeGen/CGVTables.cpp | 27
clang/lib/CodeGen/ItaniumCXXABI.cpp | 6 +++
clang/lib/CodeGen/ModuleBuilder.cpp | 2 +-
clang/test/CodeGenCXX/modules-vtable.cppm | 27 +++-
clang/test/CodeGenCXX/pr70585.cppm| 50 +++
5 files changed, 94 insertions(+), 18 deletions(-)
create mode 100644 clang/test/CodeGenCXX/pr70585.cppm
diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index 27a2cab4f75319..06b02df93b26d8 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -1045,6 +1045,15 @@ llvm::GlobalVariable::LinkageTypes
CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) {
if (!RD->isExternallyVisible())
return llvm::GlobalVariable::InternalLinkage;
+
+ // Previously we'll decide the linkage of the vtable by the linkage
+ // of the key function. But within modules, the concept of key functions
+ // becomes meaningless. So the linkage of the vtable should always be
+ // external if the class is externally visible.
+ //
+ // TODO: How about the case of AppleKext, DLLExportAttr and DLLImportAttr.
+ if (Module *M = RD->getOwningModule(); M && M->isNamedModule())
+return llvm::GlobalVariable::ExternalLinkage;
// We're at the end of the translation unit, so the current key
// function is fully correct.
@@ -1180,6 +1189,16 @@ bool CodeGenVTables::isVTableExternal(const
CXXRecordDecl *RD) {
TSK == TSK_ExplicitInstantiationDefinition)
return false;
+ // Itanium C++ ABI [5.2.3]:
+ // Virtual tables for dynamic classes are emitted as follows:
+ //
+ // - If the class is templated, the tables are emitted in every object that
references any of them.
+ // - Otherwise, if the class is attached to a module, the tables are
uniquely emitted in the object for the module unit in which it is defined.
+ // - Otherwise, if the class has a key function (see below), the tables are
emitted in the object for the translation unit containing the definition of the
key function. This is unique if the key function is not inline.
+ // - Otherwise, the tables are emitted in every object that references any
of them.
+ if (Module *M = RD->getOwningModule(); M && M->isNamedModule())
+return M != CGM.getContext().getCurrentNamedModule();
+
// Otherwise, if the class doesn't have a key function (possibly
// anymore), the vtable must be defined here.
const CXXMethodDecl *keyFunction =
CGM.getContext().getCurrentKeyFunction(RD);
@@ -1189,13 +1208,7 @@ bool CodeGenVTables::isVTableExternal(const
CXXRecordDecl *RD) {
const FunctionDecl *Def;
// Otherwise, if we don't have a definition of the key function, the
// vtable must be defined somewhere else.
- if (!keyFunction->hasBody(Def))
-return true;
-
- assert(Def && "The body of the key function is not assigned to Def?");
- // If the non-inline key function comes from another module unit, the vtable
- // must be defined there.
- return Def->isInAnotherModuleUnit() && !Def->isInlineSpecified();
+ return !keyFunction->hasBody(Def);
}
/// Given that we're currently at the end of the translation unit, and
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index d173806ec8ce53..7e2c1191071c84 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1801,6 +1801,12 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables
,
if (VTable->hasInitializer())
return;
+ // If the class are attached to a C++ named module other than the one
+ // we're currently compiling, the vtable should be defined there.
+ if (Module *M = RD->getOwningModule(); M && M->isNamedModule() &&
+ M != CGM.getContext().getCurrentNamedModule())
+return;
+
ItaniumVTableContext = CGM.getItaniumVTableContext();
const VTableLayout = VTContext.getVTableLayout(RD);
llvm::GlobalVariable::LinkageTypes Linkage =
[clang] [C++20] [Modules] Bring Decls Hash to BMI for C++20 Module units (PR #71627)
https://github.com/ChuanqiXu9 converted_to_draft https://github.com/llvm/llvm-project/pull/71627 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Bring Decls Hash to BMI for C++20 Module units (PR #71627)
ChuanqiXu9 wrote: As I summarized in https://discourse.llvm.org/t/rfc-c-20-modules-introduce-thin-bmi-and-decls-hash/74755/43, people are interested in this direction, but it may be too early for us to implement it. Let's postpone this. https://github.com/llvm/llvm-project/pull/71627 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce thin BMI (PR #71622)
https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/71622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce thin BMI (PR #71622)
ChuanqiXu9 wrote: Since there is no meaningful review opinions here, let's continue it in a new and clean PR: https://github.com/llvm/llvm-project/pull/75894 https://github.com/llvm/llvm-project/pull/71622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce reduced BMI (PR #75894)
ChuanqiXu9 wrote: My impression to the feedbacks is that every one of us loves the direction, while we may need more agreement on the user interfaces. To make it easier to review, I split all the user interfaces related part to following patches. So that the current patch won't affect users. This patch introduced an CC1 option `-emit-thin-module-interface` and tests the behavior for almost every places we can to make sure it is stable. While the diff is big, we can find most parts of it are boiler plate, so don't be panic. I feel the patch good and hope we can land this quickly so that we can start the work to help users actually. https://github.com/llvm/llvm-project/pull/75894 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
ChuanqiXu9 wrote: @sam-mccall ping~ https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] e55bda0 - [NFC] Fix the warning Wcovered-switch-default
Author: Chuanqi Xu
Date: 2023-12-15T11:39:20+08:00
New Revision: e55bda06dc2bb1ef11ff4fcc43f90d8bf843f967
URL:
https://github.com/llvm/llvm-project/commit/e55bda06dc2bb1ef11ff4fcc43f90d8bf843f967
DIFF:
https://github.com/llvm/llvm-project/commit/e55bda06dc2bb1ef11ff4fcc43f90d8bf843f967.diff
LOG: [NFC] Fix the warning Wcovered-switch-default
There is a warning saying:
ASTWriter.h:766:5: error: default label in switch which covers all
enumeration values [-Werror,-Wcovered-switch-default]
766 | default:
| ^
1 error generated.
And this patch tries to fix this.
Added:
Modified:
clang/include/clang/Serialization/ASTWriter.h
Removed:
diff --git a/clang/include/clang/Serialization/ASTWriter.h
b/clang/include/clang/Serialization/ASTWriter.h
index 16ab9583f8ed8e..de69f99003d827 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -763,9 +763,8 @@ class ASTWriter : public ASTDeserializationListener,
return DeclDependentNonTemplateCXXMethodAbbrev;
case FunctionDecl::TK_DependentFunctionTemplateSpecialization:
return DeclDependentSpecializationCXXMethodAbbrev;
-default:
- llvm_unreachable("Unknwon Template Kind!");
}
+llvm_unreachable("Unknwon Template Kind!");
}
unsigned getDeclTemplateTypeParmAbbrev() const {
return DeclTemplateTypeParmAbbrev;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 9cdb825 - [NFC] [Serialization] Packing more bits and refactor AbbrevToUse
Author: Chuanqi Xu
Date: 2023-12-15T11:12:52+08:00
New Revision: 9cdb825a4f1bf9e75829d03879620c6144d0b7bc
URL:
https://github.com/llvm/llvm-project/commit/9cdb825a4f1bf9e75829d03879620c6144d0b7bc
DIFF:
https://github.com/llvm/llvm-project/commit/9cdb825a4f1bf9e75829d03879620c6144d0b7bc.diff
LOG: [NFC] [Serialization] Packing more bits and refactor AbbrevToUse
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.
Added:
Modified:
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
Removed:
diff --git a/clang/include/clang/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index 9bb89ec9410911..a6dd779386dc16 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2422,6 +2422,8 @@ class BitsUnpacker {
CurrentBitsIndex = 0;
}
+ void advance(uint32_t BitsWidth) { CurrentBitsIndex += BitsWidth; }
+
bool getNextBit() {
assert(isValid());
return Value & (1 << CurrentBitsIndex++);
diff --git a/clang/include/clang/Serialization/ASTWriter.h
b/clang/include/clang/Serialization/ASTWriter.h
index a56929ef0245ee..16ab9583f8ed8e 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -564,11 +564,25 @@ class ASTWriter : public ASTDeserializationListener,
unsigned DeclEnumAbbrev = 0;
unsigned DeclObjCIvarAbbrev = 0;
unsigned DeclCXXMethodAbbrev = 0;
+ unsigned DeclDependentNonTemplateCXXMethodAbbrev = 0;
+ unsigned DeclTemplateCXXMethodAbbrev = 0;
+ unsigned DeclMemberSpecializedCXXMethodAbbrev = 0;
+ unsigned DeclTemplateSpecializedCXXMethodAbbrev = 0;
+ unsigned DeclDependentSpecializationCXXMethodAbbrev = 0;
+ unsigned DeclTemplateTypeParmAbbrev = 0;
+ unsigned DeclUsingShadowAbbrev = 0;
unsigned DeclRefExprAbbrev = 0;
unsigned CharacterLiteralAbbrev = 0;
unsigned IntegerLiteralAbbrev = 0;
unsigned ExprImplicitCastAbbrev = 0;
+ unsigned BinaryOperatorAbbrev = 0;
+ unsigned CompoundAssignOperatorAbbrev = 0;
+ unsigned CallExprAbbrev = 0;
+ unsigned CXXOperatorCallExprAbbrev = 0;
+ unsigned CXXMemberCallExprAbbrev = 0;
+
+ unsigned CompoundStmtAbbrev = 0;
void WriteDeclAbbrevs();
void WriteDecl(ASTContext , Decl *D);
@@ -735,12 +749,42 @@ class ASTWriter : public ASTDeserializationListener,
unsigned getDeclFieldAbbrev() const { return DeclFieldAbbrev; }
unsigned getDeclEnumAbbrev() const { return DeclEnumAbbrev; }
unsigned getDeclObjCIvarAbbrev() const { return DeclObjCIvarAbbrev; }
- unsigned getDeclCXXMethodAbbrev() const { return DeclCXXMethodAbbrev; }
+ unsigned getDeclCXXMethodAbbrev(FunctionDecl::TemplatedKind Kind) const {
+switch (Kind) {
+case FunctionDecl::TK_NonTemplate:
+ return DeclCXXMethodAbbrev;
+case FunctionDecl::TK_FunctionTemplate:
+ return DeclTemplateCXXMethodAbbrev;
+case FunctionDecl::TK_MemberSpecialization:
+ return DeclMemberSpecializedCXXMethodAbbrev;
+case FunctionDecl::TK_FunctionTemplateSpecialization:
+ return DeclTemplateSpecializedCXXMethodAbbrev;
+case FunctionDecl::TK_DependentNonTemplate:
+ return DeclDependentNonTemplateCXXMethodAbbrev;
+case FunctionDecl::TK_DependentFunctionTemplateSpecialization:
+ return DeclDependentSpecializationCXXMethodAbbrev;
+default:
+ llvm_unreachable("Unknwon Template Kind!");
+}
+ }
+ unsigned getDeclTemplateTypeParmAbbrev() const {
+return DeclTemplateTypeParmAbbrev;
+ }
+ unsigned getDeclUsingShadowAbbrev() const { return DeclUsingShadowAbbrev; }
unsigned getDeclRefExprAbbrev() const { return DeclRefExprAbbrev; }
unsigned getCharacterLiteralAbbrev() const { return CharacterLiteralAbbrev; }
unsigned getIntegerLiteralAbbrev() const { return IntegerLiteralAbbrev; }
unsigned getExprImplicitCastAbbrev() const { return ExprImplicitCastAbbrev; }
+ unsigned getBinaryOperatorAbbrev() const { return BinaryOperatorAbbrev; }
+ unsigned getCompoundAssignOperatorAbbrev() const {
+return CompoundAssignOperatorAbbrev;
+ }
+ unsigned getCallExprAbbrev() const { return CallExprAbbrev; }
+ unsigned getCXXOperatorCallExprAbbrev() { return CXXOperatorCallExprAbbrev; }
+ unsigned getCXXMemberCallExprAbbrev() { return CXXMemberCallExprAbbrev; }
+
+ unsigned getCompoundStmtAbbrev() const { return CompoundStmtAbbrev; }
bool hasChain() const { return Chain; }
ASTReader
[clang] 2ce9a79 - [Serialization] Use packed bits to initialize UserDefinedLiteral
Author: Chuanqi Xu
Date: 2023-12-11T18:24:09+08:00
New Revision: 2ce9a799f950678cef844706ecb55a483d3c225b
URL:
https://github.com/llvm/llvm-project/commit/2ce9a799f950678cef844706ecb55a483d3c225b
DIFF:
https://github.com/llvm/llvm-project/commit/2ce9a799f950678cef844706ecb55a483d3c225b.diff
LOG: [Serialization] Use packed bits to initialize UserDefinedLiteral
UserDefinedLiteral is also a sub class of CallExpr but we forgot to
initialize it in the same way as other sub classes of CallExpr.
Added:
Modified:
clang/lib/Serialization/ASTReaderStmt.cpp
Removed:
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp
b/clang/lib/Serialization/ASTReaderStmt.cpp
index d9eedb2e1089fb..b3a6f619372b4a 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3851,11 +3851,14 @@ Stmt *ASTReader::ReadStmtFromStream(ModuleFile ) {
S = new (Context) BuiltinBitCastExpr(Empty);
break;
-case EXPR_USER_DEFINED_LITERAL:
- S = UserDefinedLiteral::CreateEmpty(
- Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields],
- /*HasFPFeatures=*/Record[ASTStmtReader::NumExprFields + 1], Empty);
+case EXPR_USER_DEFINED_LITERAL: {
+ BitsUnpacker CallExprBits(Record[ASTStmtReader::NumExprFields]);
+ auto NumArgs = CallExprBits.getNextBits(/*Width=*/16);
+ auto HasFPFeatures = CallExprBits.getNextBit();
+ S = UserDefinedLiteral::CreateEmpty(Context, NumArgs, HasFPFeatures,
+ Empty);
break;
+}
case EXPR_CXX_STD_INITIALIZER_LIST:
S = new (Context) CXXStdInitializerListExpr(Empty);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 65b12a8 - Recommit [NFC] [Serialization] Packing more bits
Author: Chuanqi Xu
Date: 2023-12-11T16:47:51+08:00
New Revision: 65b12a8af37ffa390ff45c8215904513eb75659e
URL:
https://github.com/llvm/llvm-project/commit/65b12a8af37ffa390ff45c8215904513eb75659e
DIFF:
https://github.com/llvm/llvm-project/commit/65b12a8af37ffa390ff45c8215904513eb75659e.diff
LOG: Recommit [NFC] [Serialization] Packing more bits
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.
Added:
Modified:
clang/include/clang/Serialization/ASTReader.h
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
clang/test/Modules/decl-params-determinisim.m
Removed:
diff --git a/clang/include/clang/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index 7eefdca6815cda..9bb89ec9410911 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2415,12 +2415,7 @@ class BitsUnpacker {
BitsUnpacker(BitsUnpacker &&) = delete;
BitsUnpacker operator=(const BitsUnpacker &) = delete;
BitsUnpacker operator=(BitsUnpacker &&) = delete;
- ~BitsUnpacker() {
-#ifndef NDEBUG
-while (isValid())
- assert(!getNextBit() && "There are unprocessed bits!");
-#endif
- }
+ ~BitsUnpacker() = default;
void updateValue(uint32_t V) {
Value = V;
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index bc16cfc67a24f9..7140a14aefbf9b 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -583,6 +583,9 @@ void ASTDeclReader::Visit(Decl *D) {
}
void ASTDeclReader::VisitDecl(Decl *D) {
+ BitsUnpacker DeclBits(Record.readInt());
+ bool HasStandaloneLexicalDC = DeclBits.getNextBit();
+
if (D->isTemplateParameter() || D->isTemplateParameterPack() ||
isa(D)) {
// We don't want to deserialize the DeclContext of a template
@@ -592,7 +595,8 @@ void ASTDeclReader::VisitDecl(Decl *D) {
// return type of the function). Use the translation unit DeclContext as a
// placeholder.
GlobalDeclID SemaDCIDForTemplateParmDecl = readDeclID();
-GlobalDeclID LexicalDCIDForTemplateParmDecl = readDeclID();
+GlobalDeclID LexicalDCIDForTemplateParmDecl =
+HasStandaloneLexicalDC ? readDeclID() : 0;
if (!LexicalDCIDForTemplateParmDecl)
LexicalDCIDForTemplateParmDecl = SemaDCIDForTemplateParmDecl;
Reader.addPendingDeclContextInfo(D,
@@ -601,7 +605,8 @@ void ASTDeclReader::VisitDecl(Decl *D) {
D->setDeclContext(Reader.getContext().getTranslationUnitDecl());
} else {
auto *SemaDC = readDeclAs();
-auto *LexicalDC = readDeclAs();
+auto *LexicalDC =
+HasStandaloneLexicalDC ? readDeclAs() : nullptr;
if (!LexicalDC)
LexicalDC = SemaDC;
// If the context is a class, we might not have actually merged it yet, in
@@ -618,7 +623,6 @@ void ASTDeclReader::VisitDecl(Decl *D) {
}
D->setLocation(ThisDeclLoc);
- BitsUnpacker DeclBits(Record.readInt());
D->InvalidDecl = DeclBits.getNextBit();
bool HasAttrs = DeclBits.getNextBit();
D->setImplicit(DeclBits.getNextBit());
@@ -765,7 +769,7 @@ ASTDeclReader::RedeclarableResult
ASTDeclReader::VisitTagDecl(TagDecl *TD) {
TD->setCompleteDefinitionRequired(TagDeclBits.getNextBit());
TD->setBraceRange(readSourceRange());
- switch (Record.readInt()) {
+ switch (TagDeclBits.getNextBits(/*Width=*/2)) {
case 0:
break;
case 1: { // ExtInfo
@@ -1089,7 +1093,8 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
FD->setCachedLinkage((Linkage)FunctionDeclBits.getNextBits(/*Width=*/3));
FD->EndRangeLoc = readSourceLocation();
- FD->setDefaultLoc(readSourceLocation());
+ if (FD->isExplicitlyDefaulted())
+FD->setDefaultLoc(readSourceLocation());
FD->ODRHash = Record.readInt();
FD->setHasODRHash(true);
@@ -1703,7 +1708,7 @@ void ASTDeclReader::VisitParmVarDecl(ParmVarDecl *PD) {
unsigned isObjCMethodParam = ParmVarDeclBits.getNextBit();
unsigned scopeDepth = ParmVarDeclBits.getNextBits(/*Width=*/7);
unsigned scopeIndex = ParmVarDeclBits.getNextBits(/*Width=*/8);
- unsigned declQualifier = Record.readInt();
+ unsigned declQualifier = ParmVarDeclBits.getNextBits(/*Width=*/7);
if (isObjCMethodParam) {
assert(scopeDepth == 0);
PD->setObjCMethodScopeInfo(scopeIndex);
@@ -1716,7 +1721,9 @@ void ASTDeclReader::VisitParmVarDecl(ParmVarDecl *PD) {
PD->ParmVarDeclBits.HasInheritedDefaultArg = ParmVarDeclBits.getNextBit();
if (ParmVarDeclBits.getNextBit()) // hasUninstantiatedDefaultArg.
[clang] 9a46518 - Revert "[clang] Remove unused variable 'ExprDependenceBits' in ASTWriterDecl.cpp (NFC)"
Author: Chuanqi Xu
Date: 2023-12-11T14:15:16+08:00
New Revision: 9a46518630869e7fcb495e72378abdbedf68d40d
URL:
https://github.com/llvm/llvm-project/commit/9a46518630869e7fcb495e72378abdbedf68d40d
DIFF:
https://github.com/llvm/llvm-project/commit/9a46518630869e7fcb495e72378abdbedf68d40d.diff
LOG: Revert "[clang] Remove unused variable 'ExprDependenceBits' in
ASTWriterDecl.cpp (NFC)"
This reverts commit 10951050b5f371eb3e7cacce1691c4eb2fe2eab5.
This should be part of 8c334627818437180176b16b1932 to revert
9406ea3fe32e59a7d2 completely.
Added:
Modified:
clang/lib/Serialization/ASTWriterDecl.cpp
Removed:
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp
b/clang/lib/Serialization/ASTWriterDecl.cpp
index a6fc41fdb4ce4d..bf082e5b8eac61 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2346,6 +2346,7 @@ void ASTWriter::WriteDeclAbbrevs() {
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));
DeclCXXMethodAbbrev = Stream.EmitAbbrev(std::move(Abv));
+ unsigned ExprDependenceBits = llvm::BitWidth;
// Abbreviation for EXPR_DECL_REF
Abv = std::make_shared();
Abv->Add(BitCodeAbbrevOp(serialization::EXPR_DECL_REF));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 8c33462 - Revert "[NFC] [Serialization] Packing more bits"
Author: Chuanqi Xu
Date: 2023-12-11T14:06:32+08:00
New Revision: 8c334627818437180176b16b1932b2a26372d8ae
URL:
https://github.com/llvm/llvm-project/commit/8c334627818437180176b16b1932b2a26372d8ae
DIFF:
https://github.com/llvm/llvm-project/commit/8c334627818437180176b16b1932b2a26372d8ae.diff
LOG: Revert "[NFC] [Serialization] Packing more bits"
This reverts commit 9406ea3fe32e59a7d28de0dcbd0317b4cdfa4c62.
There are build bots complaining this. Revert it first to try to keep
the bots green.
Added:
Modified:
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
clang/test/Modules/decl-params-determinisim.m
Removed:
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 7140a14aefbf9b..bc16cfc67a24f9 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -583,9 +583,6 @@ void ASTDeclReader::Visit(Decl *D) {
}
void ASTDeclReader::VisitDecl(Decl *D) {
- BitsUnpacker DeclBits(Record.readInt());
- bool HasStandaloneLexicalDC = DeclBits.getNextBit();
-
if (D->isTemplateParameter() || D->isTemplateParameterPack() ||
isa(D)) {
// We don't want to deserialize the DeclContext of a template
@@ -595,8 +592,7 @@ void ASTDeclReader::VisitDecl(Decl *D) {
// return type of the function). Use the translation unit DeclContext as a
// placeholder.
GlobalDeclID SemaDCIDForTemplateParmDecl = readDeclID();
-GlobalDeclID LexicalDCIDForTemplateParmDecl =
-HasStandaloneLexicalDC ? readDeclID() : 0;
+GlobalDeclID LexicalDCIDForTemplateParmDecl = readDeclID();
if (!LexicalDCIDForTemplateParmDecl)
LexicalDCIDForTemplateParmDecl = SemaDCIDForTemplateParmDecl;
Reader.addPendingDeclContextInfo(D,
@@ -605,8 +601,7 @@ void ASTDeclReader::VisitDecl(Decl *D) {
D->setDeclContext(Reader.getContext().getTranslationUnitDecl());
} else {
auto *SemaDC = readDeclAs();
-auto *LexicalDC =
-HasStandaloneLexicalDC ? readDeclAs() : nullptr;
+auto *LexicalDC = readDeclAs();
if (!LexicalDC)
LexicalDC = SemaDC;
// If the context is a class, we might not have actually merged it yet, in
@@ -623,6 +618,7 @@ void ASTDeclReader::VisitDecl(Decl *D) {
}
D->setLocation(ThisDeclLoc);
+ BitsUnpacker DeclBits(Record.readInt());
D->InvalidDecl = DeclBits.getNextBit();
bool HasAttrs = DeclBits.getNextBit();
D->setImplicit(DeclBits.getNextBit());
@@ -769,7 +765,7 @@ ASTDeclReader::RedeclarableResult
ASTDeclReader::VisitTagDecl(TagDecl *TD) {
TD->setCompleteDefinitionRequired(TagDeclBits.getNextBit());
TD->setBraceRange(readSourceRange());
- switch (TagDeclBits.getNextBits(/*Width=*/2)) {
+ switch (Record.readInt()) {
case 0:
break;
case 1: { // ExtInfo
@@ -1093,8 +1089,7 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
FD->setCachedLinkage((Linkage)FunctionDeclBits.getNextBits(/*Width=*/3));
FD->EndRangeLoc = readSourceLocation();
- if (FD->isExplicitlyDefaulted())
-FD->setDefaultLoc(readSourceLocation());
+ FD->setDefaultLoc(readSourceLocation());
FD->ODRHash = Record.readInt();
FD->setHasODRHash(true);
@@ -1708,7 +1703,7 @@ void ASTDeclReader::VisitParmVarDecl(ParmVarDecl *PD) {
unsigned isObjCMethodParam = ParmVarDeclBits.getNextBit();
unsigned scopeDepth = ParmVarDeclBits.getNextBits(/*Width=*/7);
unsigned scopeIndex = ParmVarDeclBits.getNextBits(/*Width=*/8);
- unsigned declQualifier = ParmVarDeclBits.getNextBits(/*Width=*/7);
+ unsigned declQualifier = Record.readInt();
if (isObjCMethodParam) {
assert(scopeDepth == 0);
PD->setObjCMethodScopeInfo(scopeIndex);
@@ -1721,9 +1716,7 @@ void ASTDeclReader::VisitParmVarDecl(ParmVarDecl *PD) {
PD->ParmVarDeclBits.HasInheritedDefaultArg = ParmVarDeclBits.getNextBit();
if (ParmVarDeclBits.getNextBit()) // hasUninstantiatedDefaultArg.
PD->setUninstantiatedDefaultArg(Record.readExpr());
-
- if (ParmVarDeclBits.getNextBit()) // Valid explicit object parameter
-PD->ExplicitObjectParameterIntroducerLoc = Record.readSourceLocation();
+ PD->ExplicitObjectParameterIntroducerLoc = Record.readSourceLocation();
// FIXME: If this is a redeclaration of a function from another module,
handle
// inheritance of default arguments.
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp
b/clang/lib/Serialization/ASTReaderStmt.cpp
index 865322ec0782cd..d7d0c0e5bb21b4 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -108,7 +108,7 @@ namespace clang {
/// The number of record fields required for the Expr class
/// itself.
-static const unsigned NumExprFields = NumStmtFields + 2;
+static const
[clang] 9406ea3 - [NFC] [Serialization] Packing more bits
Author: Chuanqi Xu
Date: 2023-12-11T10:18:12+08:00
New Revision: 9406ea3fe32e59a7d28de0dcbd0317b4cdfa4c62
URL:
https://github.com/llvm/llvm-project/commit/9406ea3fe32e59a7d28de0dcbd0317b4cdfa4c62
DIFF:
https://github.com/llvm/llvm-project/commit/9406ea3fe32e59a7d28de0dcbd0317b4cdfa4c62.diff
LOG: [NFC] [Serialization] Packing more bits
This patch tries to reduce the size of the BMIs by packing more bits
into an unsigned integer.
Added:
Modified:
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
clang/test/Modules/decl-params-determinisim.m
Removed:
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index bc16cfc67a24f9..7140a14aefbf9b 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -583,6 +583,9 @@ void ASTDeclReader::Visit(Decl *D) {
}
void ASTDeclReader::VisitDecl(Decl *D) {
+ BitsUnpacker DeclBits(Record.readInt());
+ bool HasStandaloneLexicalDC = DeclBits.getNextBit();
+
if (D->isTemplateParameter() || D->isTemplateParameterPack() ||
isa(D)) {
// We don't want to deserialize the DeclContext of a template
@@ -592,7 +595,8 @@ void ASTDeclReader::VisitDecl(Decl *D) {
// return type of the function). Use the translation unit DeclContext as a
// placeholder.
GlobalDeclID SemaDCIDForTemplateParmDecl = readDeclID();
-GlobalDeclID LexicalDCIDForTemplateParmDecl = readDeclID();
+GlobalDeclID LexicalDCIDForTemplateParmDecl =
+HasStandaloneLexicalDC ? readDeclID() : 0;
if (!LexicalDCIDForTemplateParmDecl)
LexicalDCIDForTemplateParmDecl = SemaDCIDForTemplateParmDecl;
Reader.addPendingDeclContextInfo(D,
@@ -601,7 +605,8 @@ void ASTDeclReader::VisitDecl(Decl *D) {
D->setDeclContext(Reader.getContext().getTranslationUnitDecl());
} else {
auto *SemaDC = readDeclAs();
-auto *LexicalDC = readDeclAs();
+auto *LexicalDC =
+HasStandaloneLexicalDC ? readDeclAs() : nullptr;
if (!LexicalDC)
LexicalDC = SemaDC;
// If the context is a class, we might not have actually merged it yet, in
@@ -618,7 +623,6 @@ void ASTDeclReader::VisitDecl(Decl *D) {
}
D->setLocation(ThisDeclLoc);
- BitsUnpacker DeclBits(Record.readInt());
D->InvalidDecl = DeclBits.getNextBit();
bool HasAttrs = DeclBits.getNextBit();
D->setImplicit(DeclBits.getNextBit());
@@ -765,7 +769,7 @@ ASTDeclReader::RedeclarableResult
ASTDeclReader::VisitTagDecl(TagDecl *TD) {
TD->setCompleteDefinitionRequired(TagDeclBits.getNextBit());
TD->setBraceRange(readSourceRange());
- switch (Record.readInt()) {
+ switch (TagDeclBits.getNextBits(/*Width=*/2)) {
case 0:
break;
case 1: { // ExtInfo
@@ -1089,7 +1093,8 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
FD->setCachedLinkage((Linkage)FunctionDeclBits.getNextBits(/*Width=*/3));
FD->EndRangeLoc = readSourceLocation();
- FD->setDefaultLoc(readSourceLocation());
+ if (FD->isExplicitlyDefaulted())
+FD->setDefaultLoc(readSourceLocation());
FD->ODRHash = Record.readInt();
FD->setHasODRHash(true);
@@ -1703,7 +1708,7 @@ void ASTDeclReader::VisitParmVarDecl(ParmVarDecl *PD) {
unsigned isObjCMethodParam = ParmVarDeclBits.getNextBit();
unsigned scopeDepth = ParmVarDeclBits.getNextBits(/*Width=*/7);
unsigned scopeIndex = ParmVarDeclBits.getNextBits(/*Width=*/8);
- unsigned declQualifier = Record.readInt();
+ unsigned declQualifier = ParmVarDeclBits.getNextBits(/*Width=*/7);
if (isObjCMethodParam) {
assert(scopeDepth == 0);
PD->setObjCMethodScopeInfo(scopeIndex);
@@ -1716,7 +1721,9 @@ void ASTDeclReader::VisitParmVarDecl(ParmVarDecl *PD) {
PD->ParmVarDeclBits.HasInheritedDefaultArg = ParmVarDeclBits.getNextBit();
if (ParmVarDeclBits.getNextBit()) // hasUninstantiatedDefaultArg.
PD->setUninstantiatedDefaultArg(Record.readExpr());
- PD->ExplicitObjectParameterIntroducerLoc = Record.readSourceLocation();
+
+ if (ParmVarDeclBits.getNextBit()) // Valid explicit object parameter
+PD->ExplicitObjectParameterIntroducerLoc = Record.readSourceLocation();
// FIXME: If this is a redeclaration of a function from another module,
handle
// inheritance of default arguments.
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp
b/clang/lib/Serialization/ASTReaderStmt.cpp
index d7d0c0e5bb21b4..865322ec0782cd 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -108,7 +108,7 @@ namespace clang {
/// The number of record fields required for the Expr class
/// itself.
-static const unsigned NumExprFields = NumStmtFields + 4;
+static const unsigned NumExprFields = NumStmtFields + 2;
/// Read and
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
ChuanqiXu9 wrote: @sam-mccall ping! https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] b768b39 - [C++20] [Modules] Skip Writing diagnostic options, header search paths and pragma diagnostic mappings
Author: Chuanqi Xu
Date: 2023-12-07T16:54:00+08:00
New Revision: b768b393429419d27e3f76518842136bac9d5b25
URL:
https://github.com/llvm/llvm-project/commit/b768b393429419d27e3f76518842136bac9d5b25
DIFF:
https://github.com/llvm/llvm-project/commit/b768b393429419d27e3f76518842136bac9d5b25.diff
LOG: [C++20] [Modules] Skip Writing diagnostic options, header search paths and
pragma diagnostic mappings
It simply wastes of space and time to write diagnostic options, header
search paths and pragma diagnostic mappings for C++20 Named modules.
This patch tries to avoid the unnecessary writings.
Added:
Modified:
clang/include/clang/Frontend/FrontendActions.h
clang/lib/Frontend/FrontendActions.cpp
Removed:
diff --git a/clang/include/clang/Frontend/FrontendActions.h
b/clang/include/clang/Frontend/FrontendActions.h
index 3940e00eeb8db..fcce31ac0590f 100644
--- a/clang/include/clang/Frontend/FrontendActions.h
+++ b/clang/include/clang/Frontend/FrontendActions.h
@@ -151,6 +151,9 @@ class GenerateModuleInterfaceAction : public
GenerateModuleAction {
private:
bool BeginSourceFileAction(CompilerInstance ) override;
+ std::unique_ptr CreateASTConsumer(CompilerInstance ,
+ StringRef InFile) override;
+
std::unique_ptr
CreateOutputFile(CompilerInstance , StringRef InFile) override;
};
diff --git a/clang/lib/Frontend/FrontendActions.cpp
b/clang/lib/Frontend/FrontendActions.cpp
index 2afcf1cf9f68c..c1d6e71455365 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -258,6 +258,16 @@ bool GenerateModuleInterfaceAction::BeginSourceFileAction(
return GenerateModuleAction::BeginSourceFileAction(CI);
}
+std::unique_ptr
+GenerateModuleInterfaceAction::CreateASTConsumer(CompilerInstance ,
+ StringRef InFile) {
+ CI.getHeaderSearchOpts().ModulesSkipDiagnosticOptions = true;
+ CI.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = true;
+ CI.getHeaderSearchOpts().ModulesSkipPragmaDiagnosticMappings = true;
+
+ return GenerateModuleAction::CreateASTConsumer(CI, InFile);
+}
+
std::unique_ptr
GenerateModuleInterfaceAction::CreateOutputFile(CompilerInstance ,
StringRef InFile) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Enable -fmodules-embed-all-files by default for named modules (PR #74419)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/74419 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Enable -fmodules-embed-all-files by default for named modules (PR #74419)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/74419
Close https://github.com/llvm/llvm-project/issues/72383
It looks incorrect or odd for the BMIs to dependent on the real files. It
prevents we moving the BMIs and the distributed builds. And it looks like there
is nothing beneficial we got by not enabling this. Also the cost is relatively
small. See the discussion in the above issue for details.
>From b28f94d795582ef13e6fe8ce111c9638f4ed7f30 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Tue, 5 Dec 2023 14:24:10 +0800
Subject: [PATCH] [C++20] [Modules] Enable -fmodules-embed-all-files by default
for named modules
---
clang/include/clang/Driver/Types.h| 3 +++
clang/lib/Driver/ToolChains/Clang.cpp | 7 ++
clang/lib/Driver/Types.cpp| 4
.../test/Driver/modules-embed-all-files.cppm | 22 +++
.../Serialization/ForceCheckFileInputTest.cpp | 2 ++
5 files changed, 38 insertions(+)
create mode 100644 clang/test/Driver/modules-embed-all-files.cppm
diff --git a/clang/include/clang/Driver/Types.h
b/clang/include/clang/Driver/Types.h
index 121b58a6b477d..53340bdc64f70 100644
--- a/clang/include/clang/Driver/Types.h
+++ b/clang/include/clang/Driver/Types.h
@@ -80,6 +80,9 @@ namespace types {
/// isCXX - Is this a "C++" input (C++ and Obj-C++ sources and headers).
bool isCXX(ID Id);
+ /// isCXXModuleUnit - Is this a "C++ module unit" input.
+ bool isCXXModuleUnit(ID Id);
+
/// Is this LLVM IR.
bool isLLVMIR(ID Id);
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp
b/clang/lib/Driver/ToolChains/Clang.cpp
index f02f7c841b91f..9081258626eaa 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3817,6 +3817,13 @@ static bool RenderModulesOptions(Compilation , const
Driver ,
Args.ClaimAllArgs(options::OPT_fmodules_disable_diagnostic_validation);
}
+ // '-fmodules-embed-all-files' should be enabled by default for C++20 named
+ // modules.
+ // See the discussion in https://github.com/llvm/llvm-project/issues/72383.
+ if (types::isCXXModuleUnit(Input.getType()) && HaveModules) {
+CmdArgs.push_back("-fmodules-embed-all-files");
+ }
+
// Claim `-fmodule-output` and `-fmodule-output=` to avoid unused warnings.
Args.ClaimAllArgs(options::OPT_fmodule_output);
Args.ClaimAllArgs(options::OPT_fmodule_output_EQ);
diff --git a/clang/lib/Driver/Types.cpp b/clang/lib/Driver/Types.cpp
index 08df34ade7b65..52e5d9748d375 100644
--- a/clang/lib/Driver/Types.cpp
+++ b/clang/lib/Driver/Types.cpp
@@ -249,6 +249,10 @@ bool types::isCXX(ID Id) {
}
}
+bool types::isCXXModuleUnit(ID Id) {
+ return Id == TY_CXXModule || Id == TY_PP_CXXModule;
+}
+
bool types::isLLVMIR(ID Id) {
switch (Id) {
default:
diff --git a/clang/test/Driver/modules-embed-all-files.cppm
b/clang/test/Driver/modules-embed-all-files.cppm
new file mode 100644
index 0..86a4389fb4319
--- /dev/null
+++ b/clang/test/Driver/modules-embed-all-files.cppm
@@ -0,0 +1,22 @@
+// Tests that we'll enable -fmodules-embed-all-files for C++20 module units.
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang %t/a.cppm -### 2>&1 | FileCheck %t/a.cppm --check-prefix=PRE20
+// RUN: %clang -std=c++20 %t/a.cppm -### 2>&1 | FileCheck %t/a.cppm
+
+// RUN: %clang %t/a.cpp -### 2>&1 | FileCheck %t/a.cpp
--check-prefix=NO-CXX-MODULE
+// RUN: %clang -std=c++20 %t/a.cpp -### 2>&1 | FileCheck %t/a.cpp
--check-prefix=NO-CXX-MODULE
+// RUN: %clang -std=c++20 -x c++-module %t/a.cpp -### 2>&1 | FileCheck %t/a.cpp
+
+//--- a.cppm
+
+// PRE20-NOT: -fmodules-embed-all-files
+// CHECK: -fmodules-embed-all-files
+
+//--- a.cpp
+
+// NO-CXX-MODULE-NOT: -fmodules-embed-all-files
+// CHECK: -fmodules-embed-all-files
diff --git a/clang/unittests/Serialization/ForceCheckFileInputTest.cpp
b/clang/unittests/Serialization/ForceCheckFileInputTest.cpp
index ed0daa43436eb..eeb77914107cd 100644
--- a/clang/unittests/Serialization/ForceCheckFileInputTest.cpp
+++ b/clang/unittests/Serialization/ForceCheckFileInputTest.cpp
@@ -76,6 +76,7 @@ export int aa = 43;
createInvocation(Args, CIOpts);
EXPECT_TRUE(Invocation);
Invocation->getFrontendOpts().DisableFree = false;
+Invocation->getFrontendOpts().ModulesEmbedAllFiles = false;
auto Buf = CIOpts.VFS->getBufferForFile("a.cppm");
EXPECT_TRUE(Buf);
@@ -115,6 +116,7 @@ export int aa = 43;
createInvocation(Args, CIOpts);
EXPECT_TRUE(Invocation);
Invocation->getFrontendOpts().DisableFree = false;
+Invocation->getFrontendOpts().ModulesEmbedAllFiles = false;
CompilerInstance Clang;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] db3d0e4 - [C++20] [Modules] Don't diagnose on invisible namesapce
Author: Chuanqi Xu
Date: 2023-12-04T17:05:27+08:00
New Revision: db3d0e4dfa34e59fab90c0726a6722f82db48462
URL:
https://github.com/llvm/llvm-project/commit/db3d0e4dfa34e59fab90c0726a6722f82db48462
DIFF:
https://github.com/llvm/llvm-project/commit/db3d0e4dfa34e59fab90c0726a6722f82db48462.diff
LOG: [C++20] [Modules] Don't diagnose on invisible namesapce
Close https://github.com/llvm/llvm-project/issues/73893
As the issue shows, generally, the diagnose information for
invisible namespace is confusing more than helpful. Also this patch
implements the same solution as suggested in the issue: don't diagnose
on invisible namespace.
Added:
clang/test/Modules/pr73893.cppm
Modified:
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaLookup.cpp
clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp
clang/test/CXX/module/module.interface/p2.cpp
clang/test/Modules/submodules-merge-defs.cpp
Removed:
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 7385eac48d8c8..8fedf41d8424a 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -12114,6 +12114,24 @@ class NamespaceValidatorCCC final : public
CorrectionCandidateCallback {
}
+static void DiagnoseInvisibleNamespace(const TypoCorrection ,
+ Sema ) {
+ auto *ND = cast(Corrected.getFoundDecl());
+ Module *M = ND->getOwningModule();
+ assert(M && "hidden namespace definition not in a module?");
+
+ if (M->isExplicitGlobalModule())
+S.Diag(Corrected.getCorrectionRange().getBegin(),
+ diag::err_module_unimported_use_header)
+<< (int)Sema::MissingImportKind::Declaration <<
Corrected.getFoundDecl()
+<< /*Header Name*/ false;
+ else
+S.Diag(Corrected.getCorrectionRange().getBegin(),
+ diag::err_module_unimported_use)
+<< (int)Sema::MissingImportKind::Declaration <<
Corrected.getFoundDecl()
+<< M->getTopLevelModuleName();
+}
+
static bool TryNamespaceTypoCorrection(Sema , LookupResult , Scope *Sc,
CXXScopeSpec ,
SourceLocation IdentLoc,
@@ -12123,7 +12141,16 @@ static bool TryNamespaceTypoCorrection(Sema ,
LookupResult , Scope *Sc,
if (TypoCorrection Corrected =
S.CorrectTypo(R.getLookupNameInfo(), R.getLookupKind(), Sc, , CCC,
Sema::CTK_ErrorRecovery)) {
-if (DeclContext *DC = S.computeDeclContext(SS, false)) {
+// Generally we find it is confusing more than helpful to diagnose the
+// invisible namespace.
+// See https://github.com/llvm/llvm-project/issues/73893.
+//
+// However, we should diagnose when the users are trying to using an
+// invisible namespace. So we handle the case specially here.
+if (isa_and_nonnull(Corrected.getFoundDecl()) &&
+Corrected.requiresImport()) {
+ DiagnoseInvisibleNamespace(Corrected, S);
+} else if (DeclContext *DC = S.computeDeclContext(SS, false)) {
std::string CorrectedStr(Corrected.getAsString(S.getLangOpts()));
bool DroppedSpecifier = Corrected.WillReplaceSpecifier() &&
Ident->getName().equals(CorrectedStr);
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 54b48f7ab80eb..996f8b57233ba 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -5712,6 +5712,11 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc,
const NamedDecl *Decl,
MissingImportKind MIK, bool Recover) {
assert(!Modules.empty());
+ // See https://github.com/llvm/llvm-project/issues/73893. It is generally
+ // confusing than helpful to show the namespace is not visible.
+ if (isa(Decl))
+return;
+
auto NotePrevious = [&] {
// FIXME: Suppress the note backtrace even under
// -fdiagnostics-show-note-include-stack. We don't care how this
diff --git a/clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp
b/clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp
index c1a824bd51493..a27946bd90a46 100644
--- a/clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp
+++ b/clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp
@@ -50,9 +50,7 @@ void test() {
auto x = make();
// error: R and R::f are not visible here
- R::f(x); // expected-error {{declaration of 'R' must be imported from module
'N' before it is required}}
- // expected-n...@n.cpp:4 {{declaration here is not visible}}
- // expected-error@-2 {{no type named 'f' in namespace 'R'}}
+ R::f(x); // expected-error {{no type named 'f' in namespace 'R'}}
f(x); // Found by [basic.lookup.argdep] / p4.3
diff --git a/clang/test/CXX/module/module.interface/p2.cpp
b/clang/test/CXX/module/module.interface/p2.cpp
index
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
ChuanqiXu9 wrote: @sam-mccall ping https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,14 +129,48 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
- if (const auto *CE = dyn_cast(E))
-if (const auto *Proto =
-CE->getMethodDecl()->getType()->getAs())
- if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
- Proto->canThrow() == CT_Cannot)
-return false;
- return true;
+// Check if function can throw based on prototype noexcept, also works for
+// destructors which are implicitly noexcept but can be marked noexcept(false).
+static bool FunctionCanThrow(const FunctionDecl *D) {
+ const auto *Proto = D->getType()->getAs();
+ if (!Proto) {
+// Function proto is not found, we conservatively assume throwing.
+return true;
+ }
+ return !isNoexceptExceptionSpec(Proto->getExceptionSpecType()) ||
+ Proto->canThrow() != CT_Cannot;
+}
+
+static bool ResumeStmtCanThrow(const Stmt *S) {
+ if (const auto *CE = dyn_cast(S)) {
+const auto *Callee = CE->getDirectCallee();
+if (!Callee)
+ // We don't have direct callee. Conservatively assume throwing.
+ return true;
+
+if (FunctionCanThrow(Callee))
+ return true;
ChuanqiXu9 wrote:
Please don't land it when you have different opinions.
It should be safe to return false directly if we know the function is nothrow.
Then it may be slightly more efficient.
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 649e811 - [C++20] [Modules] Handling capturing strucuted bindings
Author: Chuanqi Xu
Date: 2023-11-29T11:45:31+08:00
New Revision: 649e8111a95ae0d8814576e9ca74823572ee404b
URL:
https://github.com/llvm/llvm-project/commit/649e8111a95ae0d8814576e9ca74823572ee404b
DIFF:
https://github.com/llvm/llvm-project/commit/649e8111a95ae0d8814576e9ca74823572ee404b.diff
LOG: [C++20] [Modules] Handling capturing strucuted bindings
Close https://github.com/llvm/llvm-project/issues/72828.
This should be an overlook that we extend the type of captures but we
forgot to fix it in deserializer side.
Added:
clang/test/Modules/pr72828.cppm
Modified:
clang/lib/Serialization/ASTReaderDecl.cpp
Removed:
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 79817b3fb1ec3a0..bc16cfc67a24f9f 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2032,7 +2032,7 @@ void ASTDeclReader::ReadCXXDefinitionData(
break;
case LCK_ByCopy:
case LCK_ByRef:
-auto *Var = readDeclAs();
+auto *Var = readDeclAs();
SourceLocation EllipsisLoc = readSourceLocation();
new (ToCapture) Capture(Loc, IsImplicit, Kind, Var, EllipsisLoc);
ToCapture++;
diff --git a/clang/test/Modules/pr72828.cppm b/clang/test/Modules/pr72828.cppm
new file mode 100644
index 000..574523188507a17
--- /dev/null
+++ b/clang/test/Modules/pr72828.cppm
@@ -0,0 +1,24 @@
+// Test that we can handle capturing structured bindings.
+//
+// RUN: rm -fr %t
+// RUN: mkdir %t
+//
+// RUN: %clang_cc1 -std=c++23 -triple %itanium_abi_triple \
+// RUN: %s -emit-module-interface -o %t/m.pcm
+// RUN: %clang_cc1 -std=c++23 -triple %itanium_abi_triple \
+// RUN: -S -emit-llvm -disable-llvm-passes %t/m.pcm \
+// RUN: -o - | FileCheck %s
+
+export module m;
+
+struct s {
+ int m;
+};
+
+void f() {
+ auto [x] = s();
+ [x] {};
+}
+
+// Check that we can generate the LLVM IR expectedly.
+// CHECK: define{{.*}}@_ZGIW1m
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,14 +129,48 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
- if (const auto *CE = dyn_cast(E))
-if (const auto *Proto =
-CE->getMethodDecl()->getType()->getAs())
- if (isNoexceptExceptionSpec(Proto->getExceptionSpecType()) &&
- Proto->canThrow() == CT_Cannot)
-return false;
- return true;
+// Check if function can throw based on prototype noexcept, also works for
+// destructors which are implicitly noexcept but can be marked noexcept(false).
+static bool FunctionCanThrow(const FunctionDecl *D) {
+ const auto *Proto = D->getType()->getAs();
+ if (!Proto) {
+// Function proto is not found, we conservatively assume throwing.
+return true;
+ }
+ return !isNoexceptExceptionSpec(Proto->getExceptionSpecType()) ||
+ Proto->canThrow() != CT_Cannot;
+}
+
+static bool ResumeStmtCanThrow(const Stmt *S) {
+ if (const auto *CE = dyn_cast(S)) {
+const auto *Callee = CE->getDirectCallee();
+if (!Callee)
+ // We don't have direct callee. Conservatively assume throwing.
+ return true;
+
+if (FunctionCanThrow(Callee))
+ return true;
ChuanqiXu9 wrote:
```suggestion
if (!FunctionCanThrow(Callee))
return false;
```
nit: This reads better. In case we know the called function is nothrow, we
don't need to check it further recursively especially await_resume doesn't have
arguments except `this`.
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/ChuanqiXu9 approved this pull request. LGTM with nit. https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,7 +130,14 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr ) {
+ const Expr *E = S.getResumeExpr();
+
+ // If the return type of await_resume is not void, get the CXXMemberCallExpr
+ // from its subexpr.
+ if (const auto *BindTempExpr = dyn_cast(E)) {
+E = BindTempExpr->getSubExpr();
+ }
ChuanqiXu9 wrote:
Maybe it is not a such big task as you imaged, you can take a look at
`Sema::checkFinalSuspendNoThrow` and we should be able to make a simpler change
than that.
The reason why I don't like the current pattern is that, every time I wrote:
```
if (auto *WantedExpressionType = isa(E))
E = WantedExpressionType->subExpr();
...
```
I found I always missed some situations.
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Introduce reusable modules builder (PR #73483)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/73483 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
ChuanqiXu9 wrote: @sam-mccall I created https://github.com/llvm/llvm-project/pull/73483 as the following patches to reuse built module files. I think that patch should be necessary since the current patch may waste too many time and space since it won't reuse the module files across source files. I am wondering if we can land the current patch if it doesn't have big issues. Then we can turn into the next iterations. https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Introduce reusable modules builder (PR #73483)
https://github.com/ChuanqiXu9 converted_to_draft https://github.com/llvm/llvm-project/pull/73483 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/66462
>From 32010ae7e0a47cd4a70a9401980b32ed1d3e10f6 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Fri, 15 Sep 2023 11:33:53 +0800
Subject: [PATCH] [clangd] [C++20] [Modules] Introduce initial support for
C++20 Modules
Alternatives to https://reviews.llvm.org/D153114.
Try to address https://github.com/clangd/clangd/issues/1293.
See the links for design ideas. We want to have some initial support in
clang18.
This is the initial support for C++20 Modules in clangd.
As suggested by sammccall in https://reviews.llvm.org/D153114,
we should minimize the scope of the initial patch to make it easier
to review and understand so that every one are in the same page:
> Don't attempt any cross-file or cross-version coordination: i.e. don't
> try to reuse BMIs between different files, don't try to reuse BMIs
> between (preamble) reparses of the same file, don't try to persist the
> module graph. Instead, when building a preamble, synchronously scan
> for the module graph, build the required PCMs on the single preamble
> thread with filenames private to that preamble, and then proceed to
> build the preamble.
And this patch reflects the above opinions.
---
clang-tools-extra/clangd/CMakeLists.txt | 4 +
clang-tools-extra/clangd/ClangdServer.cpp | 2 +
clang-tools-extra/clangd/ClangdServer.h | 3 +
.../clangd/GlobalCompilationDatabase.cpp | 23 ++
.../clangd/GlobalCompilationDatabase.h| 14 +
.../clangd/ModuleDependencyScanner.cpp| 81 +
.../clangd/ModuleDependencyScanner.h | 106 ++
clang-tools-extra/clangd/ModulesBuilder.cpp | 339 ++
clang-tools-extra/clangd/ModulesBuilder.h | 71
clang-tools-extra/clangd/ParsedAST.cpp| 7 +
clang-tools-extra/clangd/Preamble.cpp | 28 +-
clang-tools-extra/clangd/Preamble.h | 10 +
.../clangd/PrerequisiteModules.h | 87 +
clang-tools-extra/clangd/ProjectModules.cpp | 62
clang-tools-extra/clangd/ProjectModules.h | 55 +++
clang-tools-extra/clangd/TUScheduler.cpp | 50 ++-
clang-tools-extra/clangd/TUScheduler.h| 7 +
clang-tools-extra/clangd/test/CMakeLists.txt | 1 +
clang-tools-extra/clangd/test/modules.test| 83 +
clang-tools-extra/clangd/tool/Check.cpp | 13 +-
clang-tools-extra/clangd/tool/ClangdMain.cpp | 8 +
.../clangd/unittests/CMakeLists.txt | 2 +
.../clangd/unittests/CodeCompleteTests.cpp| 22 +-
.../clangd/unittests/FileIndexTests.cpp | 4 +-
.../unittests/ModuleDependencyScannerTest.cpp | 176 +
.../clangd/unittests/ModulesTestSetup.h | 103 ++
.../clangd/unittests/ParsedASTTests.cpp | 8 +-
.../clangd/unittests/PreambleTests.cpp| 6 +-
.../unittests/PrerequisiteModulesTest.cpp | 224
clang-tools-extra/clangd/unittests/TestTU.cpp | 14 +-
clang-tools-extra/docs/ReleaseNotes.rst | 3 +
31 files changed, 1576 insertions(+), 40 deletions(-)
create mode 100644 clang-tools-extra/clangd/ModuleDependencyScanner.cpp
create mode 100644 clang-tools-extra/clangd/ModuleDependencyScanner.h
create mode 100644 clang-tools-extra/clangd/ModulesBuilder.cpp
create mode 100644 clang-tools-extra/clangd/ModulesBuilder.h
create mode 100644 clang-tools-extra/clangd/PrerequisiteModules.h
create mode 100644 clang-tools-extra/clangd/ProjectModules.cpp
create mode 100644 clang-tools-extra/clangd/ProjectModules.h
create mode 100644 clang-tools-extra/clangd/test/modules.test
create mode 100644
clang-tools-extra/clangd/unittests/ModuleDependencyScannerTest.cpp
create mode 100644 clang-tools-extra/clangd/unittests/ModulesTestSetup.h
create mode 100644
clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
diff --git a/clang-tools-extra/clangd/CMakeLists.txt
b/clang-tools-extra/clangd/CMakeLists.txt
index 3911fb6c6c746a8..242a8ad2e350be7 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -97,7 +97,10 @@ add_clang_library(clangDaemon
IncludeFixer.cpp
InlayHints.cpp
JSONTransport.cpp
+ ModuleDependencyScanner.cpp
+ ModulesBuilder.cpp
PathMapping.cpp
+ ProjectModules.cpp
Protocol.cpp
Quality.cpp
ParsedAST.cpp
@@ -161,6 +164,7 @@ clang_target_link_libraries(clangDaemon
clangAST
clangASTMatchers
clangBasic
+ clangDependencyScanning
clangDriver
clangFormat
clangFrontend
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp
b/clang-tools-extra/clangd/ClangdServer.cpp
index 13d788162817fb4..8ba4b38c420ab6a 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -202,6 +202,7 @@ ClangdServer::Options::operator TUScheduler::Options()
const {
Opts.UpdateDebounce = UpdateDebounce;
Opts.ContextProvider = ContextProvider;
Opts.PreambleThrottler = PreambleThrottler;
+
[clang] [clang][AST][ASTMerge] prevent AST nodes from being deallocated early (PR #73096)
ChuanqiXu9 wrote: > Debug the https://github.com/llvm/llvm-project/issues/72783 can prove it. > Address interval (local from 0x3a9a00 to 0x3aaa00) allocated by allocator > contains a IdentifierInfo variable (local address:0x3aa190) whose address is > freed early. In this case, it looks better to extract the use-after-free variable only instead of extracting the whole ASTUnit. > As system header like stdio.h or math.h can't be put into test, it's hard to > add testcase. Could anyone give me some guidance? Thanks in advance! Generally, we need to reduce them in this case. e.g., we need to preprocess them, and remove unncessary parts until we can't. It is time consuming but it is worthy. https://github.com/llvm/llvm-project/pull/73096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -38,9 +39,52 @@ Task coro_create() {
co_return;
}
-// CHECK-LABEL: define{{.*}} ptr @_Z11coro_createv(
+// CHECK-LABEL: define{{.*}} ptr @_ZN9can_throw11coro_createEv(
// CHECK: init.ready:
// CHECK-NEXT: store i1 true, ptr {{.*}}
-// CHECK-NEXT: call void @_ZN4Task23initial_suspend_awaiter12await_resumeEv(
-// CHECK-NEXT: call void @_ZN14NontrivialTypeD1Ev(
+// CHECK-NEXT: call void
@_ZN9can_throw4Task23initial_suspend_awaiter12await_resumeEv(
+// CHECK-NEXT: call void @_ZN9can_throw14NontrivialTypeD1Ev(
// CHECK-NEXT: store i1 false, ptr {{.*}}
+}
+
+namespace no_throw {
+struct NontrivialType {
+ ~NontrivialType() {}
+};
+
+struct Task {
ChuanqiXu9 wrote:
It looks a little bit confusing. Let's try to rename it to InitNoThrowTask.
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -129,7 +130,14 @@ static SmallString<32> buildSuspendPrefixStr(CGCoroData
, AwaitKind Kind) {
return Prefix;
}
-static bool memberCallExpressionCanThrow(const Expr *E) {
+static bool ResumeExprCanThrow(const CoroutineSuspendExpr ) {
+ const Expr *E = S.getResumeExpr();
+
+ // If the return type of await_resume is not void, get the CXXMemberCallExpr
+ // from its subexpr.
+ if (const auto *BindTempExpr = dyn_cast(E)) {
+E = BindTempExpr->getSubExpr();
+ }
ChuanqiXu9 wrote:
Such pattern match doesn't smell good. How about looking into its children
recursively if we find `E` is not CXXMemberCallExpr?
https://github.com/llvm/llvm-project/pull/73160
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] CGCoroutines skip emitting try block for value returning `noexcept` init `await_resume` calls (PR #73160)
@@ -12,9 +12,10 @@ #include "CGCleanup.h" #include "CodeGenFunction.h" -#include "llvm/ADT/ScopeExit.h" +#include "clang/AST/ExprCXX.h" #include "clang/AST/StmtCXX.h" #include "clang/AST/StmtVisitor.h" +#include "llvm/ADT/ScopeExit.h" ChuanqiXu9 wrote: Is this change necessary? https://github.com/llvm/llvm-project/pull/73160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Coroutines] Properly emit EH code for initial suspend `await_resume` (PR #73073)
https://github.com/ChuanqiXu9 approved this pull request. LGTM. Thanks. https://github.com/llvm/llvm-project/pull/73073 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Introduce a tool 'clang-named-modules-querier' and two plugins 'ClangGetUsedFilesFromModulesPlugin' and 'ClangGetDeclsInModulesPlugin' (PR #72956)
ChuanqiXu9 wrote: > I'm still really hesitant about this direction. > > One starting concern: what happens if someone adds an overload, or other > interesting name resolution to the module? The downstream caller hasn't > textually changed, but it should be rebuilt because it should be calling a > different overload candidate now? (& even if we then track every function > with the same name, there's other cases - like adding an implicit conversion > operator, operator overload, etc, that might complicate things) Oh, nice catch. It is a problem for the used file based solution if we add the overload to a separate unused files. While the hash based solution can handle the overloads case, it'll be a problem if we add an implicit conversion we didn't use before. https://github.com/llvm/llvm-project/pull/72956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
