[clang] [Serialization] No transitive identifier change (PR #92085)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/92085
>From e8f756ec7f8ea7e5bf18cc122a965fb2f258fd15 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Tue, 14 May 2024 15:33:12 +0800
Subject: [PATCH] [Serialization] No transitive identifier change
---
.../clang/Lex/ExternalPreprocessorSource.h| 54 -
clang/include/clang/Lex/HeaderSearch.h| 12 +-
.../include/clang/Serialization/ASTBitCodes.h | 2 +-
clang/include/clang/Serialization/ASTReader.h | 19 ++-
.../include/clang/Serialization/ModuleFile.h | 3 -
clang/lib/Lex/HeaderSearch.cpp| 33 +++---
clang/lib/Serialization/ASTReader.cpp | 98
clang/lib/Serialization/ASTWriter.cpp | 63 ++
clang/lib/Serialization/GlobalModuleIndex.cpp | 3 +-
clang/lib/Serialization/ModuleFile.cpp| 1 -
.../no-transitive-identifier-change.cppm | 110 ++
11 files changed, 286 insertions(+), 112 deletions(-)
create mode 100644 clang/test/Modules/no-transitive-identifier-change.cppm
diff --git a/clang/include/clang/Lex/ExternalPreprocessorSource.h
b/clang/include/clang/Lex/ExternalPreprocessorSource.h
index 6775841860373..48429948dbffe 100644
--- a/clang/include/clang/Lex/ExternalPreprocessorSource.h
+++ b/clang/include/clang/Lex/ExternalPreprocessorSource.h
@@ -36,12 +36,64 @@ class ExternalPreprocessorSource {
/// Return the identifier associated with the given ID number.
///
/// The ID 0 is associated with the NULL identifier.
- virtual IdentifierInfo *GetIdentifier(unsigned ID) = 0;
+ virtual IdentifierInfo *GetIdentifier(uint64_t ID) = 0;
/// Map a module ID to a module.
virtual Module *getModule(unsigned ModuleID) = 0;
};
+// Either a pointer to an IdentifierInfo of the controlling macro or the ID
+// number of the controlling macro.
+class LazyIdentifierInfoPtr {
+ // If the low bit is clear, a pointer to the IdentifierInfo. If the low
+ // bit is set, the upper 63 bits are the ID number.
+ mutable uint64_t Ptr = 0;
+
+public:
+ LazyIdentifierInfoPtr() = default;
+
+ explicit LazyIdentifierInfoPtr(const IdentifierInfo *Ptr)
+ : Ptr(reinterpret_cast(Ptr)) {}
+
+ explicit LazyIdentifierInfoPtr(uint64_t ID) : Ptr((ID << 1) | 0x01) {
+assert((ID << 1 >> 1) == ID && "ID must require < 63 bits");
+if (ID == 0)
+ Ptr = 0;
+ }
+
+ LazyIdentifierInfoPtr =(const IdentifierInfo *Ptr) {
+this->Ptr = reinterpret_cast(Ptr);
+return *this;
+ }
+
+ LazyIdentifierInfoPtr =(uint64_t ID) {
+assert((ID << 1 >> 1) == ID && "IDs must require < 63 bits");
+if (ID == 0)
+ Ptr = 0;
+else
+ Ptr = (ID << 1) | 0x01;
+
+return *this;
+ }
+
+ /// Whether this pointer is non-NULL.
+ ///
+ /// This operation does not require the AST node to be deserialized.
+ bool isValid() const { return Ptr != 0; }
+
+ /// Whether this pointer is currently stored as ID.
+ bool isID() const { return Ptr & 0x01; }
+
+ IdentifierInfo *getPtr() const {
+assert(!isID());
+return reinterpret_cast(Ptr);
+ }
+
+ uint64_t getID() const {
+assert(isID());
+return Ptr >> 1;
+ }
+};
}
#endif
diff --git a/clang/include/clang/Lex/HeaderSearch.h
b/clang/include/clang/Lex/HeaderSearch.h
index 5ac634d4e..65700b8f9dc11 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -16,6 +16,7 @@
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/DirectoryLookup.h"
+#include "clang/Lex/ExternalPreprocessorSource.h"
#include "clang/Lex/HeaderMap.h"
#include "clang/Lex/ModuleMap.h"
#include "llvm/ADT/ArrayRef.h"
@@ -119,13 +120,6 @@ struct HeaderFileInfo {
LLVM_PREFERRED_TYPE(bool)
unsigned IsValid : 1;
- /// The ID number of the controlling macro.
- ///
- /// This ID number will be non-zero when there is a controlling
- /// macro whose IdentifierInfo may not yet have been loaded from
- /// external storage.
- unsigned ControllingMacroID = 0;
-
/// If this file has a \#ifndef XXX (or equivalent) guard that
/// protects the entire contents of the file, this is the identifier
/// for the macro that controls whether or not it has any effect.
@@ -134,7 +128,7 @@ struct HeaderFileInfo {
/// the controlling macro of this header, since
/// getControllingMacro() is able to load a controlling macro from
/// external storage.
- const IdentifierInfo *ControllingMacro = nullptr;
+ LazyIdentifierInfoPtr LazyControllingMacro;
/// If this header came from a framework include, this is the name
/// of the framework.
@@ -580,7 +574,7 @@ class HeaderSearch {
/// no-op \#includes.
void SetFileControllingMacro(FileEntryRef File,
const IdentifierInfo *ControllingMacro) {
-getFileInfo(File).ControllingMacro = ControllingMacro;
+getFileInfo(File).LazyControllingMacro = ControllingMacro;
}
/// Determine
[clang] [Serialization] No transitive identifier change (PR #92085)
@@ -3896,7 +3903,7 @@ void ASTWriter::WriteIdentifierTable(Preprocessor , // Write out identifiers if either the ID is local or the identifier has // changed since it was loaded. - if (ID >= FirstIdentID || !Chain || !II->isFromAST() || + if (isLocalIdentifierID(ID) || !Chain || !II->isFromAST() || ChuanqiXu9 wrote: Nice catch! The `!II->isFromAST()` check looks redundant indeed. I've made a NFC patch to address this: https://github.com/llvm/llvm-project/commit/799ae77993fa5d7b0638f10b3895090f8748de92 https://github.com/llvm/llvm-project/pull/92085 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 799ae77 - [NFC] [Serialization] Avoid unnecessary check for if Identifier from AST
Author: Chuanqi Xu Date: 2024-06-04T15:28:24+08:00 New Revision: 799ae77993fa5d7b0638f10b3895090f8748de92 URL: https://github.com/llvm/llvm-project/commit/799ae77993fa5d7b0638f10b3895090f8748de92 DIFF: https://github.com/llvm/llvm-project/commit/799ae77993fa5d7b0638f10b3895090f8748de92.diff LOG: [NFC] [Serialization] Avoid unnecessary check for if Identifier from AST Inspired by the review process in https://github.com/llvm/llvm-project/pull/92085. The check `ID >= FirstIdentID` can cover the following check `!II->isFromAST()`. Added: Modified: clang/lib/Serialization/ASTWriter.cpp Removed: diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 4f1d2c532bc91..b8b613db712f4 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -3895,8 +3895,7 @@ void ASTWriter::WriteIdentifierTable(Preprocessor , // Write out identifiers if either the ID is local or the identifier has // changed since it was loaded. - if (ID >= FirstIdentID || !Chain || !II->isFromAST() || - II->hasChangedSinceDeserialization() || + if (ID >= FirstIdentID || II->hasChangedSinceDeserialization() || (Trait.needDecls() && II->hasFETokenInfoChangedSinceDeserialization())) Generator.insert(II, ID, Trait); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] d8ec452 - [serialization] no transitive decl change (#92083)
Author: Chuanqi Xu
Date: 2024-06-04T14:45:00+08:00
New Revision: d8ec452db016f359feeec28994f6560b30b49824
URL:
https://github.com/llvm/llvm-project/commit/d8ec452db016f359feeec28994f6560b30b49824
DIFF:
https://github.com/llvm/llvm-project/commit/d8ec452db016f359feeec28994f6560b30b49824.diff
LOG: [serialization] no transitive decl change (#92083)
Following of https://github.com/llvm/llvm-project/pull/86912
The motivation of the patch series is that, for a module interface unit
`X`, when the dependent modules of `X` changes, if the changes is not
relevant with `X`, we hope the BMI of `X` won't change. For the specific
patch, we hope if the changes was about irrelevant declaration changes,
we hope the BMI of `X` won't change. **However**, I found the patch
itself is not very useful in practice, since the adding or removing
declarations, will change the state of identifiers and types in most
cases.
That said, for the most simple example,
```
// partA.cppm
export module m:partA;
// partA.v1.cppm
export module m:partA;
export void a() {}
// partB.cppm
export module m:partB;
export void b() {}
// m.cppm
export module m;
export import :partA;
export import :partB;
// onlyUseB;
export module onlyUseB;
import m;
export inline void onluUseB() {
b();
}
```
the BMI of `onlyUseB` will change after we change the implementation of
`partA.cppm` to `partA.v1.cppm`. Since `partA.v1.cppm` introduces new
identifiers and types (the function prototype).
So in this patch, we have to write the tests as:
```
// partA.cppm
export module m:partA;
export int getA() { ... }
export int getA2(int) { ... }
// partA.v1.cppm
export module m:partA;
export int getA() { ... }
export int getA(int) { ... }
export int getA2(int) { ... }
// partB.cppm
export module m:partB;
export void b() {}
// m.cppm
export module m;
export import :partA;
export import :partB;
// onlyUseB;
export module onlyUseB;
import m;
export inline void onluUseB() {
b();
}
```
so that the new introduced declaration `int getA(int)` doesn't introduce
new identifiers and types, then the BMI of `onlyUseB` can keep
unchanged.
While it looks not so great, the patch should be the base of the patch
to erase the transitive change for identifiers and types since I don't
know how can we introduce new types and identifiers without introducing
new declarations. Given how tightly the relationship between
declarations, types and identifiers, I think we can only reach the ideal
state after we made the series for all of the three entties.
The design of the patch is similar to
https://github.com/llvm/llvm-project/pull/86912, which extends the
32-bit DeclID to 64-bit and use the higher bits to store the module file
index and the lower bits to store the Local Decl ID.
A slight difference is that we only use 48 bits to store the new DeclID
since we try to use the higher 16 bits to store the module ID in the
prefix of Decl class. Previously, we use 32 bits to store the module ID
and 32 bits to store the DeclID. I don't want to allocate additional
space so I tried to make the additional space the same as 64 bits. An
potential interesting thing here is about the relationship between the
module ID and the module file index. I feel we can get the module file
index by the module ID. But I didn't prove it or implement it. Since I
want to make the patch itself as small as possible. We can make it in
the future if we want.
Another change in the patch is the new concept Decl Index, which means
the index of the very big array `DeclsLoaded` in ASTReader. Previously,
the index of a loaded declaration is simply the Decl ID minus
PREDEFINED_DECL_NUMs. So there are some places they got used
ambiguously. But this patch tried to split these two concepts.
As https://github.com/llvm/llvm-project/pull/86912 did, the change will
increase the on-disk PCM file sizes. As the declaration ID may be the
most IDs in the PCM file, this can have the biggest impact on the size.
In my experiments, this change will bring 6.6% increase of the on-disk
PCM size. No compile-time performance regression observed. Given the
benefits in the motivation example, I think the cost is worthwhile.
Added:
clang/test/Modules/no-transitive-decls-change.cppm
Modified:
clang/include/clang/AST/DeclBase.h
clang/include/clang/AST/DeclID.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ModuleFile.h
clang/include/clang/Serialization/ModuleManager.h
clang/lib/AST/DeclBase.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ModuleFile.cpp
Removed:
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 3a311d4c55916..c4eb053146d32 100644
---
[clang] [serialization] no transitive decl change (PR #92083)
ChuanqiXu9 wrote: Thanks for reporting. I've reproduced them (including the lldb test) locally and fixed them. I'll try to land it again to see what happens. https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 6b30180 - Revert "[serialization] no transitive decl change (#92083)"
Author: Chuanqi Xu
Date: 2024-06-03T18:49:18+08:00
New Revision: 6b30180b663e1fe4de32046398581a374c8a54f2
URL:
https://github.com/llvm/llvm-project/commit/6b30180b663e1fe4de32046398581a374c8a54f2
DIFF:
https://github.com/llvm/llvm-project/commit/6b30180b663e1fe4de32046398581a374c8a54f2.diff
LOG: Revert "[serialization] no transitive decl change (#92083)"
This reverts commit ccb73e882b2d727877cfda42a14a6979cfd31f04.
It looks like there are some bots complaining about the patch.
See the post commit comment in
https://github.com/llvm/llvm-project/pull/92083 to track it.
Added:
Modified:
clang/include/clang/AST/DeclBase.h
clang/include/clang/AST/DeclID.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ModuleFile.h
clang/include/clang/Serialization/ModuleManager.h
clang/lib/AST/DeclBase.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ModuleFile.cpp
Removed:
clang/test/Modules/no-transitive-decls-change.cppm
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 4bdf27aa99405..e43e812cd9455 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -701,7 +701,10 @@ class alignas(8) Decl {
/// Set the owning module ID. This may only be called for
/// deserialized Decls.
- void setOwningModuleID(unsigned ID);
+ void setOwningModuleID(unsigned ID) {
+assert(isFromASTFile() && "Only works on a deserialized declaration");
+*((unsigned*)this - 2) = ID;
+ }
public:
/// Determine the availability of the given declaration.
@@ -774,11 +777,19 @@ class alignas(8) Decl {
/// Retrieve the global declaration ID associated with this
/// declaration, which specifies where this Decl was loaded from.
- GlobalDeclID getGlobalID() const;
+ GlobalDeclID getGlobalID() const {
+if (isFromASTFile())
+ return (*((const GlobalDeclID *)this - 1));
+return GlobalDeclID();
+ }
/// Retrieve the global ID of the module that owns this particular
/// declaration.
- unsigned getOwningModuleID() const;
+ unsigned getOwningModuleID() const {
+if (isFromASTFile())
+ return *((const unsigned*)this - 2);
+return 0;
+ }
private:
Module *getOwningModuleSlow() const;
diff --git a/clang/include/clang/AST/DeclID.h
b/clang/include/clang/AST/DeclID.h
index 32d2ed41a374a..614ba06b63860 100644
--- a/clang/include/clang/AST/DeclID.h
+++ b/clang/include/clang/AST/DeclID.h
@@ -19,8 +19,6 @@
#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/ADT/iterator.h"
-#include
-
namespace clang {
/// Predefined declaration IDs.
@@ -109,16 +107,12 @@ class DeclIDBase {
///
/// DeclID should only be used directly in serialization. All other users
/// should use LocalDeclID or GlobalDeclID.
- using DeclID = uint64_t;
+ using DeclID = uint32_t;
protected:
DeclIDBase() : ID(PREDEF_DECL_NULL_ID) {}
explicit DeclIDBase(DeclID ID) : ID(ID) {}
- explicit DeclIDBase(unsigned LocalID, unsigned ModuleFileIndex) {
-ID = (DeclID)LocalID | ((DeclID)ModuleFileIndex << 32);
- }
-
public:
DeclID get() const { return ID; }
@@ -130,10 +124,6 @@ class DeclIDBase {
bool isInvalid() const { return ID == PREDEF_DECL_NULL_ID; }
- unsigned getModuleFileIndex() const { return ID >> 32; }
-
- unsigned getLocalDeclIndex() const;
-
friend bool operator==(const DeclIDBase , const DeclIDBase ) {
return LHS.ID == RHS.ID;
}
@@ -166,9 +156,6 @@ class LocalDeclID : public DeclIDBase {
LocalDeclID(PredefinedDeclIDs ID) : Base(ID) {}
explicit LocalDeclID(DeclID ID) : Base(ID) {}
- explicit LocalDeclID(unsigned LocalID, unsigned ModuleFileIndex)
- : Base(LocalID, ModuleFileIndex) {}
-
LocalDeclID ++() {
++ID;
return *this;
@@ -188,9 +175,6 @@ class GlobalDeclID : public DeclIDBase {
GlobalDeclID() : Base() {}
explicit GlobalDeclID(DeclID ID) : Base(ID) {}
- explicit GlobalDeclID(unsigned LocalID, unsigned ModuleFileIndex)
- : Base(LocalID, ModuleFileIndex) {}
-
// For DeclIDIterator to be able to convert a GlobalDeclID
// to a LocalDeclID.
explicit operator LocalDeclID() const { return LocalDeclID(this->ID); }
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h
b/clang/include/clang/Serialization/ASTBitCodes.h
index 9e4b21baa7d20..fe1bd47348be1 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -255,12 +255,6 @@ class DeclOffset {
}
};
-// The unaligned decl ID used in the Blobs of bistreams.
-using unaligned_decl_id_t =
-llvm::support::detail::packed_endian_specific_integral<
-serialization::DeclID, llvm::endianness::native,
-
[clang] [serialization] no transitive decl change (PR #92083)
ChuanqiXu9 wrote: I'll close it as there are some crashes. --- The crash log: ``` ** TEST 'Clang :: Modules/redecl-ivars.m' FAILED Exit Code: 1 Command Output (stderr): -- RUN: at line 2: rm -rf /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp + rm -rf /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp RUN: at line 3: split-file /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/test/Modules/redecl-ivars.m /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp + split-file /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang/test/Modules/redecl-ivars.m /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp RUN: at line 4: /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/lib/clang/19/include -nostdsysteminc -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/include /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/test-mismatch-in-extension.m + /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/lib/clang/19/include -nostdsysteminc -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/include /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/test-mismatch-in-extension.m RUN: at line 5: /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/lib/clang/19/include -nostdsysteminc -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/include /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/test-mismatch-in-extension.m -fmodules -fimplicit-module-maps -fmodules-cache-path=/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/modules.cache + /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/lib/clang/19/include -nostdsysteminc -fsyntax-only -fobjc-runtime=macosx-10.9 -verify -I/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/include /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/test-mismatch-in-extension.m -fmodules -fimplicit-module-maps -fmodules-cache-path=/b/sanitizer-x86_64-linux-fast/build/llvm_build_asan_ubsan/tools/clang/test/Modules/Output/redecl-ivars.m.tmp/modules.cache /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan_ubsan/include/c++/v1/__algorithm/lower_bound.h:39:53: runtime error: reference binding to misaligned address 0x52945c44 for type 'const clang::serialization::ObjCCategoriesInfo', which requires 8 byte alignment 0x52945c44: note: pointer points here 00 00 00 00 02 00 00 00 01 00 00 00 02 00 00 00 00 00 00 00 c3 0d 88 60 0a a8 81 10 03 20 08 00 ^ SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan_ubsan/include/c++/v1/__algorithm/lower_bound.h:39:53 -- It looks like misaligned access too. I'll try to fix it. (It is somewhat hurting that we can't find them on the trunk.) ``` https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] No transitive identifier change (PR #92085)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/92085 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] a41a20b - [NFC] [C++20] [Modules] [Reduced BMI] Reorder Emitting reduced BMI and normal BMI for named modules
Author: Chuanqi Xu
Date: 2024-06-03T15:47:34+08:00
New Revision: a41a20bd47968b16bb84761578628752080e9f24
URL:
https://github.com/llvm/llvm-project/commit/a41a20bd47968b16bb84761578628752080e9f24
DIFF:
https://github.com/llvm/llvm-project/commit/a41a20bd47968b16bb84761578628752080e9f24.diff
LOG: [NFC] [C++20] [Modules] [Reduced BMI] Reorder Emitting reduced BMI and
normal BMI for named modules
When we generate the reduced BMI on the fly, the order of the emitting
phase is different within `-emit-obj` and `-emit-module-interface`.
Although this is meant to be fine, we observed it in
https://github.com/llvm/llvm-project/issues/93859 (that the different phase
order may cause problems).
Also it turns out to be a different fundamental reason to the orders.
But it might be fine to make the order of emitting reducing BMI at first
to avoid such confusions in the future.
Added:
Modified:
clang/lib/Frontend/FrontendActions.cpp
Removed:
diff --git a/clang/lib/Frontend/FrontendActions.cpp
b/clang/lib/Frontend/FrontendActions.cpp
index 454653a31534c..4f064321997a2 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -273,9 +273,6 @@ std::unique_ptr
GenerateModuleInterfaceAction::CreateASTConsumer(CompilerInstance ,
StringRef InFile) {
std::vector> Consumers;
- Consumers.push_back(std::make_unique(
- CI.getPreprocessor(), CI.getModuleCache(),
- CI.getFrontendOpts().OutputFile));
if (CI.getFrontendOpts().GenReducedBMI &&
!CI.getFrontendOpts().ModuleOutputPath.empty()) {
@@ -284,6 +281,10 @@
GenerateModuleInterfaceAction::CreateASTConsumer(CompilerInstance ,
CI.getFrontendOpts().ModuleOutputPath));
}
+ Consumers.push_back(std::make_unique(
+ CI.getPreprocessor(), CI.getModuleCache(),
+ CI.getFrontendOpts().OutputFile));
+
return std::make_unique(std::move(Consumers));
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
ChuanqiXu9 wrote: I'd like to land this since: - I want to give more time testing this - the design of the patch is pretty similar with the previous one (https://github.com/llvm/llvm-project/pull/86912) - the scale of the patch is not very big (100+ lines of code except new tests) - I do think the functionality is very very important to modules. https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] a68638b - [C++20] [Modules] [Reduced BMI] Handling Deduction Guide in reduced BMI
Author: Chuanqi Xu
Date: 2024-06-03T14:59:23+08:00
New Revision: a68638bf6a6a5cb60947753ccaf7d1de80f6c89e
URL:
https://github.com/llvm/llvm-project/commit/a68638bf6a6a5cb60947753ccaf7d1de80f6c89e
DIFF:
https://github.com/llvm/llvm-project/commit/a68638bf6a6a5cb60947753ccaf7d1de80f6c89e.diff
LOG: [C++20] [Modules] [Reduced BMI] Handling Deduction Guide in reduced BMI
carefully
Close https://github.com/llvm/llvm-project/issues/93859
The direct pattern of the issue is that, in a reduced BMI, we're going
to wrtie a class but we didn't write the deduction guide. Although we
handled deduction guide, but we tried to record the found deduction
guide from `noload_lookup` directly.
It is slightly problematic if the found deduction guide is from AST.
e.g.,
```
module;
export module m;
import xxx; // Also contains the class and the deduction guide
...
```
Then when we writes the class in the current file, we tried to record
the deduction guide, but `noload_lookup` returns the deduction guide
from the AST file then we didn't record the local deduction guide. Then
mismatch happens.
To mitiagte the problem, we tried to record the canonical declaration
for the decution guide.
Added:
clang/test/Modules/pr93859.cppm
Modified:
clang/lib/Serialization/ASTWriterDecl.cpp
Removed:
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp
b/clang/lib/Serialization/ASTWriterDecl.cpp
index bbd16dbdb8fff..5a6ab4408eb2b 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1733,7 +1733,7 @@ void
ASTDeclWriter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
if (Writer.isGeneratingReducedBMI()) {
auto Name = Context.DeclarationNames.getCXXDeductionGuideName(D);
for (auto *DG : D->getDeclContext()->noload_lookup(Name))
- Writer.GetDeclRef(DG);
+ Writer.GetDeclRef(DG->getCanonicalDecl());
}
Code = serialization::DECL_CLASS_TEMPLATE;
diff --git a/clang/test/Modules/pr93859.cppm b/clang/test/Modules/pr93859.cppm
new file mode 100644
index 0..d1d45bb975308
--- /dev/null
+++ b/clang/test/Modules/pr93859.cppm
@@ -0,0 +1,146 @@
+// Reduced from https://github.com/llvm/llvm-project/issues/93859
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/reduced_std.cppm
-emit-reduced-module-interface -o %t/reduced_std.pcm
+// RUN: %clang_cc1 -std=c++20 %t/Misc.cppm -emit-reduced-module-interface -o
%t/Misc.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/Instance.cppm -emit-reduced-module-interface
-o %t/Instance.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/Device.cppm -emit-reduced-module-interface -o
%t/Device.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/Overlay.cppm -emit-reduced-module-interface
-o %t/Overlay.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/App.cppm -emit-module-interface -o /dev/null \
+// RUN: -fexperimental-modules-reduced-bmi -fmodule-output=%t/App.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/test.cc -fsyntax-only -verify \
+// RUN: -fprebuilt-module-path=%t
+
+//--- header.h
+namespace std {
+
+template
+struct pair
+{
+ _T1 first;
+ _T2 second;
+
+ constexpr pair()
+ : first(), second() {}
+
+ constexpr pair(_T1 const& __t1, _T2 const& __t2)
+ : first(__t1), second(__t2) {}
+};
+
+template
+pair(_T1, _T2) -> pair<_T1, _T2>;
+
+template
+class __tree_const_iterator {
+public:
+ template
+ friend class __tree;
+};
+
+template
+class __tree {
+public:
+ typedef _Tp value_type;
+ typedef __tree_const_iterator const_iterator;
+
+ template
+ friend class map;
+};
+
+template
+class set {
+public:
+ typedef __tree<_Key> __base;
+
+ typedef typename __base::const_iterator iterator;
+
+ set() {}
+
+ pair
+ insert(const _Key& __v);
+};
+
+template
+inline constexpr _OutputIterator
+copy(_InputIterator __first, _InputIterator __last, _OutputIterator __result) {
+ return pair{__first, __last}.second;
+}
+
+}
+
+//--- reduced_std.cppm
+module;
+#include "header.h"
+export module reduced_std;
+
+export namespace std {
+using std::set;
+using std::copy;
+}
+
+//--- Misc.cppm
+export module Misc;
+import reduced_std;
+
+export void check_result(int res) {
+std::set extensions;
+extensions.insert('f');
+}
+
+//--- Instance.cppm
+export module Instance;
+import reduced_std;
+
+export class Instance {
+public:
+Instance() {
+std::set extensions;
+extensions.insert("foo");
+}
+};
+
+//--- Device.cppm
+export module Device;
+import reduced_std;
+import Instance;
+import Misc;
+
+std::set wtf_set;
+
+//--- Overlay.cppm
+export module Overlay;
+
+import reduced_std;
+import Device;
+
+void overlay_vector_use() {
+std::set nums;
+
[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
@@ -1180,6 +1185,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())
ChuanqiXu9 wrote:
Yeah, I agree it might be good conceptly. But given the clang header modules
with object files are not widely used and the current patch may be somewhat
impactful, I feel better to do that in the future when we really need to.
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] [clang-tools-extra] [libcxx] [clang][Modules] Remove unnecessary includes of `Module.h` (PR #93417)
https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/93417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [libcxx] [clang][Modules] Remove unnecessary includes of `Module.h` (PR #93417)
ChuanqiXu9 wrote: > Is there any additional work needed on this before it can be merged? No, it looks good. I'll merge this. https://github.com/llvm/llvm-project/pull/93417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [libcxx] [clang][Modules] Remove unnecessary includes of `Module.h` (PR #93417)
https://github.com/ChuanqiXu9 approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/93417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Be const-correct with all uses of `Module *`. (PR #93493)
https://github.com/ChuanqiXu9 approved this pull request. I feel good with this too. But if later you have similar multiple consecutive large NFC patches like this, I'll recommend you send the PR as stacked PR and then we can land the continuously. It'll help the downstream project slightly. https://github.com/llvm/llvm-project/pull/93493 ___ 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)
@@ -1802,6 +1802,12 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables , if (VTable->hasInitializer()) return; + // If the class is 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()) ChuanqiXu9 wrote: Yeah, understood. I just wanted to avoid checkout the old branch so that I can save the building time locally. And this point looks minor. I feel good to make it in following patches or with other required changes together in this patch. 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)
@@ -1180,6 +1185,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())
ChuanqiXu9 wrote:
> hasExternalDefinitions isn't "is this AST from some other AST file" but "did
> the AST file say it'd handle the object-file definition of this thing" - the
> pcm file specifies which things it'll handle.
> So if the intent is not to home anything in the GMF, then pcms could be
> written out that don't set "hasExternalDefinitions" to true for those
> entities.
No, it is not the intent. The intent is to keep GMF as legacy code instead of
named modules.
For example,
```
// header.h
void func() { } // ! Non inline
// x.cppm
module;
#include "header.h"
export module x;
...
```
Then in the object file of `x.cppm`, (if `func()` is not discarded out), we
should be able to see the definition of `func()`. Although it is rare since the
definitions in the headers are generally inline, but we as the compiler can't
assume that.
I think the current implementation is more straight forward and clear.
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] [clang] fix merging of UsingShadowDecl (PR #80245)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/80245 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] fix merging of UsingShadowDecl (PR #80245)
https://github.com/ChuanqiXu9 approved this pull request. LG. It looks not wise to block this good PR. After all, it is a change 6 lines. Let's go ahead. https://github.com/llvm/llvm-project/pull/80245 ___ 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)
@@ -1802,6 +1802,12 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables , if (VTable->hasInitializer()) return; + // If the class is 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()) ChuanqiXu9 wrote: If we feel this is too long, I can wrap them into a member function of Decl in a following patch. 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)
@@ -1180,6 +1185,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())
ChuanqiXu9 wrote:
Maybe it is not strictly correct to use `hasExternalDefinitions()` since it
can't handle the global module case. I mean, from the current wording, the
class in the global module should follow the old ABI rules (key functions). But
`hasExternalDefinitions()` won't make that clear.
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] [Serialization] Check for stack exhaustion when reading declarations (PR #79875)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/79875 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Check for stack exhaustion when reading declarations (PR #79875)
https://github.com/ChuanqiXu9 approved this pull request. If we don't insist on testing this in this commit, them LGTM. https://github.com/llvm/llvm-project/pull/79875 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Check for stack exhaustion when reading declarations (PR #79875)
@@ -9403,6 +9404,20 @@ DiagnosticBuilder ASTReader::Diag(SourceLocation Loc,
unsigned DiagID) const {
return Diags.Report(Loc, DiagID);
}
+void ASTReader::warnStackExhausted(SourceLocation Loc) {
+ // When Sema is available, avoid duplicate errors. This should be rare.
ChuanqiXu9 wrote:
```suggestion
// When Sema is available, avoid duplicate errors.
```
https://github.com/llvm/llvm-project/pull/79875
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't generate the defintition for non-const available external variables (PR #93530)
https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/93530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't generate the defintition for non-const available external variables (PR #93530)
@@ -5341,6 +5341,15 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D, !IsDefinitionAvailableExternally && D->needsDestruction(getContext()) == QualType::DK_cxx_destructor; + // It is helpless to emit the definition for an available_externally variable + // which can't be marked as const. + // We don't need to check if it needs global ctor or dtor. See the above + // comment for ideas. + if (IsDefinitionAvailableExternally && + (!D->hasConstantInitialization() || + !D->getType().isConstantStorage(getContext(), true, true))) ChuanqiXu9 wrote: Got it. Makes sense. Done. https://github.com/llvm/llvm-project/pull/93530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't generate the defintition for non-const available external variables (PR #93530)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/93530
>From ebe47b2b411d7623ddafadad45a9be25913fe1c1 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 29 May 2024 10:48:51 +0800
Subject: [PATCH] [C++20] [Modules] Don't generate the defintition for
non-const available external variables
Close #93497
The root cause of the problem is, we mark the variable from other
modules as constnant in LLVM incorrectly. This patch fixes this problem
by not emitting the defintition for non-const available external
variables. Since the non const available externally variable is not
helpful to the optimization.
---
clang/lib/CodeGen/CodeGenModule.cpp | 12 +++
clang/test/CodeGenCXX/partitions.cpp | 8 +-
clang/test/Modules/pr93497.cppm | 106 +++
3 files changed, 122 insertions(+), 4 deletions(-)
create mode 100644 clang/test/Modules/pr93497.cppm
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp
b/clang/lib/CodeGen/CodeGenModule.cpp
index e4774a587707a..0b0b659e1fd49 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5341,6 +5341,18 @@ void CodeGenModule::EmitGlobalVarDefinition(const
VarDecl *D,
!IsDefinitionAvailableExternally &&
D->needsDestruction(getContext()) == QualType::DK_cxx_destructor;
+ // It is helpless to emit the definition for an available_externally variable
+ // which can't be marked as const.
+ // We don't need to check if it needs global ctor or dtor. See the above
+ // comment for ideas.
+ if (IsDefinitionAvailableExternally &&
+ (!D->hasConstantInitialization() ||
+ // TODO: Update this when we have interface to check constexpr
+ // destructor.
+ D->needsDestruction(getContext()) ||
+ !D->getType().isConstantStorage(getContext(), true, true)))
+return;
+
const VarDecl *InitDecl;
const Expr *InitExpr = D->getAnyInitializer(InitDecl);
diff --git a/clang/test/CodeGenCXX/partitions.cpp
b/clang/test/CodeGenCXX/partitions.cpp
index d283dd071f6b2..e80e68f82974b 100644
--- a/clang/test/CodeGenCXX/partitions.cpp
+++ b/clang/test/CodeGenCXX/partitions.cpp
@@ -40,12 +40,12 @@ export int use() {
}
// FIXME: The definition of the variables shouldn't be exported too.
-// CHECK: @_ZW3mod1a = available_externally global
-// CHECK: @_ZW3mod1b = available_externally global
+// CHECK: @_ZW3mod1a = external global
+// CHECK: @_ZW3mod1b = external global
// CHECK: declare{{.*}} i32 @_ZW3mod3foov
// CHECK: declare{{.*}} i32 @_ZW3mod3barv
-// CHECK-OPT: @_ZW3mod1a = available_externally global
-// CHECK-OPT: @_ZW3mod1b = available_externally global
+// CHECK-OPT: @_ZW3mod1a = external global
+// CHECK-OPT: @_ZW3mod1b = external global
// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3foov
// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3barv
diff --git a/clang/test/Modules/pr93497.cppm b/clang/test/Modules/pr93497.cppm
new file mode 100644
index 0..64a08e2a85e63
--- /dev/null
+++ b/clang/test/Modules/pr93497.cppm
@@ -0,0 +1,106 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/mod.cppm \
+// RUN: -emit-module-interface -o %t/mod.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/use.cpp \
+// RUN: -fmodule-file=mod=%t/mod.pcm -emit-llvm \
+// RUN: -o - | opt -S --passes=simplifycfg | FileCheck %t/use.cpp
+
+//--- mod.cppm
+export module mod;
+
+export struct Thing {
+static const Thing One;
+explicit Thing(int raw) :raw(raw) { }
+int raw;
+};
+
+const Thing Thing::One = Thing(1);
+
+export struct C {
+int value;
+};
+export const C ConstantValue = {1};
+
+export const C *ConstantPtr =
+
+C NonConstantValue = {1};
+export const C = NonConstantValue;
+
+export struct NonConstexprDtor {
+constexpr NonConstexprDtor(int raw) : raw(raw) {}
+~NonConstexprDtor();
+
+int raw;
+};
+
+export const NonConstexprDtor NonConstexprDtorValue = {1};
+
+//--- use.cpp
+import mod;
+
+int consume(int);
+int consumeC(C);
+
+extern "C" __attribute__((noinline)) inline int unneeded() {
+return consume(43);
+}
+
+extern "C" __attribute__((noinline)) inline int needed() {
+return consume(43);
+}
+
+int use() {
+Thing t1 = Thing::One;
+return consume(t1.raw);
+}
+
+int use2() {
+if (ConstantValue.value)
+return consumeC(ConstantValue);
+return unneeded();
+}
+
+int use3() {
+auto Ptr = ConstantPtr;
+if (Ptr->value)
+return consumeC(*Ptr);
+return needed();
+}
+
+int use4() {
+auto Ref = ConstantRef;
+if (Ref.value)
+return consumeC(Ref);
+return needed();
+}
+
+int use5() {
+NonConstexprDtor V = NonConstexprDtorValue;
+if (V.raw)
+return consume(V.raw);
+return needed();
+}
+
+// CHECK: @_ZNW3mod5Thing3OneE = external
+// CHECK: @_ZW3mod13ConstantValue ={{.*}}available_externally{{.*}} constant
+// CHECK: @_ZW3mod11ConstantPtr =
[clang] [C++20] [Modules] Don't generate the defintition for non-const available external variables (PR #93530)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/93530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't generate the defintition for non-const available external variables (PR #93530)
@@ -5341,6 +5341,15 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D, !IsDefinitionAvailableExternally && D->needsDestruction(getContext()) == QualType::DK_cxx_destructor; + // It is helpless to emit the definition for an available_externally variable + // which can't be marked as const. + // We don't need to check if it needs global ctor or dtor. See the above + // comment for ideas. + if (IsDefinitionAvailableExternally && + (!D->hasConstantInitialization() || + !D->getType().isConstantStorage(getContext(), true, true))) ChuanqiXu9 wrote: (we think) the available externally variables don't need ctor or dtor in the current TU. We mentioned this in the comment. https://github.com/llvm/llvm-project/pull/93530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't generate the defintition for non-const available external variables (PR #93530)
ChuanqiXu9 wrote: > I think if a variable is GVA_AvailableExternally, and we can't emit a > constant, we should just completely skip emitting the definition: there isn't > any point to emitting an available_externally definition that doesn't > actually contain any information the optimizer can use. > > Not sure off the top of my head where that check belongs; might be okay to > just stick it into EmitGlobalVarDefinition itself. Neat suggestion! I've applied it and the generated code looks better. https://github.com/llvm/llvm-project/pull/93530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't generate the defintition for non-const available external variables (PR #93530)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/93530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't generate the defintition for non-const available external variables (PR #93530)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/93530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't mark variables from other modules as constant if its initializer is not constant (PR #93530)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/93530
>From 05542b6176a84888438dd8c6cc9a86cb6f1b7282 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 29 May 2024 10:48:51 +0800
Subject: [PATCH] [C++20] [Modules] Don't generate the defintition for
non-const available external variables
Close #93497
The root cause of the problem is, we mark the variable from other
modules as constnant in LLVM incorrectly. This patch fixes this problem
by not emitting the defintition for non-const available external
variables. Since the non const available externally variable is not
helpful to the optimization.
---
clang/lib/CodeGen/CodeGenModule.cpp | 9 +++
clang/test/CodeGenCXX/partitions.cpp | 8 +--
clang/test/Modules/pr93497.cppm | 83
3 files changed, 96 insertions(+), 4 deletions(-)
create mode 100644 clang/test/Modules/pr93497.cppm
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp
b/clang/lib/CodeGen/CodeGenModule.cpp
index e4774a587707a..1661a195d5a38 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5341,6 +5341,15 @@ void CodeGenModule::EmitGlobalVarDefinition(const
VarDecl *D,
!IsDefinitionAvailableExternally &&
D->needsDestruction(getContext()) == QualType::DK_cxx_destructor;
+ // It is helpless to emit the definition for an available_externally variable
+ // which can't be marked as const.
+ // We don't need to check if it needs global ctor or dtor. See the above
+ // comment for ideas.
+ if (IsDefinitionAvailableExternally &&
+ (!D->hasConstantInitialization() ||
+ !D->getType().isConstantStorage(getContext(), true, true)))
+return;
+
const VarDecl *InitDecl;
const Expr *InitExpr = D->getAnyInitializer(InitDecl);
diff --git a/clang/test/CodeGenCXX/partitions.cpp
b/clang/test/CodeGenCXX/partitions.cpp
index d283dd071f6b2..e80e68f82974b 100644
--- a/clang/test/CodeGenCXX/partitions.cpp
+++ b/clang/test/CodeGenCXX/partitions.cpp
@@ -40,12 +40,12 @@ export int use() {
}
// FIXME: The definition of the variables shouldn't be exported too.
-// CHECK: @_ZW3mod1a = available_externally global
-// CHECK: @_ZW3mod1b = available_externally global
+// CHECK: @_ZW3mod1a = external global
+// CHECK: @_ZW3mod1b = external global
// CHECK: declare{{.*}} i32 @_ZW3mod3foov
// CHECK: declare{{.*}} i32 @_ZW3mod3barv
-// CHECK-OPT: @_ZW3mod1a = available_externally global
-// CHECK-OPT: @_ZW3mod1b = available_externally global
+// CHECK-OPT: @_ZW3mod1a = external global
+// CHECK-OPT: @_ZW3mod1b = external global
// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3foov
// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3barv
diff --git a/clang/test/Modules/pr93497.cppm b/clang/test/Modules/pr93497.cppm
new file mode 100644
index 0..fa88eadabdcd7
--- /dev/null
+++ b/clang/test/Modules/pr93497.cppm
@@ -0,0 +1,83 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/mod.cppm \
+// RUN: -emit-module-interface -o %t/mod.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/use.cpp \
+// RUN: -fmodule-file=mod=%t/mod.pcm -emit-llvm \
+// RUN: -o - | FileCheck %t/use.cpp
+
+//--- mod.cppm
+export module mod;
+
+export struct Thing {
+static const Thing One;
+explicit Thing(int raw) :raw(raw) { }
+int raw;
+};
+
+const Thing Thing::One = Thing(1);
+
+export struct C {
+int value;
+};
+export const C ConstantValue = {1};
+
+export const C *ConstantPtr =
+
+C NonConstantValue = {1};
+export const C = NonConstantValue;
+
+//--- use.cpp
+import mod;
+
+int consume(int);
+int consumeC(C);
+
+extern "C" __attribute__((noinline)) inline int unneeded() {
+return consume(43);
+}
+
+extern "C" __attribute__((noinline)) inline int needed() {
+return consume(43);
+}
+
+int use() {
+Thing t1 = Thing::One;
+return consume(t1.raw);
+}
+
+int use2() {
+if (ConstantValue.value)
+return consumeC(ConstantValue);
+return unneeded();
+}
+
+int use3() {
+if (ConstantPtr->value)
+return consumeC(*ConstantPtr);
+return needed();
+}
+
+int use4() {
+if (ConstantRef.value)
+return consumeC(ConstantRef);
+return needed();
+}
+
+// CHECK: @_ZNW3mod5Thing3OneE = external
+// CHECK: @_ZW3mod13ConstantValue ={{.*}}available_externally{{.*}} constant
+// CHECK: @_ZW3mod11ConstantPtr = external
+// CHECK: @_ZW3mod16NonConstantValue = external
+
+// Check that the middle end can optimize the program by the constant
information.
+// CHECK-NOT: @unneeded(
+
+// Check that the use of ConstantPtr won't get optimized incorrectly.
+// CHECK-LABEL: @_Z4use3v(
+// CHECK: @needed(
+
+// Check that the use of ConstantRef won't get optimized incorrectly.
+// CHECK-LABEL: @_Z4use4v(
+// CHECK: @needed(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
[clang] [C++20] [Modules] Don't mark variables from other modules as constant if its initializer is not constant (PR #93530)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/93530
>From eaf720ca9d3a6a576eefbf9ac553b108611ec00d Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 29 May 2024 10:48:51 +0800
Subject: [PATCH] [C++20] [Modules] Don't generate the defintition for
non-const available external variables
Close #93497
The root cause of the problem is, we mark the variable from other
modules as constnant in LLVM incorrectly. This patch fixes this problem
by not emitting the defintition for non-const available external
variables. Since the non const available externally variable is not
helpful to the optimization.
---
clang/lib/CodeGen/CodeGenModule.cpp | 9 +++
clang/test/CodeGenCXX/partitions.cpp | 8 +--
clang/test/Modules/pr93497.cppm | 83
3 files changed, 96 insertions(+), 4 deletions(-)
create mode 100644 clang/test/Modules/pr93497.cppm
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp
b/clang/lib/CodeGen/CodeGenModule.cpp
index e4774a587707a..77ce49ea46120 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5341,6 +5341,15 @@ void CodeGenModule::EmitGlobalVarDefinition(const
VarDecl *D,
!IsDefinitionAvailableExternally &&
D->needsDestruction(getContext()) == QualType::DK_cxx_destructor;
+ // It is helpless to emit the definition for an available_externally variable
+ // which can't be marked as const.
+ // We don't need to check if it needs global ctor or dtor. See the above
comment
+ // for ideas.
+ if (IsDefinitionAvailableExternally &&
+ (!D->hasConstantInitialization() ||
+ !D->getType().isConstantStorage(getContext(), true, true)))
+return;
+
const VarDecl *InitDecl;
const Expr *InitExpr = D->getAnyInitializer(InitDecl);
diff --git a/clang/test/CodeGenCXX/partitions.cpp
b/clang/test/CodeGenCXX/partitions.cpp
index d283dd071f6b2..e80e68f82974b 100644
--- a/clang/test/CodeGenCXX/partitions.cpp
+++ b/clang/test/CodeGenCXX/partitions.cpp
@@ -40,12 +40,12 @@ export int use() {
}
// FIXME: The definition of the variables shouldn't be exported too.
-// CHECK: @_ZW3mod1a = available_externally global
-// CHECK: @_ZW3mod1b = available_externally global
+// CHECK: @_ZW3mod1a = external global
+// CHECK: @_ZW3mod1b = external global
// CHECK: declare{{.*}} i32 @_ZW3mod3foov
// CHECK: declare{{.*}} i32 @_ZW3mod3barv
-// CHECK-OPT: @_ZW3mod1a = available_externally global
-// CHECK-OPT: @_ZW3mod1b = available_externally global
+// CHECK-OPT: @_ZW3mod1a = external global
+// CHECK-OPT: @_ZW3mod1b = external global
// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3foov
// CHECK-OPT: declare{{.*}} i32 @_ZW3mod3barv
diff --git a/clang/test/Modules/pr93497.cppm b/clang/test/Modules/pr93497.cppm
new file mode 100644
index 0..fa88eadabdcd7
--- /dev/null
+++ b/clang/test/Modules/pr93497.cppm
@@ -0,0 +1,83 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/mod.cppm \
+// RUN: -emit-module-interface -o %t/mod.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/use.cpp \
+// RUN: -fmodule-file=mod=%t/mod.pcm -emit-llvm \
+// RUN: -o - | FileCheck %t/use.cpp
+
+//--- mod.cppm
+export module mod;
+
+export struct Thing {
+static const Thing One;
+explicit Thing(int raw) :raw(raw) { }
+int raw;
+};
+
+const Thing Thing::One = Thing(1);
+
+export struct C {
+int value;
+};
+export const C ConstantValue = {1};
+
+export const C *ConstantPtr =
+
+C NonConstantValue = {1};
+export const C = NonConstantValue;
+
+//--- use.cpp
+import mod;
+
+int consume(int);
+int consumeC(C);
+
+extern "C" __attribute__((noinline)) inline int unneeded() {
+return consume(43);
+}
+
+extern "C" __attribute__((noinline)) inline int needed() {
+return consume(43);
+}
+
+int use() {
+Thing t1 = Thing::One;
+return consume(t1.raw);
+}
+
+int use2() {
+if (ConstantValue.value)
+return consumeC(ConstantValue);
+return unneeded();
+}
+
+int use3() {
+if (ConstantPtr->value)
+return consumeC(*ConstantPtr);
+return needed();
+}
+
+int use4() {
+if (ConstantRef.value)
+return consumeC(ConstantRef);
+return needed();
+}
+
+// CHECK: @_ZNW3mod5Thing3OneE = external
+// CHECK: @_ZW3mod13ConstantValue ={{.*}}available_externally{{.*}} constant
+// CHECK: @_ZW3mod11ConstantPtr = external
+// CHECK: @_ZW3mod16NonConstantValue = external
+
+// Check that the middle end can optimize the program by the constant
information.
+// CHECK-NOT: @unneeded(
+
+// Check that the use of ConstantPtr won't get optimized incorrectly.
+// CHECK-LABEL: @_Z4use3v(
+// CHECK: @needed(
+
+// Check that the use of ConstantRef won't get optimized incorrectly.
+// CHECK-LABEL: @_Z4use4v(
+// CHECK: @needed(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
[clang] [clang] Be const-correct with all uses of `Module *`. (PR #93493)
ChuanqiXu9 wrote: BTW, for the series patches, it may be better to use stacked PR (we use stacked PR by the method in https://llvm.org/docs/GitHub.html) if you have following patches (and it looks like you have a lot). Also, as a downstream vendor, I hope we can land the (large, NFC) patch series together. Since it shows some pains in backporting such patch series. https://github.com/llvm/llvm-project/pull/93493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't mark variables from other modules as constant if its initializer is not constant (PR #93530)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/93530
Close https://github.com/llvm/llvm-project/issues/93497
The root cause of the problem is, we mark the variable from other modules as
constnant in LLVM incorrectly. This patch fixes this problem and only mark the
variable from other modules as constant if its initializer is const.
>From 778205df51376ddd38253b2ce5bab9dcab512774 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Tue, 28 May 2024 18:44:18 +0800
Subject: [PATCH] [C++20] [Modules] Don't mark variables from other modules as
constant if its initializer is not constant
Close https://github.com/llvm/llvm-project/issues/93497
The root cause of the problem is, we mark the variable from other
modules as constnant in LLVM incorrectly. This patch fixes this problem
and only mark the variable from other modules as constant if its
initializer is const.
---
clang/lib/CodeGen/CodeGenModule.cpp | 2 +
clang/test/Modules/pr93497.cppm | 81 +
2 files changed, 83 insertions(+)
create mode 100644 clang/test/Modules/pr93497.cppm
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp
b/clang/lib/CodeGen/CodeGenModule.cpp
index e4774a587707a..5596a6708e4a1 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5492,6 +5492,8 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl
*D,
// If it is safe to mark the global 'constant', do so now.
GV->setConstant(!NeedsGlobalCtor && !NeedsGlobalDtor &&
+ (!IsDefinitionAvailableExternally ||
+ D->hasConstantInitialization()) &&
D->getType().isConstantStorage(getContext(), true, true));
// If it is in a read-only section, mark it 'constant'.
diff --git a/clang/test/Modules/pr93497.cppm b/clang/test/Modules/pr93497.cppm
new file mode 100644
index 0..dc0d8eb462e27
--- /dev/null
+++ b/clang/test/Modules/pr93497.cppm
@@ -0,0 +1,81 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/mod.cppm \
+// RUN: -emit-module-interface -o %t/mod.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/use.cpp \
+// RUN: -fmodule-file=mod=%t/mod.pcm -emit-llvm \
+// RUN: -o - | FileCheck %t/use.cpp
+
+//--- mod.cppm
+export module mod;
+
+export struct Thing {
+static const Thing One;
+explicit Thing(int raw) :raw(raw) { }
+int raw;
+};
+
+const Thing Thing::One = Thing(1);
+
+export struct C {
+int value;
+};
+export const C ConstantValue = {1};
+
+export const C *ConstantPtr =
+
+C NonConstantValue = {1};
+export const C = NonConstantValue;
+
+//--- use.cpp
+import mod;
+
+int consume(int);
+int consumeC(C);
+
+extern "C" __attribute__((noinline)) inline int unneeded() {
+return consume(43);
+}
+
+extern "C" __attribute__((noinline)) inline int needed() {
+return consume(43);
+}
+
+int use() {
+Thing t1 = Thing::One;
+return consume(t1.raw);
+}
+
+int use2() {
+if (ConstantValue.value)
+return consumeC(ConstantValue);
+return unneeded();
+}
+
+int use3() {
+if (ConstantPtr->value)
+return consumeC(*ConstantPtr);
+return needed();
+}
+
+int use4() {
+if (ConstantRef.value)
+return consumeC(ConstantRef);
+return needed();
+}
+
+// CHECK-NOT: @_ZNW3mod5Thing3OneE = {{.*}}constant
+// CHECK: @_ZW3mod13ConstantValue ={{.*}}available_externally{{.*}} constant
+
+// Check that the middle end can optimize the program by the constant
information.
+// CHECK-NOT: @unneeded(
+
+// Check that the use of ConstantPtr won't get optimized incorrectly.
+// CHECK-LABEL: @_Z4use3v(
+// CHECK: @needed(
+
+// Check that the use of ConstantRef won't get optimized incorrectly.
+// CHECK-LABEL: @_Z4use4v(
+// CHECK: @needed(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 34ba1c0 - [NFC] [Serialization] Emit Name for DECL_EXPORT
Author: Chuanqi Xu
Date: 2024-05-28T14:27:48+08:00
New Revision: 34ba1c043af0c3bbcbc1c9e66fbcc6509e4b8e9d
URL:
https://github.com/llvm/llvm-project/commit/34ba1c043af0c3bbcbc1c9e66fbcc6509e4b8e9d
DIFF:
https://github.com/llvm/llvm-project/commit/34ba1c043af0c3bbcbc1c9e66fbcc6509e4b8e9d.diff
LOG: [NFC] [Serialization] Emit Name for DECL_EXPORT
Added:
Modified:
clang/lib/Serialization/ASTWriter.cpp
clang/test/Modules/no-implicit-declarations.cppm
Removed:
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index a85cd94fd5b5a..dd548fabfd955 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1049,6 +1049,7 @@ void ASTWriter::WriteBlockInfoBlock() {
RECORD(DECL_UNRESOLVED_USING_VALUE);
RECORD(DECL_UNRESOLVED_USING_TYPENAME);
RECORD(DECL_LINKAGE_SPEC);
+ RECORD(DECL_EXPORT);
RECORD(DECL_CXX_RECORD);
RECORD(DECL_CXX_METHOD);
RECORD(DECL_CXX_CONSTRUCTOR);
diff --git a/clang/test/Modules/no-implicit-declarations.cppm
b/clang/test/Modules/no-implicit-declarations.cppm
index 319d3a432ea23..79c3c5e76f63e 100644
--- a/clang/test/Modules/no-implicit-declarations.cppm
+++ b/clang/test/Modules/no-implicit-declarations.cppm
@@ -17,7 +17,7 @@ export int a = 43;
// CHECK: https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,88 @@
+//===--- CIRGenAction.cpp - LLVM Code generation Frontend Action -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/CIRFrontendAction/CIRGenAction.h"
+#include "clang/CIR/CIRGenerator.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+#include "mlir/IR/MLIRContext.h"
+#include "mlir/IR/OwningOpRef.h"
+
+using namespace cir;
+using namespace clang;
+
+namespace cir {
+
+class CIRGenConsumer : public clang::ASTConsumer {
+
+ virtual void anchor();
+
+ [[maybe_unused]] CIRGenAction::OutputType action;
+
+ [[maybe_unused]] DiagnosticsEngine
+ [[maybe_unused]] const HeaderSearchOptions
+ [[maybe_unused]] const CodeGenOptions
+ [[maybe_unused]] const TargetOptions
+ [[maybe_unused]] const LangOptions
+ [[maybe_unused]] const FrontendOptions
+
+ std::unique_ptr outputStream;
+
+ [[maybe_unused]] ASTContext *astContext{nullptr};
+ IntrusiveRefCntPtr FS;
+ std::unique_ptr gen;
+
+public:
+ CIRGenConsumer(CIRGenAction::OutputType action,
+ DiagnosticsEngine ,
+ IntrusiveRefCntPtr VFS,
+ const HeaderSearchOptions ,
+ const CodeGenOptions ,
+ const TargetOptions ,
+ const LangOptions ,
+ const FrontendOptions ,
+ std::unique_ptr os)
+ : action(action), diagnosticsEngine(diagnosticsEngine),
+headerSearchOptions(headerSearchOptions),
+codeGenOptions(codeGenOptions), targetOptions(targetOptions),
+langOptions(langOptions), feOptions(feOptions),
+outputStream(std::move(os)), FS(VFS),
+gen(std::make_unique(diagnosticsEngine, std::move(VFS),
+ codeGenOptions)) {}
+
+ bool HandleTopLevelDecl(DeclGroupRef D) override {
+gen->HandleTopLevelDecl(D);
+return true;
+ }
+};
+} // namespace cir
+
+void CIRGenConsumer::anchor() {}
+
+CIRGenAction::CIRGenAction(OutputType act, mlir::MLIRContext *mlirContext)
+: mlirContext(mlirContext ? mlirContext : new mlir::MLIRContext),
+ action(act) {}
+
+CIRGenAction::~CIRGenAction() { mlirModule.release(); }
+
+std::unique_ptr
+CIRGenAction::CreateASTConsumer(CompilerInstance , StringRef inputFile) {
+ auto out = ci.takeOutputStream();
+
+ auto Result = std::make_unique(
+ action, ci.getDiagnostics(), (),
+ ci.getHeaderSearchOpts(), ci.getCodeGenOpts(), ci.getTargetOpts(),
+ ci.getLangOpts(), ci.getFrontendOpts(), std::move(out));
+ cgConsumer = Result.get();
+
+ return std::move(Result);
ChuanqiXu9 wrote:
It may be pessimizing move
```suggestion
return Result;
```
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,59 @@
+//===- CIRGenerator.h - CIR Generation from Clang AST
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file declares a simple interface to perform CIR generation from Clang
+// AST
+//
+//===--===//
+
+#ifndef CLANG_CIRGENERATOR_H_
+#define CLANG_CIRGENERATOR_H_
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/DeclGroup.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+#include
+
+namespace mlir {
+class MLIRContext;
+} // namespace mlir
+namespace cir {
+class CIRGenModule;
+
+class CIRGenerator : public clang::ASTConsumer {
+ virtual void anchor();
+ clang::DiagnosticsEngine
+ clang::ASTContext *astCtx;
+ llvm::IntrusiveRefCntPtr
+ fs; // Only used for debug info.
+
+ const clang::CodeGenOptions codeGenOpts; // Intentionally copied in.
ChuanqiXu9 wrote:
It may be helpful to add a comment to explain why this is intentionally copied
in?
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,61 @@ +//=== CIRGenAction.h - CIR Code Generation Frontend Action -*- C++ -*--===// ChuanqiXu9 wrote: Should we move this header to `CIR` or `FrontendAction`? Currently it lives in `CIRFrontendAction` but its implementation file lives in `FrontendAction` https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -42,6 +47,14 @@ CreateFrontendBaseAction(CompilerInstance ) {
StringRef Action("unknown");
(void)Action;
+ auto UseCIR = CI.getFrontendOpts().UseClangIRPipeline;
+ auto Act = CI.getFrontendOpts().ProgramAction;
+ auto EmitsCIR = Act == EmitCIR;
+
+ if (!UseCIR && EmitsCIR)
+llvm::report_fatal_error(
+"-emit-cir and -emit-cir-only only valid when using -fclangir");
ChuanqiXu9 wrote:
What is `-emit-cir-only`? Should we remove that?
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
@@ -0,0 +1,88 @@
+//===--- CIRGenAction.cpp - LLVM Code generation Frontend Action -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/CIRFrontendAction/CIRGenAction.h"
+#include "clang/CIR/CIRGenerator.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+#include "mlir/IR/MLIRContext.h"
+#include "mlir/IR/OwningOpRef.h"
+
+using namespace cir;
+using namespace clang;
+
+namespace cir {
+
+class CIRGenConsumer : public clang::ASTConsumer {
+
+ virtual void anchor();
+
+ [[maybe_unused]] CIRGenAction::OutputType action;
+
+ [[maybe_unused]] DiagnosticsEngine
+ [[maybe_unused]] const HeaderSearchOptions
+ [[maybe_unused]] const CodeGenOptions
+ [[maybe_unused]] const TargetOptions
+ [[maybe_unused]] const LangOptions
+ [[maybe_unused]] const FrontendOptions
+
+ std::unique_ptr outputStream;
+
+ [[maybe_unused]] ASTContext *astContext{nullptr};
+ IntrusiveRefCntPtr FS;
+ std::unique_ptr gen;
+
+public:
+ CIRGenConsumer(CIRGenAction::OutputType action,
+ DiagnosticsEngine ,
+ IntrusiveRefCntPtr VFS,
+ const HeaderSearchOptions ,
+ const CodeGenOptions ,
+ const TargetOptions ,
+ const LangOptions ,
+ const FrontendOptions ,
+ std::unique_ptr os)
+ : action(action), diagnosticsEngine(diagnosticsEngine),
+headerSearchOptions(headerSearchOptions),
+codeGenOptions(codeGenOptions), targetOptions(targetOptions),
+langOptions(langOptions), feOptions(feOptions),
+outputStream(std::move(os)), FS(VFS),
+gen(std::make_unique(diagnosticsEngine, std::move(VFS),
+ codeGenOptions)) {}
+
+ bool HandleTopLevelDecl(DeclGroupRef D) override {
+gen->HandleTopLevelDecl(D);
+return true;
+ }
+};
+} // namespace cir
+
+void CIRGenConsumer::anchor() {}
+
+CIRGenAction::CIRGenAction(OutputType act, mlir::MLIRContext *mlirContext)
+: mlirContext(mlirContext ? mlirContext : new mlir::MLIRContext),
+ action(act) {}
+
+CIRGenAction::~CIRGenAction() { mlirModule.release(); }
+
+std::unique_ptr
+CIRGenAction::CreateASTConsumer(CompilerInstance , StringRef inputFile) {
+ auto out = ci.takeOutputStream();
+
+ auto Result = std::make_unique(
+ action, ci.getDiagnostics(), (),
+ ci.getHeaderSearchOpts(), ci.getCodeGenOpts(), ci.getTargetOpts(),
+ ci.getLangOpts(), ci.getFrontendOpts(), std::move(out));
+ cgConsumer = Result.get();
ChuanqiXu9 wrote:
If I read correctly, `cgConsumer` is only used here? I guess it is needed in
following patches. But slightly odd.
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)
https://github.com/ChuanqiXu9 commented: BTW, it will be helpful to create subscribing team to help people to get informed in time. (I just saw the patch accidently.) https://github.com/llvm/llvm-project/pull/91007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't record implicitly declarations to BMI by default (PR #93459)
https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/93459 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Be const-correct with all uses of `Module *`. (PR #93493)
ChuanqiXu9 wrote: > > Can you make sure that at every place this PR touches `const` makes sense? > > I found out recently that we can be quite good at pretending that something > > is `const`, all the way down until we realize we need a `const_cast`, > > because modification is required in that one place. > > I'm not quite sure I understand the question. This PR doesn't add any > `const_cast`, and `const` is checked by the compiler so a successful build > shows that we're never modifying something declared `const`. What additional > work are you wanting? The question is that it may be fine to be `const` today but it becomes not the case later. So we may have to make const function back to non-const function again. So one style to do such things is to understand that the new `const` decorated places are meant to be `const`. Otherwise I'll suggest to only mark the places that need to be change by the following patch as `const`. https://github.com/llvm/llvm-project/pull/93493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [libcxx] [clang][Modules] Remove unnecessary includes of `Module.h` (PR #93417)
@@ -159,7 +159,8 @@ class APINotesManager {
ArrayRef getCurrentModuleReaders() const {
bool HasPublic = CurrentModuleReaders[ReaderKind::Public];
bool HasPrivate = CurrentModuleReaders[ReaderKind::Private];
-assert((!HasPrivate || HasPublic) && "private module requires public
module");
+assert((!HasPrivate || HasPublic) &&
+ "private module requires public module");
ChuanqiXu9 wrote:
Yes, we prefer to format the changed line only. Otherwise the backporting may
be problematic. And git blaming will be harder.
One possible way maybe:
> git diff -U0 --no-color --relative HEAD^ |
> clang/tools/clang-format/clang-format-diff.py -p1 -i
https://github.com/llvm/llvm-project/pull/93417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C++20] [Modules] Don't record implicitly declarations to BMI by default (PR #93459)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/93459
I found we may insert unused implciit declarations like AArch SVE declarations
by default on AArch64 due to we will insert that by default. But it should be
completely redundant and this patch tries to remove that.
>From b9cc02ffea56214179fd70c3a0e206932a557f8f Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Mon, 27 May 2024 19:25:49 +0800
Subject: [PATCH] [C++20] [Modules] Don't record implicitly declarations to BMI
by default
I found we may insert unused implciit declarations like AArch SVE
declarations by default on AArch64 due to we will insert that by
default. But it should be completely redundant and this patch tries to
remove that.
---
clang/lib/Serialization/ASTWriter.cpp | 13 --
.../Modules/no-implicit-declarations.cppm | 26 +++
2 files changed, 37 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Modules/no-implicit-declarations.cppm
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 00b0e48083217..a85cd94fd5b5a 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -5037,6 +5037,14 @@ void ASTWriter::PrepareWritingSpecialDecls(Sema
) {
continue;
}
+// If we're writing C++ named modules, don't emit declarations which are
+// not from modules by default. They may be built in declarations (be
+// handled above) or implcit declarations (see the implementation of
+// `Sema::Initialize()` for example).
+if (isWritingStdCXXNamedModules() && !D->getOwningModule() &&
+D->isImplicit())
+ continue;
+
GetDeclRef(D);
}
@@ -6197,8 +6205,9 @@ bool ASTWriter::wasDeclEmitted(const Decl *D) const {
return true;
bool Emitted = DeclIDs.contains(D);
- assert((Emitted || GeneratingReducedBMI) &&
- "The declaration can only be omitted in reduced BMI.");
+ assert((Emitted || (!D->getOwningModule() && isWritingStdCXXNamedModules())
||
+ GeneratingReducedBMI) &&
+ "The declaration within modules can only be omitted in reduced BMI.");
return Emitted;
}
diff --git a/clang/test/Modules/no-implicit-declarations.cppm
b/clang/test/Modules/no-implicit-declarations.cppm
new file mode 100644
index 0..319d3a432ea23
--- /dev/null
+++ b/clang/test/Modules/no-implicit-declarations.cppm
@@ -0,0 +1,26 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+//
+// RUN: %clang_cc1 -std=c++20 %s -emit-module-interface -o %t/a.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs
%t/a.pcm > %t/a.dump
+// RUN: cat %t/a.dump | FileCheck %s
+//
+// RUN: %clang_cc1 -std=c++20 %s -emit-reduced-module-interface -o %t/a.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs
%t/a.pcm > %t/a.dump
+// RUN: cat %t/a.dump | FileCheck %s
+
+export module a;
+// Contain something at least to make sure the compiler won't
+// optimize this out.
+export int a = 43;
+
+// CHECK:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
ChuanqiXu9 wrote: Thanks for reviewing : ) https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
@@ -111,6 +109,28 @@ void *Decl::operator new(std::size_t Size, const
ASTContext ,
return ::operator new(Size + Extra, Ctx);
}
+GlobalDeclID Decl::getGlobalID() const {
+ if (!isFromASTFile())
+return GlobalDeclID();
+ uint64_t ID = *((const uint64_t *)this - 1);
ChuanqiXu9 wrote:
Previously, for deserialized declaration, clang would apply an additional 64
bits for it. And clang would store the global declaration ID at the lower 32
bits and store the module index at the higher 32 bits.
And after this patch, we extend the global declaration ID to 64 bits and I
don't want to apply additional spaces. So I decided to use the lower 48 bits to
store the declaration ID and the higher 16 bits to store the module index.
I added a comment here to clarify that.
And as I mentioned in the message, it looks like there are relationships
between module index and module file index (the upper 16 bits of the new
declaration ID). But on the one hand, I want to make the patch as simple as
possible to make it land faster. On the other hand, it looks like 16 bits is
enough for both module index and module file index. And we can't get any
improvements by merge it. So I just leave it as is.
https://github.com/llvm/llvm-project/pull/92083
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
@@ -7802,20 +7800,31 @@ Decl *ASTReader::GetDecl(GlobalDeclID ID) {
LocalDeclID ASTReader::mapGlobalIDToModuleFileGlobalID(ModuleFile ,
GlobalDeclID GlobalID) {
- DeclID ID = GlobalID.get();
- if (ID < NUM_PREDEF_DECL_IDS)
+ if (GlobalID.get() < NUM_PREDEF_DECL_IDS)
+return LocalDeclID(GlobalID.get());
+
+ if (!M.ModuleOffsetMap.empty())
+ReadModuleOffsetMap(M);
+
+ ModuleFile *Owner = getOwningModuleFile(GlobalID);
+ DeclID ID = GlobalID.getLocalDeclIndex();
+
+ if (Owner == ) {
+ID += NUM_PREDEF_DECL_IDS;
return LocalDeclID(ID);
+ }
- GlobalDeclMapType::const_iterator I = GlobalDeclMap.find(GlobalID);
- assert(I != GlobalDeclMap.end() && "Corrupted global declaration map");
- ModuleFile *Owner = I->second;
+ uint64_t OrignalModuleFileIndex = 0;
+ for (unsigned I = 0; I < M.DependentModules.size(); I++)
ChuanqiXu9 wrote:
Done in
https://github.com/llvm/llvm-project/commit/b590ba73a76609bace9949ea8195d2ee8213cb3f
https://github.com/llvm/llvm-project/pull/92083
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
@@ -255,6 +255,12 @@ class DeclOffset {
}
};
+// The unaligned decl ID used in the Blobs of bistreams.
+using unalighed_decl_id_t =
ChuanqiXu9 wrote:
Nice catch! Done in
https://github.com/llvm/llvm-project/commit/e73e4951b20c70f24354e2a2820876c818dcaee3
https://github.com/llvm/llvm-project/pull/92083
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
@@ -124,6 +130,15 @@ class DeclIDBase {
bool isInvalid() const { return ID == PREDEF_DECL_NULL_ID; }
+ unsigned getModuleFileIndex() const { return ID >> 32; }
+
+ unsigned getLocalDeclIndex() const {
+// Implement it directly instead of calling `llvm::maskTrailingOnes` since
+// we don't want `MathExtras.h` to be inclued here.
ChuanqiXu9 wrote:
Maybe I just didn't want to create an implementation file for such a single
simple function. But I just realized I can append it in `DeclBase.cpp` just
like many other classes did. So done.
https://github.com/llvm/llvm-project/pull/92083
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/92083
>From 8b7a650e128d6f2bfec9b0768169cf4aaa4af337 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Fri, 10 May 2024 15:36:31 +0800
Subject: [PATCH] [serialization] no transitive decl change
---
clang/include/clang/AST/DeclBase.h| 17 +-
clang/include/clang/AST/DeclID.h | 18 +-
.../include/clang/Serialization/ASTBitCodes.h | 6 +
clang/include/clang/Serialization/ASTReader.h | 36 ++--
.../include/clang/Serialization/ModuleFile.h | 18 +-
.../clang/Serialization/ModuleManager.h | 2 +-
clang/lib/AST/DeclBase.cpp| 40 -
clang/lib/Serialization/ASTReader.cpp | 159 ++
clang/lib/Serialization/ASTReaderDecl.cpp | 12 +-
clang/lib/Serialization/ASTWriter.cpp | 7 +-
clang/lib/Serialization/ModuleFile.cpp| 3 +-
.../Modules/no-transitive-decls-change.cppm | 112
12 files changed, 283 insertions(+), 147 deletions(-)
create mode 100644 clang/test/Modules/no-transitive-decls-change.cppm
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index e43e812cd9455..4bdf27aa99405 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -701,10 +701,7 @@ class alignas(8) Decl {
/// Set the owning module ID. This may only be called for
/// deserialized Decls.
- void setOwningModuleID(unsigned ID) {
-assert(isFromASTFile() && "Only works on a deserialized declaration");
-*((unsigned*)this - 2) = ID;
- }
+ void setOwningModuleID(unsigned ID);
public:
/// Determine the availability of the given declaration.
@@ -777,19 +774,11 @@ class alignas(8) Decl {
/// Retrieve the global declaration ID associated with this
/// declaration, which specifies where this Decl was loaded from.
- GlobalDeclID getGlobalID() const {
-if (isFromASTFile())
- return (*((const GlobalDeclID *)this - 1));
-return GlobalDeclID();
- }
+ GlobalDeclID getGlobalID() const;
/// Retrieve the global ID of the module that owns this particular
/// declaration.
- unsigned getOwningModuleID() const {
-if (isFromASTFile())
- return *((const unsigned*)this - 2);
-return 0;
- }
+ unsigned getOwningModuleID() const;
private:
Module *getOwningModuleSlow() const;
diff --git a/clang/include/clang/AST/DeclID.h b/clang/include/clang/AST/DeclID.h
index 614ba06b63860..32d2ed41a374a 100644
--- a/clang/include/clang/AST/DeclID.h
+++ b/clang/include/clang/AST/DeclID.h
@@ -19,6 +19,8 @@
#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/ADT/iterator.h"
+#include
+
namespace clang {
/// Predefined declaration IDs.
@@ -107,12 +109,16 @@ class DeclIDBase {
///
/// DeclID should only be used directly in serialization. All other users
/// should use LocalDeclID or GlobalDeclID.
- using DeclID = uint32_t;
+ using DeclID = uint64_t;
protected:
DeclIDBase() : ID(PREDEF_DECL_NULL_ID) {}
explicit DeclIDBase(DeclID ID) : ID(ID) {}
+ explicit DeclIDBase(unsigned LocalID, unsigned ModuleFileIndex) {
+ID = (DeclID)LocalID | ((DeclID)ModuleFileIndex << 32);
+ }
+
public:
DeclID get() const { return ID; }
@@ -124,6 +130,10 @@ class DeclIDBase {
bool isInvalid() const { return ID == PREDEF_DECL_NULL_ID; }
+ unsigned getModuleFileIndex() const { return ID >> 32; }
+
+ unsigned getLocalDeclIndex() const;
+
friend bool operator==(const DeclIDBase , const DeclIDBase ) {
return LHS.ID == RHS.ID;
}
@@ -156,6 +166,9 @@ class LocalDeclID : public DeclIDBase {
LocalDeclID(PredefinedDeclIDs ID) : Base(ID) {}
explicit LocalDeclID(DeclID ID) : Base(ID) {}
+ explicit LocalDeclID(unsigned LocalID, unsigned ModuleFileIndex)
+ : Base(LocalID, ModuleFileIndex) {}
+
LocalDeclID ++() {
++ID;
return *this;
@@ -175,6 +188,9 @@ class GlobalDeclID : public DeclIDBase {
GlobalDeclID() : Base() {}
explicit GlobalDeclID(DeclID ID) : Base(ID) {}
+ explicit GlobalDeclID(unsigned LocalID, unsigned ModuleFileIndex)
+ : Base(LocalID, ModuleFileIndex) {}
+
// For DeclIDIterator to be able to convert a GlobalDeclID
// to a LocalDeclID.
explicit operator LocalDeclID() const { return LocalDeclID(this->ID); }
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h
b/clang/include/clang/Serialization/ASTBitCodes.h
index fe1bd47348be1..9e4b21baa7d20 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -255,6 +255,12 @@ class DeclOffset {
}
};
+// The unaligned decl ID used in the Blobs of bistreams.
+using unaligned_decl_id_t =
+llvm::support::detail::packed_endian_specific_integral<
+serialization::DeclID, llvm::endianness::native,
+llvm::support::unaligned>;
+
/// The number of predefined preprocessed entity IDs.
const unsigned int NUM_PREDEF_PP_ENTITY_IDS = 1;
[clang] b590ba7 - [NFC] Rename 'DependentModules' in ModuleFile to `TransitiveImports`
Author: Chuanqi Xu
Date: 2024-05-27T13:53:38+08:00
New Revision: b590ba73a76609bace9949ea8195d2ee8213cb3f
URL:
https://github.com/llvm/llvm-project/commit/b590ba73a76609bace9949ea8195d2ee8213cb3f
DIFF:
https://github.com/llvm/llvm-project/commit/b590ba73a76609bace9949ea8195d2ee8213cb3f.diff
LOG: [NFC] Rename 'DependentModules' in ModuleFile to `TransitiveImports`
Required in the review process of
https://github.com/llvm/llvm-project/pull/92083
Added:
Modified:
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ModuleFile.h
clang/lib/Serialization/ASTReader.cpp
Removed:
diff --git a/clang/include/clang/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index 75e41ea91715b..4ece4593f0738 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2246,7 +2246,7 @@ class ASTReader
auto [Loc, ModuleFileIndex] = ReadUntranslatedSourceLocation(Raw, Seq);
ModuleFile *OwningModuleFile =
-ModuleFileIndex == 0 ? : MF.DependentModules[ModuleFileIndex - 1];
+ModuleFileIndex == 0 ? : MF.TransitiveImports[ModuleFileIndex - 1];
assert(!SourceMgr.isLoadedSourceLocation(Loc) &&
"Run out source location space");
diff --git a/clang/include/clang/Serialization/ModuleFile.h
b/clang/include/clang/Serialization/ModuleFile.h
index 7d8cbe3d40f56..992d26a8b88c1 100644
--- a/clang/include/clang/Serialization/ModuleFile.h
+++ b/clang/include/clang/Serialization/ModuleFile.h
@@ -513,11 +513,11 @@ class ModuleFile {
/// List of modules which this modules dependent on. Different
/// from `Imports`, this includes indirectly imported modules too.
- /// The order of DependentModules is significant. It should keep
+ /// The order of TransitiveImports is significant. It should keep
/// the same order with that module file manager when we write
/// the current module file. The value of the member will be initialized
/// in `ASTReader::ReadModuleOffsetMap`.
- llvm::SmallVector DependentModules;
+ llvm::SmallVector TransitiveImports;
/// Determine whether this module was directly imported at
/// any point during translation.
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 54414af3a646e..4a6e1d23161be 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -4059,7 +4059,7 @@ void ASTReader::ReadModuleOffsetMap(ModuleFile ) const {
RemapBuilder DeclRemap(F.DeclRemap);
RemapBuilder TypeRemap(F.TypeRemap);
- auto = F.DependentModules;
+ auto = F.TransitiveImports;
assert(ImportedModuleVector.empty());
while (Data < DataEnd) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] e73e495 - [NFC] Fix typo unalighed_decl_id_t
Author: Chuanqi Xu
Date: 2024-05-27T13:48:00+08:00
New Revision: e73e4951b20c70f24354e2a2820876c818dcaee3
URL:
https://github.com/llvm/llvm-project/commit/e73e4951b20c70f24354e2a2820876c818dcaee3
DIFF:
https://github.com/llvm/llvm-project/commit/e73e4951b20c70f24354e2a2820876c818dcaee3.diff
LOG: [NFC] Fix typo unalighed_decl_id_t
It should be unalignhed_decl_id_t
Added:
Modified:
clang/include/clang/Serialization/ASTReader.h
clang/lib/Serialization/ASTReader.cpp
Removed:
diff --git a/clang/include/clang/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index 1bb5fa27a2419..75e41ea91715b 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -601,11 +601,11 @@ class ASTReader
/// An array of lexical contents of a declaration context, as a sequence of
/// Decl::Kind, DeclID pairs.
- using unalighed_decl_id_t =
+ using unaligned_decl_id_t =
llvm::support::detail::packed_endian_specific_integral<
serialization::DeclID, llvm::endianness::native,
llvm::support::unaligned>;
- using LexicalContents = ArrayRef;
+ using LexicalContents = ArrayRef;
/// Map from a DeclContext to its lexical contents.
llvm::DenseMap>
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index d7fc6697eaf74..54414af3a646e 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -1264,7 +1264,7 @@ bool ASTReader::ReadLexicalDeclContextStorage(ModuleFile
,
if (!Lex.first) {
Lex = std::make_pair(
, llvm::ArrayRef(
-reinterpret_cast(Blob.data()),
+reinterpret_cast(Blob.data()),
Blob.size() / sizeof(DeclID)));
}
DC->setHasExternalLexicalStorage(true);
@@ -3401,7 +3401,7 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile ,
case TU_UPDATE_LEXICAL: {
DeclContext *TU = ContextObj->getTranslationUnitDecl();
LexicalContents Contents(
- reinterpret_cast(Blob.data()),
+ reinterpret_cast(Blob.data()),
static_cast(Blob.size() / sizeof(DeclID)));
TULexicalDecls.push_back(std::make_pair(, Contents));
TU->setHasExternalLexicalStorage(true);
___
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: Oh, I don't know why I didn't get this in files page so I missed this. But since we can't get rid of writing/reading the modules actually in `ModulesBuilder` (Or it is pretty hard). Then it looks not so worthy to introduce the layer. 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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,339 @@
+//===- ModulesBuilder.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "PrerequisiteModules.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+/// Get or create a path to store module files. Generally it should be:
+///
+/// project_root/.cache/clangd/module_files/{RequiredPrefixDir}/.
+///
+/// \param MainFile is used to get the root of the project from global
+/// compilation database. \param RequiredPrefixDir is used to get the user
+/// defined prefix for module files. This is useful when we want to seperate
+/// module files. e.g., we want to build module files for the same module unit
+/// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the
+/// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then
+/// we can put the 2 module files into different dirs like:
+///
+/// project_root/.cache/clangd/module_files/b.cpp/a.pcm
+/// project_root/.cache/clangd/module_files/c.cpp/a.pcm
+llvm::SmallString<256> getModuleFilesPath(PathRef MainFile,
+ const GlobalCompilationDatabase ,
+ StringRef RequiredPrefixDir) {
+ std::optional PI = CDB.getProjectInfo(MainFile);
+ if (!PI)
+return {};
+
+ // FIXME: PI->SourceRoot may be empty, depending on the CDB strategy.
+ llvm::SmallString<256> Result(PI->SourceRoot);
+
+ llvm::sys::path::append(Result, ".cache");
+ llvm::sys::path::append(Result, "clangd");
+ llvm::sys::path::append(Result, "module_files");
+
+ llvm::sys::path::append(Result, RequiredPrefixDir);
+
+ llvm::sys::fs::create_directories(Result, /*IgnoreExisting=*/true);
+
+ return Result;
+}
+
+/// Get the absolute path for the filename from the compile command.
+llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand ) {
+ llvm::SmallString<128> AbsolutePath;
+ if (llvm::sys::path::is_absolute(Cmd.Filename)) {
+AbsolutePath = Cmd.Filename;
+ } else {
+AbsolutePath = Cmd.Directory;
+llvm::sys::path::append(AbsolutePath, Cmd.Filename);
+llvm::sys::path::remove_dots(AbsolutePath, true);
+ }
+ return AbsolutePath;
+}
+
+/// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getUniqueModuleFilePath(StringRef ModuleName,
+PathRef ModuleFilesPrefix) {
+ llvm::SmallString<256> ModuleFilePattern(ModuleFilesPrefix);
+ auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+ llvm::sys::path::append(ModuleFilePattern, PrimaryModuleName);
+ if (!PartitionName.empty()) {
+ModuleFilePattern.append("-");
+ModuleFilePattern.append(PartitionName);
+ }
+
+ ModuleFilePattern.append("-%%-%%-%%-%%-%%-%%");
+ ModuleFilePattern.append(".pcm");
+
+ llvm::SmallString<256> ModuleFilePath;
+ llvm::sys::fs::createUniquePath(ModuleFilePattern, ModuleFilePath,
+ /*MakeAbsolute=*/false);
+
+ return (std::string)ModuleFilePath;
+}
+} // namespace
+
+bool ModulesBuilder::buildModuleFile(StringRef ModuleName,
+ const ThreadsafeFS *TFS,
+ std::shared_ptr MDB,
+ PathRef ModuleFilesPrefix,
+ PrerequisiteModules ) {
+ if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName))
+return true;
+
+ PathRef ModuleUnitFileName = MDB->getSourceForModuleName(ModuleName);
+ /// It is possible that we're meeting third party modules (modules whose
+ /// source are not in the project. e.g, the std module may be a third-party
+ /// module for most project) or something wrong with the implementation of
+ /// ProjectModules.
+ /// FIXME: How should we treat third party modules here? If we want to ignore
+ /// third party modules, we should return true instead of false here.
+ /// Currently we simply bail out.
+ if (ModuleUnitFileName.empty())
+return false;
+
+ for (auto : MDB->getRequiredModules(ModuleUnitFileName))
{
+// Return early if there are errors building the module file.
+if (!buildModuleFile(RequiredModuleName, TFS, MDB, ModuleFilesPrefix,
+ BuiltModuleFiles)) {
+ log("Failed to build module {0}", RequiredModuleName);
+ return false;
+}
+ }
+
+ auto Cmd = CDB.getCompileCommand(ModuleUnitFileName);
+ if (!Cmd)
+return false;
+
+ std::string ModuleFileName =
+ getUniqueModuleFilePath(ModuleName, ModuleFilesPrefix);
+ Cmd->Output =
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,62 @@
+//===-- ProjectModules.h -*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ProjectModules.h"
+
+namespace clang {
+namespace clangd {
+
+/// TODO: The existing `ScanningAllProjectModules` is not efficient. See the
+/// comments in ModuleDependencyScanner for detail.
+///
+/// In the future, we wish the build system can provide a well design
+/// compilation database for modules then we can query that new compilation
+/// database directly. Or we need to have a global long-live scanner to detect
+/// the state of each file.
+class ScanningAllProjectModules : public ProjectModules {
+public:
+ ScanningAllProjectModules(std::vector &,
+const GlobalCompilationDatabase ,
+const ThreadsafeFS )
+ : AllFiles(std::move(AllFiles)), Scanner(CDB, TFS) {}
+
+ ~ScanningAllProjectModules() override = default;
+
+ std::vector getRequiredModules(PathRef File) override {
+return Scanner.getRequiredModules(File);
+ }
+
+ /// RequiredSourceFile is not used intentionally. See the comments of
+ /// ModuleDependencyScanner for detail.
+ PathRef
+ getSourceForModuleName(StringRef ModuleName,
+ PathRef RequiredSourceFile = PathRef()) override {
+if (!Scanner.isGlobalScanned())
+ Scanner.globalScan(AllFiles);
+
+return Scanner.getSourceForModuleName(ModuleName);
+ }
+
+private:
+ std::vector AllFiles;
+
+ ModuleDependencyScanner Scanner;
ChuanqiXu9 wrote:
Done. Good idea. I like it.
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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,339 @@
+//===- ModulesBuilder.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "PrerequisiteModules.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+/// Get or create a path to store module files. Generally it should be:
+///
+/// project_root/.cache/clangd/module_files/{RequiredPrefixDir}/.
+///
+/// \param MainFile is used to get the root of the project from global
+/// compilation database. \param RequiredPrefixDir is used to get the user
+/// defined prefix for module files. This is useful when we want to seperate
+/// module files. e.g., we want to build module files for the same module unit
+/// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the
+/// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then
+/// we can put the 2 module files into different dirs like:
+///
+/// project_root/.cache/clangd/module_files/b.cpp/a.pcm
+/// project_root/.cache/clangd/module_files/c.cpp/a.pcm
+llvm::SmallString<256> getModuleFilesPath(PathRef MainFile,
+ const GlobalCompilationDatabase ,
+ StringRef RequiredPrefixDir) {
+ std::optional PI = CDB.getProjectInfo(MainFile);
+ if (!PI)
+return {};
+
+ // FIXME: PI->SourceRoot may be empty, depending on the CDB strategy.
+ llvm::SmallString<256> Result(PI->SourceRoot);
+
+ llvm::sys::path::append(Result, ".cache");
+ llvm::sys::path::append(Result, "clangd");
+ llvm::sys::path::append(Result, "module_files");
+
+ llvm::sys::path::append(Result, RequiredPrefixDir);
+
+ llvm::sys::fs::create_directories(Result, /*IgnoreExisting=*/true);
+
+ return Result;
+}
+
+/// Get the absolute path for the filename from the compile command.
+llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand ) {
+ llvm::SmallString<128> AbsolutePath;
+ if (llvm::sys::path::is_absolute(Cmd.Filename)) {
+AbsolutePath = Cmd.Filename;
+ } else {
+AbsolutePath = Cmd.Directory;
+llvm::sys::path::append(AbsolutePath, Cmd.Filename);
+llvm::sys::path::remove_dots(AbsolutePath, true);
+ }
+ return AbsolutePath;
+}
+
+/// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getUniqueModuleFilePath(StringRef ModuleName,
+PathRef ModuleFilesPrefix) {
+ llvm::SmallString<256> ModuleFilePattern(ModuleFilesPrefix);
+ auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+ llvm::sys::path::append(ModuleFilePattern, PrimaryModuleName);
+ if (!PartitionName.empty()) {
+ModuleFilePattern.append("-");
+ModuleFilePattern.append(PartitionName);
+ }
+
+ ModuleFilePattern.append("-%%-%%-%%-%%-%%-%%");
+ ModuleFilePattern.append(".pcm");
+
+ llvm::SmallString<256> ModuleFilePath;
+ llvm::sys::fs::createUniquePath(ModuleFilePattern, ModuleFilePath,
+ /*MakeAbsolute=*/false);
+
+ return (std::string)ModuleFilePath;
+}
+} // namespace
+
+bool ModulesBuilder::buildModuleFile(StringRef ModuleName,
+ const ThreadsafeFS *TFS,
+ std::shared_ptr MDB,
+ PathRef ModuleFilesPrefix,
+ PrerequisiteModules ) {
+ if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName))
+return true;
+
+ PathRef ModuleUnitFileName = MDB->getSourceForModuleName(ModuleName);
+ /// It is possible that we're meeting third party modules (modules whose
+ /// source are not in the project. e.g, the std module may be a third-party
+ /// module for most project) or something wrong with the implementation of
+ /// ProjectModules.
+ /// FIXME: How should we treat third party modules here? If we want to ignore
+ /// third party modules, we should return true instead of false here.
+ /// Currently we simply bail out.
+ if (ModuleUnitFileName.empty())
+return false;
+
+ for (auto : MDB->getRequiredModules(ModuleUnitFileName))
{
+// Return early if there are errors building the module file.
+if (!buildModuleFile(RequiredModuleName, TFS, MDB, ModuleFilesPrefix,
+ BuiltModuleFiles)) {
+ log("Failed to build module {0}", RequiredModuleName);
+ return false;
+}
+ }
+
+ auto Cmd = CDB.getCompileCommand(ModuleUnitFileName);
+ if (!Cmd)
+return false;
+
+ std::string ModuleFileName =
+ getUniqueModuleFilePath(ModuleName, ModuleFilesPrefix);
+ Cmd->Output =
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,71 @@
+//===- ModulesBuilder.h --*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Experimental support for C++20 Modules.
+//
+// Currently we simplify the implementations by preventing reusing module files
+// across different versions and different source files. But this is clearly a
+// waste of time and space in the end of the day.
+//
+// FIXME: Supporting reusing module files across different versions and
+// different source files.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "ProjectModules.h"
+
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "llvm/ADT/SmallString.h"
+
+#include
+
+namespace clang {
+namespace clangd {
+
+class PrerequisiteModules;
+
+/// This class handles building module files for a given source file.
+///
+/// In the future, we want the class to manage the module files acorss
+/// different versions and different source files.
+class ModulesBuilder {
+public:
+ ModulesBuilder() = delete;
+
+ ModulesBuilder(const GlobalCompilationDatabase ) : CDB(CDB) {}
+
+ ModulesBuilder(const ModulesBuilder &) = delete;
+ ModulesBuilder(ModulesBuilder &&) = delete;
+
+ ModulesBuilder =(const ModulesBuilder &) = delete;
+ ModulesBuilder =(ModulesBuilder &&) = delete;
ChuanqiXu9 wrote:
Primarily a conservative design. Since this is owned by the ClangdLSPServer,
and it looks like we won't copy or move the server. Then it is safe to mark it
as non-copyable and non-movable. It may be easy to convert a non-moveable and
non-copyable to the opposite. But it is not the case vice versa.
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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,71 @@
+//===- ModulesBuilder.h --*-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Experimental support for C++20 Modules.
+//
+// Currently we simplify the implementations by preventing reusing module files
+// across different versions and different source files. But this is clearly a
+// waste of time and space in the end of the day.
+//
+// FIXME: Supporting reusing module files across different versions and
+// different source files.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "ProjectModules.h"
+
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "llvm/ADT/SmallString.h"
+
+#include
+
+namespace clang {
+namespace clangd {
+
+class PrerequisiteModules;
+
+/// This class handles building module files for a given source file.
+///
+/// In the future, we want the class to manage the module files acorss
+/// different versions and different source files.
+class ModulesBuilder {
+public:
+ ModulesBuilder() = delete;
+
+ ModulesBuilder(const GlobalCompilationDatabase ) : CDB(CDB) {}
+
+ ModulesBuilder(const ModulesBuilder &) = delete;
+ ModulesBuilder(ModulesBuilder &&) = delete;
+
+ ModulesBuilder =(const ModulesBuilder &) = delete;
+ ModulesBuilder =(ModulesBuilder &&) = delete;
+
+ ~ModulesBuilder() = default;
+
+ std::unique_ptr
+ buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS *TFS);
+
+private:
+ bool buildModuleFile(StringRef ModuleName, const ThreadsafeFS *TFS,
ChuanqiXu9 wrote:
Done
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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
+/// the comments of `globalScan` to see the details.
+///
+/// ModuleDependencyScanner should only be used via ScanningAllProjectModules.
+///
+/// The ModuleDependencyScanner can get the directly required module name for a
+/// specific source file. Also the ModuleDependencyScanner can get the source
+/// file declaring a specific module name.
+///
+/// IMPORTANT NOTE: we assume that every module unit is only declared once in a
+/// source file in the project. But the assumption is not strictly true even
+/// besides the invalid projects. The language specification requires that
every
+/// module unit should be unique in a valid program. But a project can contain
+/// multiple programs. Then it is valid that we can have multiple source files
+/// declaring the same module in a project as long as these source files don't
+/// interere with each other.`
+class ModuleDependencyScanner {
+public:
+ ModuleDependencyScanner(const GlobalCompilationDatabase ,
+ const ThreadsafeFS )
+ : CDB(CDB), TFS(TFS),
+Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing,
+tooling::dependencies::ScanningOutputFormat::P1689) {}
+
+ // The scanned modules dependency information for a specific source file.
+ struct ModuleDependencyInfo {
+// The name of the module if the file is a module unit.
+std::optional ModuleName;
+// A list of names for the modules that the file directly depends.
+std::vector RequiredModules;
+ };
+
+ /// Scanning the single file specified by \param FilePath.
+ /// NOTE: This is only used by unittests for external uses.
+ std::optional scan(PathRef FilePath);
+
+ /// Scanning every source file in the current project to get the
+ /// to map.
+ /// It looks unefficiency to scan the whole project especially for
+ /// every version of every file!
+ /// TODO: We should find an efficient method to get the
+ /// to map. We can make it either by providing
+ /// a global module dependency scanner to monitor every file. Or we
+ /// can simply require the build systems (or even if the end users)
+ /// to provide the map.
+ void globalScan(const std::vector );
+ bool isGlobalScanned() const { return GlobalScanned; }
ChuanqiXu9 wrote:
It was majorly for efficiency. But as we mentioned multiple times, the
efficiency is not a goal for the first version (it is expected to be slow
already) and let's improve that in the following patches. So I'd like to remove
the interface as you suggested.
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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,339 @@
+//===- ModulesBuilder.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModulesBuilder.h"
+#include "PrerequisiteModules.h"
+#include "support/Logger.h"
+
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+/// Get or create a path to store module files. Generally it should be:
+///
+/// project_root/.cache/clangd/module_files/{RequiredPrefixDir}/.
+///
+/// \param MainFile is used to get the root of the project from global
+/// compilation database. \param RequiredPrefixDir is used to get the user
+/// defined prefix for module files. This is useful when we want to seperate
+/// module files. e.g., we want to build module files for the same module unit
+/// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the
+/// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then
+/// we can put the 2 module files into different dirs like:
+///
+/// project_root/.cache/clangd/module_files/b.cpp/a.pcm
+/// project_root/.cache/clangd/module_files/c.cpp/a.pcm
+llvm::SmallString<256> getModuleFilesPath(PathRef MainFile,
+ const GlobalCompilationDatabase ,
+ StringRef RequiredPrefixDir) {
+ std::optional PI = CDB.getProjectInfo(MainFile);
+ if (!PI)
+return {};
+
+ // FIXME: PI->SourceRoot may be empty, depending on the CDB strategy.
+ llvm::SmallString<256> Result(PI->SourceRoot);
+
+ llvm::sys::path::append(Result, ".cache");
+ llvm::sys::path::append(Result, "clangd");
+ llvm::sys::path::append(Result, "module_files");
+
+ llvm::sys::path::append(Result, RequiredPrefixDir);
+
+ llvm::sys::fs::create_directories(Result, /*IgnoreExisting=*/true);
+
+ return Result;
+}
+
+/// Get the absolute path for the filename from the compile command.
+llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand ) {
+ llvm::SmallString<128> AbsolutePath;
+ if (llvm::sys::path::is_absolute(Cmd.Filename)) {
+AbsolutePath = Cmd.Filename;
+ } else {
+AbsolutePath = Cmd.Directory;
+llvm::sys::path::append(AbsolutePath, Cmd.Filename);
+llvm::sys::path::remove_dots(AbsolutePath, true);
+ }
+ return AbsolutePath;
+}
+
+/// Get a unique module file path under \param ModuleFilesPrefix.
+std::string getUniqueModuleFilePath(StringRef ModuleName,
+PathRef ModuleFilesPrefix) {
+ llvm::SmallString<256> ModuleFilePattern(ModuleFilesPrefix);
+ auto [PrimaryModuleName, PartitionName] = ModuleName.split(':');
+ llvm::sys::path::append(ModuleFilePattern, PrimaryModuleName);
+ if (!PartitionName.empty()) {
+ModuleFilePattern.append("-");
+ModuleFilePattern.append(PartitionName);
+ }
+
+ ModuleFilePattern.append("-%%-%%-%%-%%-%%-%%");
+ ModuleFilePattern.append(".pcm");
+
+ llvm::SmallString<256> ModuleFilePath;
+ llvm::sys::fs::createUniquePath(ModuleFilePattern, ModuleFilePath,
+ /*MakeAbsolute=*/false);
+
+ return (std::string)ModuleFilePath;
+}
+} // namespace
+
+bool ModulesBuilder::buildModuleFile(StringRef ModuleName,
+ const ThreadsafeFS *TFS,
+ std::shared_ptr MDB,
+ PathRef ModuleFilesPrefix,
+ PrerequisiteModules ) {
+ if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName))
+return true;
+
+ PathRef ModuleUnitFileName = MDB->getSourceForModuleName(ModuleName);
+ /// It is possible that we're meeting third party modules (modules whose
+ /// source are not in the project. e.g, the std module may be a third-party
+ /// module for most project) or something wrong with the implementation of
+ /// ProjectModules.
+ /// FIXME: How should we treat third party modules here? If we want to ignore
+ /// third party modules, we should return true instead of false here.
+ /// Currently we simply bail out.
+ if (ModuleUnitFileName.empty())
+return false;
+
+ for (auto : MDB->getRequiredModules(ModuleUnitFileName))
{
+// Return early if there are errors building the module file.
+if (!buildModuleFile(RequiredModuleName, TFS, MDB, ModuleFilesPrefix,
+ BuiltModuleFiles)) {
+ log("Failed to build module {0}", RequiredModuleName);
+ return false;
+}
+ }
+
+ auto Cmd = CDB.getCompileCommand(ModuleUnitFileName);
+ if (!Cmd)
+return false;
+
+ std::string ModuleFileName =
+ getUniqueModuleFilePath(ModuleName, ModuleFilesPrefix);
+ Cmd->Output =
[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
https://github.com/ChuanqiXu9 commented: Thanks for reviewing. It makes the code looks better indeed. 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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,81 @@
+//=== ModuleDependencyScanner.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModuleDependencyScanner.h"
+#include "support/Logger.h"
+
+namespace clang {
+namespace clangd {
+
+std::optional
+ModuleDependencyScanner::scan(PathRef FilePath) {
+ std::optional Cmd = CDB.getCompileCommand(FilePath);
+
+ if (!Cmd)
+return std::nullopt;
+
+ using namespace clang::tooling::dependencies;
+
+ llvm::SmallString<128> FilePathDir(FilePath);
+ llvm::sys::path::remove_filename(FilePathDir);
+ DependencyScanningTool ScanningTool(Service, TFS.view(FilePathDir));
+
+ llvm::Expected ScanningResult =
+ ScanningTool.getP1689ModuleDependencyFile(*Cmd, Cmd->Directory);
+
+ if (auto E = ScanningResult.takeError()) {
+log("Scanning modules dependencies for {0} failed: {1}", FilePath,
+llvm::toString(std::move(E)));
+return std::nullopt;
+ }
+
+ ModuleDependencyInfo Result;
+
+ if (ScanningResult->Provides) {
+ModuleNameToSource[ScanningResult->Provides->ModuleName] = FilePath;
ChuanqiXu9 wrote:
We can't make `scan()` const, since we will pass the member variable `Service`
as a non-const reference to construct `DependencyScanningTool`.
And we can't (or we don't want) assume the `getRequiredModules()` will be
executed after `globalScan()`. We require to run `getSourceForModuleName()`
after `globalScan()` since we have to.
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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
+/// the comments of `globalScan` to see the details.
+///
+/// ModuleDependencyScanner should only be used via ScanningAllProjectModules.
+///
+/// The ModuleDependencyScanner can get the directly required module name for a
+/// specific source file. Also the ModuleDependencyScanner can get the source
+/// file declaring a specific module name.
+///
+/// IMPORTANT NOTE: we assume that every module unit is only declared once in a
+/// source file in the project. But the assumption is not strictly true even
+/// besides the invalid projects. The language specification requires that
every
+/// module unit should be unique in a valid program. But a project can contain
+/// multiple programs. Then it is valid that we can have multiple source files
+/// declaring the same module in a project as long as these source files don't
+/// interere with each other.`
+class ModuleDependencyScanner {
+public:
+ ModuleDependencyScanner(const GlobalCompilationDatabase ,
+ const ThreadsafeFS )
+ : CDB(CDB), TFS(TFS),
+Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing,
+tooling::dependencies::ScanningOutputFormat::P1689) {}
+
+ // The scanned modules dependency information for a specific source file.
+ struct ModuleDependencyInfo {
+// The name of the module if the file is a module unit.
+std::optional ModuleName;
+// A list of names for the modules that the file directly depends.
+std::vector RequiredModules;
+ };
+
+ /// Scanning the single file specified by \param FilePath.
+ /// NOTE: This is only used by unittests for external uses.
ChuanqiXu9 wrote:
I just thought it will be better if it has less interfaces. Then currently
`scan()` is not directly called outside the class so I add the note. But I see
your thoughts about ModuleDependencyScanner to treat it as a self-contained
class. I feel it good too. So I'd like to remove the note.
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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
+/// the comments of `globalScan` to see the details.
+///
+/// ModuleDependencyScanner should only be used via ScanningAllProjectModules.
+///
+/// The ModuleDependencyScanner can get the directly required module name for a
+/// specific source file. Also the ModuleDependencyScanner can get the source
+/// file declaring a specific module name.
+///
+/// IMPORTANT NOTE: we assume that every module unit is only declared once in a
+/// source file in the project. But the assumption is not strictly true even
+/// besides the invalid projects. The language specification requires that
every
+/// module unit should be unique in a valid program. But a project can contain
+/// multiple programs. Then it is valid that we can have multiple source files
+/// declaring the same module in a project as long as these source files don't
+/// interere with each other.`
+class ModuleDependencyScanner {
+public:
+ ModuleDependencyScanner(const GlobalCompilationDatabase ,
+ const ThreadsafeFS )
+ : CDB(CDB), TFS(TFS),
+Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing,
+tooling::dependencies::ScanningOutputFormat::P1689) {}
+
+ // The scanned modules dependency information for a specific source file.
+ struct ModuleDependencyInfo {
+// The name of the module if the file is a module unit.
+std::optional ModuleName;
+// A list of names for the modules that the file directly depends.
+std::vector RequiredModules;
+ };
+
+ /// Scanning the single file specified by \param FilePath.
+ /// NOTE: This is only used by unittests for external uses.
+ std::optional scan(PathRef FilePath);
+
+ /// Scanning every source file in the current project to get the
+ /// to map.
+ /// It looks unefficiency to scan the whole project especially for
+ /// every version of every file!
+ /// TODO: We should find an efficient method to get the
+ /// to map. We can make it either by providing
+ /// a global module dependency scanner to monitor every file. Or we
+ /// can simply require the build systems (or even if the end users)
+ /// to provide the map.
+ void globalScan(const std::vector );
ChuanqiXu9 wrote:
Done
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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
+/// the comments of `globalScan` to see the details.
+///
+/// ModuleDependencyScanner should only be used via ScanningAllProjectModules.
+///
+/// The ModuleDependencyScanner can get the directly required module name for a
+/// specific source file. Also the ModuleDependencyScanner can get the source
+/// file declaring a specific module name.
+///
+/// IMPORTANT NOTE: we assume that every module unit is only declared once in a
+/// source file in the project. But the assumption is not strictly true even
+/// besides the invalid projects. The language specification requires that
every
+/// module unit should be unique in a valid program. But a project can contain
+/// multiple programs. Then it is valid that we can have multiple source files
+/// declaring the same module in a project as long as these source files don't
+/// interere with each other.`
ChuanqiXu9 wrote:
Done
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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
+/// the comments of `globalScan` to see the details.
+///
+/// ModuleDependencyScanner should only be used via ScanningAllProjectModules.
+///
+/// The ModuleDependencyScanner can get the directly required module name for a
+/// specific source file. Also the ModuleDependencyScanner can get the source
+/// file declaring a specific module name.
+///
+/// IMPORTANT NOTE: we assume that every module unit is only declared once in a
+/// source file in the project. But the assumption is not strictly true even
+/// besides the invalid projects. The language specification requires that
every
+/// module unit should be unique in a valid program. But a project can contain
+/// multiple programs. Then it is valid that we can have multiple source files
+/// declaring the same module in a project as long as these source files don't
+/// interere with each other.`
+class ModuleDependencyScanner {
+public:
+ ModuleDependencyScanner(const GlobalCompilationDatabase ,
+ const ThreadsafeFS )
+ : CDB(CDB), TFS(TFS),
+Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing,
+tooling::dependencies::ScanningOutputFormat::P1689) {}
+
+ // The scanned modules dependency information for a specific source file.
ChuanqiXu9 wrote:
Done
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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,81 @@
+//=== ModuleDependencyScanner.cpp *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ModuleDependencyScanner.h"
+#include "support/Logger.h"
+
+namespace clang {
+namespace clangd {
+
+std::optional
+ModuleDependencyScanner::scan(PathRef FilePath) {
+ std::optional Cmd = CDB.getCompileCommand(FilePath);
+
+ if (!Cmd)
+return std::nullopt;
+
+ using namespace clang::tooling::dependencies;
+
+ llvm::SmallString<128> FilePathDir(FilePath);
+ llvm::sys::path::remove_filename(FilePathDir);
+ DependencyScanningTool ScanningTool(Service, TFS.view(FilePathDir));
+
+ llvm::Expected ScanningResult =
+ ScanningTool.getP1689ModuleDependencyFile(*Cmd, Cmd->Directory);
+
+ if (auto E = ScanningResult.takeError()) {
+log("Scanning modules dependencies for {0} failed: {1}", FilePath,
ChuanqiXu9 wrote:
Done
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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -740,6 +741,21 @@
DirectoryBasedGlobalCompilationDatabase::getProjectInfo(PathRef File) const {
return Res->PI;
}
+std::shared_ptr
+DirectoryBasedGlobalCompilationDatabase::getProjectModules(PathRef File) const
{
+ CDBLookupRequest Req;
+ Req.FileName = File;
+ Req.ShouldBroadcast = false;
+ Req.FreshTime = Req.FreshTimeMissing =
+ std::chrono::steady_clock::time_point::min();
+ auto Res = lookupCDB(Req);
+ if (!Res)
+return {};
+ return ProjectModules::create(
+ ProjectModules::ProjectModulesKind::ScanningAllFiles,
+ Res->CDB->getAllFiles(), *this, Opts.TFS);
ChuanqiXu9 wrote:
I added a FIXME for this since I am not sure can we do here. Should we pass
`Res->CDB` here? If we can't solve it in a simple manner. I suggest to address
this in future versions and not blocking the initial version.
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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
+/// the comments of `globalScan` to see the details.
+///
+/// ModuleDependencyScanner should only be used via ScanningAllProjectModules.
ChuanqiXu9 wrote:
Done
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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
+/// the comments of `globalScan` to see the details.
+///
+/// ModuleDependencyScanner should only be used via ScanningAllProjectModules.
+///
+/// The ModuleDependencyScanner can get the directly required module name for a
+/// specific source file. Also the ModuleDependencyScanner can get the source
+/// file declaring a specific module name.
+///
+/// IMPORTANT NOTE: we assume that every module unit is only declared once in a
+/// source file in the project. But the assumption is not strictly true even
+/// besides the invalid projects. The language specification requires that
every
+/// module unit should be unique in a valid program. But a project can contain
+/// multiple programs. Then it is valid that we can have multiple source files
+/// declaring the same module in a project as long as these source files don't
+/// interere with each other.`
+class ModuleDependencyScanner {
+public:
+ ModuleDependencyScanner(const GlobalCompilationDatabase ,
+ const ThreadsafeFS )
+ : CDB(CDB), TFS(TFS),
+Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing,
+tooling::dependencies::ScanningOutputFormat::P1689) {}
+
+ // The scanned modules dependency information for a specific source file.
+ struct ModuleDependencyInfo {
+// The name of the module if the file is a module unit.
+std::optional ModuleName;
+// A list of names for the modules that the file directly depends.
+std::vector RequiredModules;
+ };
+
+ /// Scanning the single file specified by \param FilePath.
+ /// NOTE: This is only used by unittests for external uses.
+ std::optional scan(PathRef FilePath);
+
+ /// Scanning every source file in the current project to get the
+ /// to map.
+ /// It looks unefficiency to scan the whole project especially for
+ /// every version of every file!
+ /// TODO: We should find an efficient method to get the
+ /// to map. We can make it either by providing
+ /// a global module dependency scanner to monitor every file. Or we
+ /// can simply require the build systems (or even if the end users)
ChuanqiXu9 wrote:
Done
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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
https://github.com/ChuanqiXu9 edited 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] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)
@@ -0,0 +1,106 @@
+//===-- ModuleDependencyScanner.h *-
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H
+
+#include "GlobalCompilationDatabase.h"
+#include "support/Path.h"
+#include "support/ThreadsafeFS.h"
+
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
+
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace clang {
+namespace clangd {
+
+/// A scanner to query the dependency information for C++20 Modules.
+///
+/// The scanner can scan a single file with `scan(PathRef)` member function
+/// or scan the whole project with `globalScan(PathRef)` member function. See
ChuanqiXu9 wrote:
Done
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-tools-extra] [libcxx] [lldb] [llvm] Add clang basic module directory (PR #93388)
ChuanqiXu9 wrote: BTW, I tried to split `Module` class into `ModuleBase`, `ClangModule` and `Cpp20Modules` (and HeaderUnits) classes to improve the readability. But it showed too hard and too many things get changes then I stopped. https://github.com/llvm/llvm-project/pull/93388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [libcxx] [clang][Modules] Remove unnecessary includes of `Module.h` (PR #93417)
@@ -159,7 +159,8 @@ class APINotesManager {
ArrayRef getCurrentModuleReaders() const {
bool HasPublic = CurrentModuleReaders[ReaderKind::Public];
bool HasPrivate = CurrentModuleReaders[ReaderKind::Private];
-assert((!HasPrivate || HasPublic) && "private module requires public
module");
+assert((!HasPrivate || HasPublic) &&
+ "private module requires public module");
ChuanqiXu9 wrote:
Please don't change untouched codes.
https://github.com/llvm/llvm-project/pull/93417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [libcxx] [lldb] [llvm] Add clang basic module directory (PR #93388)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/93388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [libcxx] [lldb] [llvm] Add clang basic module directory (PR #93388)
https://github.com/ChuanqiXu9 requested changes to this pull request. I don't like the PR since I don't feel it makes the code cleaner and it may make the downstream suffering backporting. https://github.com/llvm/llvm-project/pull/93388 ___ 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 3911fb6c6c746..242a8ad2e350b 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 13d788162817f..8ba4b38c420ab 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] [Coroutines] Allow [[clang::coro_wrapper]] for class (PR #93268)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/93268
Previously we allow `[[clang::coro_wrapper]]` to be marked with function to
allow it to not be checked by `[[clang::coro_return_type]]`.
But in our internal practice, there are classes can be return type of
coroutines and non-coroutines and there are too many uses. It is slightly hard
to mark every use with `[[clang::coro_wrapper]]`. So it is a sugar to allow we
mark the class as `[[clang::coro_wrapper]]`.
>From 2e05a2871ea5beb3159ab946c040cb836201181f Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Fri, 24 May 2024 11:02:02 +0800
Subject: [PATCH] [Coroutines] Allow [[clang::coro_wrapper]] for class
Previously we allow `[[clang::coro_wrapper]]` to be marked with function
to allow it to not be checked by `[[clang::coro_return_type]]`.
But in our internal practice, there are classes can be return type of
coroutines and non-coroutines and there are too many uses. It is
slightly hard to mark every use with `[[clang::coro_wrapper]]`. So it is
a sugar to allow we mark the class as `[[clang::coro_wrapper]]`.
---
clang/include/clang/Basic/Attr.td | 2 +-
clang/lib/Sema/SemaDecl.cpp | 3 ++-
...a-attribute-supported-attributes-list.test | 2 +-
.../SemaCXX/coro-return-type-and-wrapper.cpp | 19 +++
4 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index e59cdd369..7933fd5b38d87 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1192,7 +1192,7 @@ def CoroReturnType : InheritableAttr {
def CoroWrapper : InheritableAttr {
let Spellings = [Clang<"coro_wrapper">];
- let Subjects = SubjectList<[Function]>;
+ let Subjects = SubjectList<[Function, CXXRecord]>;
let LangOpts = [CPlusPlus];
let Documentation = [CoroReturnTypeAndWrapperDoc];
let SimpleHandler = 1;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 2a87b26f17a2b..9cae3a0022dc2 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -15998,7 +15998,8 @@ void Sema::CheckCoroutineWrapper(FunctionDecl *FD) {
// Allow some_promise_type::get_return_object().
if (CanBeGetReturnObject(FD) || CanBeGetReturnTypeOnAllocFailure(FD))
return;
- if (!FD->hasAttr())
+ if (!FD->hasAttr() &&
+ !RD->getUnderlyingDecl()->hasAttr())
Diag(FD->getLocation(), diag::err_coroutine_return_type) << RD;
}
diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test
b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
index 99732694f72a5..04dc2962ac04e 100644
--- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -62,7 +62,7 @@
// CHECK-NEXT: CoroLifetimeBound (SubjectMatchRule_record)
// CHECK-NEXT: CoroOnlyDestroyWhenComplete (SubjectMatchRule_record)
// CHECK-NEXT: CoroReturnType (SubjectMatchRule_record)
-// CHECK-NEXT: CoroWrapper (SubjectMatchRule_function)
+// CHECK-NEXT: CoroWrapper (SubjectMatchRule_function, SubjectMatchRule_record)
// CHECK-NEXT: DLLExport (SubjectMatchRule_function,
SubjectMatchRule_variable, SubjectMatchRule_record,
SubjectMatchRule_objc_interface)
// CHECK-NEXT: DLLImport (SubjectMatchRule_function,
SubjectMatchRule_variable, SubjectMatchRule_record,
SubjectMatchRule_objc_interface)
// CHECK-NEXT: Destructor (SubjectMatchRule_function)
diff --git a/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp
b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp
index b08e1c9c065a0..3c174f3ffaecf 100644
--- a/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp
+++ b/clang/test/SemaCXX/coro-return-type-and-wrapper.cpp
@@ -139,3 +139,22 @@ template<> class coroutine_traits {
} // namespace std
// expected-error@+1 {{neither a coroutine nor a coroutine wrapper}}
Task foo(int) { return Task{}; }
+
+// Testing that we can add a `[[clang::coro_wrapper]]` attribute to the class.
+class [[clang::coro_return_type]] [[clang::coro_wrapper]] WrappedTask{
+public:
+ struct promise_type {
+Task get_return_object() {
+ return {};
+}
+suspend_always initial_suspend();
+suspend_always final_suspend() noexcept;
+void unhandled_exception();
+ };
+};
+namespace std {
+template<> class coroutine_traits {
+using promise_type = WrappedTask::promise_type;
+};
+} // namespace std
+WrappedTask wrapped_class(int) { return WrappedTask{}; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Support `-x` for all languages in CL mode (PR #89772)
ChuanqiXu9 wrote: I think we can land this given it is approved and the CI is green https://github.com/llvm/llvm-project/pull/89772 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)
ChuanqiXu9 wrote: Oh, it looks like some bots are not happy. I send the PR at https://github.com/llvm/llvm-project/pull/93167 https://github.com/llvm/llvm-project/pull/89751 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)
ChuanqiXu9 wrote: I land it directly given it looks straightforward and trivial in https://github.com/llvm/llvm-project/commit/31f1590e4fb324c43dc36199587c453e27b6f6fa See the commit message if you're interested. https://github.com/llvm/llvm-project/pull/89751 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)
ChuanqiXu9 wrote:
It is pretty interesting that I can pass the test by:
```
$git diff -U10
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index 450ea8234371..003bfbf7138a 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -220,20 +220,21 @@ static void lowerAwaitSuspend(IRBuilder<> ,
CoroAwaitSuspendInst *CB,
}
coro::LowererBase LB(*Wrapper->getParent());
auto *ResumeAddr = LB.makeSubFnCall(NewCall, CoroSubFnInst::ResumeIndex,
&*Builder.GetInsertPoint());
LLVMContext = Builder.getContext();
FunctionType *ResumeTy = FunctionType::get(
Type::getVoidTy(Ctx), PointerType::getUnqual(Ctx), false);
auto *ResumeCall = Builder.CreateCall(ResumeTy, ResumeAddr, {NewCall});
+ResumeCall->setCallingConv(CallingConv::Fast);
// We can't insert the 'ret' instruction and adjust the cc until the
// function has been split, so remember this for later.
Shape.SymmetricTransfers.push_back(ResumeCall);
NewCall = ResumeCall;
}
CB->replaceAllUsesWith(NewCall);
CB->eraseFromParent();
```
The breaking only happens with optimizations in a workload.
https://github.com/llvm/llvm-project/pull/89751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [coro] Lower `llvm.coro.await.suspend.handle` to resume with tail call (PR #89751)
ChuanqiXu9 wrote: I received some reports for this patch breaking some codes. I am reproducing it. It looks like related to exceptions. https://github.com/llvm/llvm-project/pull/89751 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix self-capturing `__block` variables (PR #89475)
https://github.com/ChuanqiXu9 commented: Serialization related change looks trivial and good. But I feel better to leave the formal approval to CG part reviewers. https://github.com/llvm/llvm-project/pull/89475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] NFCI: use TemplateArgumentLoc for NTTP DefaultArgument (PR #92852)
@@ -1435,7 +1436,10 @@ class NonTypeTemplateParmDecl final
bool hasDefaultArgument() const { return DefaultArgument.isSet(); }
/// Retrieve the default argument, if any.
- Expr *getDefaultArgument() const { return DefaultArgument.get(); }
+ const TemplateArgumentLoc () const {
ChuanqiXu9 wrote:
Is it better to rename `getDefaultArgument` to `getDefaultArgumentLoc`?
https://github.com/llvm/llvm-project/pull/92852
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] NFCI: use TemplateArgumentLoc for NTTP DefaultArgument (PR #92852)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/92852 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] NFCI: use TemplateArgumentLoc for NTTP DefaultArgument (PR #92852)
https://github.com/ChuanqiXu9 commented: Can you try to explain the what the patch does more and describe the rationale? https://github.com/llvm/llvm-project/pull/92852 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] d316a0b - [NFC] Remove unused ASTWriter::getTypeID
Author: Chuanqi Xu
Date: 2024-05-20T13:36:46+08:00
New Revision: d316a0bd48ceb4a0ee851d729291a2cdcc8818eb
URL:
https://github.com/llvm/llvm-project/commit/d316a0bd48ceb4a0ee851d729291a2cdcc8818eb
DIFF:
https://github.com/llvm/llvm-project/commit/d316a0bd48ceb4a0ee851d729291a2cdcc8818eb.diff
LOG: [NFC] Remove unused ASTWriter::getTypeID
As the title suggests, the `ASTWriter:getTypeID` method is not used.
This patch removes it.
Added:
Modified:
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Serialization/ASTCommon.h
clang/lib/Serialization/ASTWriter.cpp
Removed:
diff --git a/clang/include/clang/Serialization/ASTWriter.h
b/clang/include/clang/Serialization/ASTWriter.h
index 6aa2796a41e0c..88192e439a3f0 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -715,9 +715,6 @@ class ASTWriter : public ASTDeserializationListener,
/// Force a type to be emitted and get its ID.
serialization::TypeID GetOrCreateTypeID(QualType T);
- /// Determine the type ID of an already-emitted type.
- serialization::TypeID getTypeID(QualType T) const;
-
/// Find the first local declaration of a given local redeclarable
/// decl.
const Decl *getFirstLocalDecl(const Decl *D);
diff --git a/clang/lib/Serialization/ASTCommon.h
b/clang/lib/Serialization/ASTCommon.h
index 296642e3674a4..0230908d3e052 100644
--- a/clang/lib/Serialization/ASTCommon.h
+++ b/clang/lib/Serialization/ASTCommon.h
@@ -46,30 +46,6 @@ enum DeclUpdateKind {
TypeIdx TypeIdxFromBuiltin(const BuiltinType *BT);
-template
-TypeID MakeTypeID(ASTContext , QualType T, IdxForTypeTy IdxForType) {
- if (T.isNull())
-return PREDEF_TYPE_NULL_ID;
-
- unsigned FastQuals = T.getLocalFastQualifiers();
- T.removeLocalFastQualifiers();
-
- if (T.hasLocalNonFastQualifiers())
-return IdxForType(T).asTypeID(FastQuals);
-
- assert(!T.hasLocalQualifiers());
-
- if (const BuiltinType *BT = dyn_cast(T.getTypePtr()))
-return TypeIdxFromBuiltin(BT).asTypeID(FastQuals);
-
- if (T == Context.AutoDeductTy)
-return TypeIdx(PREDEF_TYPE_AUTO_DEDUCT).asTypeID(FastQuals);
- if (T == Context.AutoRRefDeductTy)
-return TypeIdx(PREDEF_TYPE_AUTO_RREF_DEDUCT).asTypeID(FastQuals);
-
- return IdxForType(T).asTypeID(FastQuals);
-}
-
unsigned ComputeHash(Selector Sel);
/// Retrieve the "definitive" declaration that provides all of the
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 2a107e4c56a3a..1d6d96932ba2c 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -6074,6 +6074,31 @@ void ASTWriter::AddTypeRef(QualType T, RecordDataImpl
) {
Record.push_back(GetOrCreateTypeID(T));
}
+template
+static TypeID MakeTypeID(ASTContext , QualType T,
+ IdxForTypeTy IdxForType) {
+ if (T.isNull())
+return PREDEF_TYPE_NULL_ID;
+
+ unsigned FastQuals = T.getLocalFastQualifiers();
+ T.removeLocalFastQualifiers();
+
+ if (T.hasLocalNonFastQualifiers())
+return IdxForType(T).asTypeID(FastQuals);
+
+ assert(!T.hasLocalQualifiers());
+
+ if (const BuiltinType *BT = dyn_cast(T.getTypePtr()))
+return TypeIdxFromBuiltin(BT).asTypeID(FastQuals);
+
+ if (T == Context.AutoDeductTy)
+return TypeIdx(PREDEF_TYPE_AUTO_DEDUCT).asTypeID(FastQuals);
+ if (T == Context.AutoRRefDeductTy)
+return TypeIdx(PREDEF_TYPE_AUTO_RREF_DEDUCT).asTypeID(FastQuals);
+
+ return IdxForType(T).asTypeID(FastQuals);
+}
+
TypeID ASTWriter::GetOrCreateTypeID(QualType T) {
assert(Context);
return MakeTypeID(*Context, T, [&](QualType T) -> TypeIdx {
@@ -6097,19 +6122,6 @@ TypeID ASTWriter::GetOrCreateTypeID(QualType T) {
});
}
-TypeID ASTWriter::getTypeID(QualType T) const {
- assert(Context);
- return MakeTypeID(*Context, T, [&](QualType T) -> TypeIdx {
-if (T.isNull())
- return TypeIdx();
-assert(!T.getLocalFastQualifiers());
-
-TypeIdxMap::const_iterator I = TypeIdxs.find(T);
-assert(I != TypeIdxs.end() && "Type not emitted!");
-return I->second;
- });
-}
-
void ASTWriter::AddEmittedDeclRef(const Decl *D, RecordDataImpl ) {
if (!wasDeclEmitted(D))
return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Read the initializer for interesting static variables before consuming it (PR #92353)
https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/92353 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [serialization] no transitive decl change (PR #92083)
ChuanqiXu9 wrote: @jansvoboda11 @Bigcheese gentle ping https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Introduce `SemaCoroutine` (PR #92645)
ChuanqiXu9 wrote: While I am not against the idea about splitting Sema, the implementation detail makes me slightly concerning. What I had in mind is to split `SemaC`, `SemaCXX` (this can be derived class of `SemaCXX`), `SemaObjC` and other dialects (e.g., ACC or CUDA) out of `Sema`. Then big features of C++ can be member of `SemaCXX` as `SemaCoroutine`, `SemaConcept`... https://github.com/llvm/llvm-project/pull/92645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Read the initializer for interesting static variables before consuming it (PR #92353)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/92353
>From 699da64855f147708f153c30177a1d02a4e014f7 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 15 May 2024 12:37:16 +0800
Subject: [PATCH 1/2] [Serialization] Read the initializer for interesting
static variables before consuming it
Close https://github.com/llvm/llvm-project/issues/91418
Since we load the variable's initializers lazily, it'd be
problematic if the initializers dependent on each other. So here we try to load
the initializers of static variables to make sure they are passed to
code generator by order. If we read any thing interesting, we would
consume that before emitting the current declaration.
---
clang/lib/Serialization/ASTReaderDecl.cpp| 29 ++-
clang/test/Modules/pr91418.cppm | 67 +
clang/test/OpenMP/nvptx_lambda_capturing.cpp | 246 +--
3 files changed, 216 insertions(+), 126 deletions(-)
create mode 100644 clang/test/Modules/pr91418.cppm
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 0c647086e304a..a6254b70560c3 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4186,12 +4186,35 @@ void ASTReader::PassInterestingDeclsToConsumer() {
GetDecl(ID);
EagerlyDeserializedDecls.clear();
- while (!PotentiallyInterestingDecls.empty()) {
-Decl *D = PotentiallyInterestingDecls.front();
-PotentiallyInterestingDecls.pop_front();
+ auto ConsumingPotentialInterestingDecls = [this]() {
+while (!PotentiallyInterestingDecls.empty()) {
+ Decl *D = PotentiallyInterestingDecls.front();
+ PotentiallyInterestingDecls.pop_front();
+ if (isConsumerInterestedIn(D))
+PassInterestingDeclToConsumer(D);
+}
+ };
+ std::deque MaybeInterestingDecls =
+ std::move(PotentiallyInterestingDecls);
+ assert(PotentiallyInterestingDecls.empty());
+ while (!MaybeInterestingDecls.empty()) {
+Decl *D = MaybeInterestingDecls.front();
+MaybeInterestingDecls.pop_front();
+// Since we load the variable's initializers lazily, it'd be problematic
+// if the initializers dependent on each other. So here we try to load the
+// initializers of static variables to make sure they are passed to code
+// generator by order. If we read anything interesting, we would consume
+// that before emitting the current declaration.
+if (auto *VD = dyn_cast(D);
+VD && VD->isFileVarDecl() && !VD->isExternallyVisible())
+ VD->getInit();
+ConsumingPotentialInterestingDecls();
if (isConsumerInterestedIn(D))
PassInterestingDeclToConsumer(D);
}
+
+ // If we add any new potential interesting decl in the last call, consume it.
+ ConsumingPotentialInterestingDecls();
}
void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord ) {
diff --git a/clang/test/Modules/pr91418.cppm b/clang/test/Modules/pr91418.cppm
new file mode 100644
index 0..33fec992439d6
--- /dev/null
+++ b/clang/test/Modules/pr91418.cppm
@@ -0,0 +1,67 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 -x c++-header
%t/foo.h \
+// RUN: -emit-pch -o %t/foo.pch
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/use.cpp
-include-pch \
+// RUN: %t/foo.pch -emit-llvm -o - | FileCheck %t/use.cpp
+
+//--- foo.h
+#ifndef FOO_H
+#define FOO_H
+typedef float __m128 __attribute__((__vector_size__(16), __aligned__(16)));
+
+static __inline__ __m128 __attribute__((__always_inline__,
__min_vector_width__(128)))
+_mm_setr_ps(float __z, float __y, float __x, float __w)
+{
+ return __extension__ (__m128){ __z, __y, __x, __w };
+}
+
+typedef __m128 VR;
+
+inline VR MakeVR( float X, float Y, float Z, float W )
+{
+ return _mm_setr_ps( X, Y, Z, W );
+}
+
+extern "C" float sqrtf(float);
+
+namespace VectorSinConstantsSSE
+{
+ float a = (16 * sqrtf(0.225f));
+ VR A = MakeVR(a, a, a, a);
+ static const float b = (16 * sqrtf(0.225f));
+ static const VR B = MakeVR(b, b, b, b);
+}
+
+#endif // FOO_H
+
+//--- use.cpp
+#include "foo.h"
+float use() {
+return VectorSinConstantsSSE::A[0] + VectorSinConstantsSSE::A[1] +
+ VectorSinConstantsSSE::A[2] + VectorSinConstantsSSE::A[3] +
+ VectorSinConstantsSSE::B[0] + VectorSinConstantsSSE::B[1] +
+ VectorSinConstantsSSE::B[2] + VectorSinConstantsSSE::B[3];
+}
+
+// CHECK: define{{.*}}@__cxx_global_var_init(
+// CHECK: store{{.*}}[[a_RESULT:%[a-zA-Z0-9]+]], ptr
@_ZN21VectorSinConstantsSSE1aE
+
+// CHECK: define{{.*}}@__cxx_global_var_init.1(
+// CHECK: [[A_CALL:%[a-zA-Z0-9]+]] = call{{.*}}@_Z6MakeVR(
+// CHECK: store{{.*}}[[A_CALL]], ptr @_ZN21VectorSinConstantsSSE1AE
+
+// CHECK: define{{.*}}@__cxx_global_var_init.2(
+// CHECK: [[B_CALL:%[a-zA-Z0-9]+]] = call{{.*}}@_Z6MakeVR(
+// CHECK: store{{.*}}[[B_CALL]], ptr @_ZN21VectorSinConstantsSSEL1BE
+
[clang] [Serialization] Read the initializer for interesting static variables before consuming it (PR #92353)
ChuanqiXu9 wrote:
> I can reproduce the failure. The problem is that the CHECK line
>
> ```
> // CHECK: [[A_CALL:%[a-zA-Z0-9]+]] = call{{.*}}@_Z6MakeVR(
> ```
>
> assumes that a value is returned. On SystemZ, the return value is passed as
> `sret` argument, and the function itself returns `void`, so the pattern does
> not match.
Oh, thanks! I never heard that before. I guess I don't need to check the call
since what I need to check is the order the variable get initialized.
I'll try to land this after the build bot gets green.
https://github.com/llvm/llvm-project/pull/92353
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
