[llvm-branch-commits] [clang] [Serialization] No transitive identifier change (PR #92085)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

@jansvoboda11 ping. Did my answers resolve your concerns?

https://github.com/llvm/llvm-project/pull/92085
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/92511

>From 57cfb2b7791666022ee46201b5126ac610c19bdd Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Fri, 17 May 2024 14:25:53 +0800
Subject: [PATCH] [serialization] No transitive type change

---
 .../include/clang/Serialization/ASTBitCodes.h |  32 --
 clang/include/clang/Serialization/ASTReader.h |  23 ++--
 .../clang/Serialization/ASTRecordReader.h |   2 +-
 .../include/clang/Serialization/ModuleFile.h  |   3 -
 clang/lib/Serialization/ASTReader.cpp | 104 +-
 clang/lib/Serialization/ASTWriter.cpp |  31 +++---
 clang/lib/Serialization/ModuleFile.cpp|   1 -
 .../Modules/no-transitive-decls-change.cppm   |  12 +-
 .../no-transitive-identifier-change.cppm  |   3 -
 .../Modules/no-transitive-type-change.cppm|  68 
 clang/test/Modules/pr5.cppm   |  36 +++---
 11 files changed, 196 insertions(+), 119 deletions(-)
 create mode 100644 clang/test/Modules/no-transitive-type-change.cppm

diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index 03bac9be2bf3d..13ee1e4724f1e 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -26,6 +26,7 @@
 #include "clang/Serialization/SourceLocationEncoding.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/Bitstream/BitCodes.h"
+#include "llvm/Support/MathExtras.h"
 #include 
 #include 
 
@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict
-/// qualifiers (as with QualType) and the upper bits provide a
-/// type index. The type index values are partitioned into two
+/// qualifiers (as with QualType).
+/// - the upper 29 bits provide a type index in the corresponding
+/// module file.
+/// - the upper 32 bits provide a module file index.
+///
+/// The type index values are partitioned into two
 /// sets. The values below NUM_PREDEF_TYPE_IDs are predefined type
 /// IDs (based on the PREDEF_TYPE_*_ID constants), with 0 as a
 /// placeholder for "no type". Values from NUM_PREDEF_TYPE_IDs are
 /// other types that have serialized representations.
-using TypeID = uint32_t;
+using TypeID = uint64_t;
 
 /// A type index; the type ID with the qualifier bits removed.
+/// Keep structure alignment 32-bit since the blob is assumed as 32-bit
+/// aligned.
 class TypeIdx {
+  uint32_t ModuleFileIndex = 0;
   uint32_t Idx = 0;
 
 public:
   TypeIdx() = default;
-  explicit TypeIdx(uint32_t index) : Idx(index) {}
+  explicit TypeIdx(uint32_t Idx) : ModuleFileIndex(0), Idx(Idx) {}
+
+  explicit TypeIdx(uint32_t ModuleFileIdx, uint32_t Idx)
+  : ModuleFileIndex(ModuleFileIdx), Idx(Idx) {}
+
+  uint32_t getModuleFileIndex() const { return ModuleFileIndex; }
 
-  uint32_t getIndex() const { return Idx; }
+  uint64_t getValue() const { return ((uint64_t)ModuleFileIndex << 32) | Idx; }
 
   TypeID asTypeID(unsigned FastQuals) const {
 if (Idx == uint32_t(-1))
   return TypeID(-1);
 
-return (Idx << Qualifiers::FastWidth) | FastQuals;
+unsigned Index = (Idx << Qualifiers::FastWidth) | FastQuals;
+return ((uint64_t)ModuleFileIndex << 32) | Index;
   }
 
   static TypeIdx fromTypeID(TypeID ID) {
 if (ID == TypeID(-1))
   return TypeIdx(-1);
 
-return TypeIdx(ID >> Qualifiers::FastWidth);
+return TypeIdx(ID >> 32, (ID & llvm::maskTrailingOnes(32)) >>
+ Qualifiers::FastWidth);
   }
 };
 
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 3f38a08f0da3a..f4180984eaad3 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -490,14 +490,6 @@ class ASTReader
   /// ID = (I + 1) << FastQual::Width has already been loaded
   llvm::PagedVector TypesLoaded;
 
-  using GlobalTypeMapType =
-  ContinuousRangeMap;
-
-  /// Mapping from global type IDs to the module in which the
-  /// type resides along with the offset that should be added to the
-  /// global type ID to produce a local ID.
-  GlobalTypeMapType GlobalTypeMap;
-
   /// Declarations that have already been loaded from the chain.
   ///
   /// When the pointer at index I is non-NULL, the declaration with ID
@@ -1423,8 +1415,8 @@ class ASTReader
 RecordLocation(ModuleFile *M, uint64_t O) : F(M), Offset(O) {}
   };
 
-  QualType readTypeRecord(unsigned Index);
-  RecordLocation TypeCursorForIndex(unsigned Index);
+  QualType readTypeRecord(serialization::TypeID ID);
+  RecordLocation TypeCursorForIndex(serialization::TypeID ID);
   void LoadedDecl(unsigned Index, Decl *D);
   Decl *ReadDeclRecord(GlobalDeclID

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

@jansvoboda11 @ilya-biryukov would you like to take look?

https://github.com/llvm/llvm-project/pull/92511
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

> Thanks for the change! I have done a round of review and left a few 
> suggestion and also have a bunch of questions. I am sorry if some of them may 
> look too obvious, I did try to dig into the code where I could, but Clang's 
> serialization is complicated and some things are easier researched through a 
> conversation than through looking at code. Please bear with me, I promise to 
> get better in the upcoming reviews.

You're welcome.

> 
> Here are a few extra question that came to mind too.
> 
> Question 1: Do we have estimates on how much bigger the PCMs become? Did you 
> observer any negative performance impact?
> 
> I would expect `TypeID`s to be much more common than decls, and the 
> corresponding size increases to be more significant. We've already lost ~6% 
> from DeclID change, I am slightly worried the types are going to be a bigger 
> hit.

I did and my local results shows, I see less than 1% change with this patch. 
This fits my understanding somehow. Since the `TypeID` is much less common than 
`DeclID`. This matches my experience in coding. The `DeclID` patch is the 
hardest one and the `TypeID` patch is the easiest one.

> 
> Question 2: could you explain why we the PCM in your example was changing 
> before? Was there some base offset inherited from the PCM files we depended 
> on?

Yes. In the higher level, it should be easy to understand. There is a new type 
in `m-partA.v1.cppm`. And in the low level, when we write a module file, we 
would record the type offset for the module files we depend on: 
https://github.com/llvm/llvm-project/blob/ad79a14c9e5ec4a369eed4adf567c22cc029863f/clang/lib/Serialization/ASTWriter.cpp#L5454

And this is exactly what the series patch want to eliminate.

https://github.com/llvm/llvm-project/pull/92511
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -7100,14 +7084,25 @@ TypeSourceInfo *ASTRecordReader::readTypeSourceInfo() {
   return TInfo;
 }
 
+std::pair
+ASTReader::translateTypeIDToIndex(serialization::TypeID ID) const {
+  unsigned Index =
+  (ID & llvm::maskTrailingOnes(32)) >> Qualifiers::FastWidth;
+
+  ModuleFile *OwningModuleFile = getOwningModuleFile(ID);
+  assert(OwningModuleFile &&
+ "untranslated type ID or local type ID shouldn't be in TypesLoaded");
+  return {OwningModuleFile, OwningModuleFile->BaseTypeIndex + Index};
+}
+
 QualType ASTReader::GetType(TypeID ID) {
   assert(ContextObj && "reading type with no AST context");
   ASTContext &Context = *ContextObj;
 
   unsigned FastQuals = ID & Qualifiers::FastMask;
-  unsigned Index = ID >> Qualifiers::FastWidth;
 
-  if (Index < NUM_PREDEF_TYPE_IDS) {
+  if (uint64_t Index = ID >> Qualifiers::FastWidth;

ChuanqiXu9 wrote:

> Are the higher bits of predefined types always 0?

Yes. Technically, the predefined ones (including decls, identifiers and types) 
doesn't belong to any modules. So their module file index  is always 0.

I will try to update a comment for this.

> (If this was done on top of results translateTypeIDToIndex, it would be very 
> clear that the code is correct, not sure if there are reasons to postpone 
> this call).

The `Index` in `translateTypeIDToIndex` means index in the 
`ASTReader::TypesLoaded` array. And these predefined ones, they are predefined, 
not loaded, shouldn't be in   `ASTReader::TypesLoaded`. Then I call 
`translateTypeIDToIndex` after dealing with the predefined ones.

https://github.com/llvm/llvm-project/pull/92511
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict
-/// qualifiers (as with QualType) and the upper bits provide a
-/// type index. The type index values are partitioned into two
+/// qualifiers (as with QualType).
+/// - the upper 29 bits provide a type index in the corresponding
+/// module file.
+/// - the upper 32 bits provide a module file index.
+///
+/// The type index values are partitioned into two
 /// sets. The values below NUM_PREDEF_TYPE_IDs are predefined type
 /// IDs (based on the PREDEF_TYPE_*_ID constants), with 0 as a
 /// placeholder for "no type". Values from NUM_PREDEF_TYPE_IDs are
 /// other types that have serialized representations.
-using TypeID = uint32_t;
+using TypeID = uint64_t;
 
 /// A type index; the type ID with the qualifier bits removed.
+/// Keep structure alignment 32-bit since the blob is assumed as 32-bit
+/// aligned.
 class TypeIdx {
+  uint32_t ModuleFileIndex = 0;
   uint32_t Idx = 0;
 
 public:
   TypeIdx() = default;
-  explicit TypeIdx(uint32_t index) : Idx(index) {}
+  explicit TypeIdx(uint32_t Idx) : ModuleFileIndex(0), Idx(Idx) {}

ChuanqiXu9 wrote:

Done.

https://github.com/llvm/llvm-project/pull/92511
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -6659,13 +6655,22 @@ void ASTWriter::MacroRead(serialization::MacroID ID, 
MacroInfo *MI) {
 }
 
 void ASTWriter::TypeRead(TypeIdx Idx, QualType T) {
-  // Always take the highest-numbered type index. This copes with an 
interesting
+  // Always take the type index that comes in later module files.
+  // This copes with an interesting
   // case for chained AST writing where we schedule writing the type and then,
   // later, deserialize the type from another AST. In this case, we want to
-  // keep the higher-numbered entry so that we can properly write it out to
+  // keep the just writing entry so that we can properly write it out to
   // the AST file.
   TypeIdx &StoredIdx = TypeIdxs[T];
-  if (Idx.getIndex() >= StoredIdx.getIndex())
+
+  // Ignore it if the type comes from the current being written module file.

ChuanqiXu9 wrote:

Since the current being written module file is the latest module. The reason 
why we don't need it previously is that, 
previously the type ID of the current being written module file is always 
larger than types from other module files.

For example, if A import B and B has 30 types, then previously, the type ID for 
the first type in A will be 31 + `NUM_PREDEFINED_TYPES`. This is also one of 
the motivation of the series patch. We don't hope the encoding of the current 
module file being affected by imported modules as much as possible.

So now, for the above example, the first type ID in A will be `<0, 1 + 
NUM_PREDEFINED_TYPES>` and type ID in B may be `<1, [0, 30]>`. So we can't 
compare them easily.

>  Do we have tests for it?

Yes, if we remove this line, there are failing tests.

https://github.com/llvm/llvm-project/pull/92511
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -6659,13 +6655,22 @@ void ASTWriter::MacroRead(serialization::MacroID ID, 
MacroInfo *MI) {
 }
 
 void ASTWriter::TypeRead(TypeIdx Idx, QualType T) {
-  // Always take the highest-numbered type index. This copes with an 
interesting
+  // Always take the type index that comes in later module files.
+  // This copes with an interesting
   // case for chained AST writing where we schedule writing the type and then,
   // later, deserialize the type from another AST. In this case, we want to
-  // keep the higher-numbered entry so that we can properly write it out to
+  // keep the just writing entry so that we can properly write it out to
   // the AST file.
   TypeIdx &StoredIdx = TypeIdxs[T];
-  if (Idx.getIndex() >= StoredIdx.getIndex())
+
+  // Ignore it if the type comes from the current being written module file.
+  unsigned ModuleFileIndex = StoredIdx.getModuleFileIndex();
+  if (ModuleFileIndex == 0 && StoredIdx.getValue())
+return;
+
+  // Otherwise, keep the highest ID since the module file comes later has
+  // higher module file indexes.
+  if (Idx.getValue() >= StoredIdx.getValue())

ChuanqiXu9 wrote:

Yes, done.

https://github.com/llvm/llvm-project/pull/92511
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -12,44 +12,44 @@
 // RUN: -fmodule-file=Module=%t/Module.pcm -emit-llvm -o - | FileCheck 
%t/Object.cppm
 
 // Test again with reduced BMI.
-// RUN: rm -rf %t
-// RUN: mkdir -p %t
-// RUN: split-file %s %t
+// RUNX: rm -rf %t

ChuanqiXu9 wrote:

Oh, sorry. This is something left during debugging. I will remove it.

https://github.com/llvm/llvm-project/pull/92511
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -7392,27 +7388,28 @@ QualType ASTReader::GetType(TypeID ID) {
   return TypesLoaded[Index].withFastQualifiers(FastQuals);
 }
 
-QualType ASTReader::getLocalType(ModuleFile &F, unsigned LocalID) {
+QualType ASTReader::getLocalType(ModuleFile &F, TypeID LocalID) {

ChuanqiXu9 wrote:

> Could we clarify what LocalID means here in a comment somewhere?

In ASTReader.h, there is a single line comment: `Resolve a local type ID within 
a given AST file into a type.`. I add a new line to explain it as "A local type 
ID is only meaningful with the corresponding module file. See the 
implementation of getGlobalTypeID for details.".

> IIUC, the ModuleFileIndex there is an index into F.TransitiveImports rather 
> than into a "global" module manager.

Yes, this is correct for local ID. But I feel it might be odd to put this in 
the comment. I feel, it might be good to know, we can use 
`ASTReader::getGlobalTypeID` to convert a `` pair to a 
`GlobalTypeID`. And the concrete process is the implementation details to 
GlobalTypeID`.

> Previously, we had distinction between unsigned in parameters to this 
> function and TypeID as a return type.
Maybe we should keep this here and accept uint64_t to avoid confusing the 
common global module file index and a local file index?

I don't feel it better. `uint64_t` doesn't provide any type information. To be 
best, maybe we can do something like for `LocalDeclID` and `GlobalDeclID`. I 
only did that for DeclID since I feel the DeclID is the most complicated and 
the TypeID are much more simpler. I think we can this later if we feel it is 
necessary.

https://github.com/llvm/llvm-project/pull/92511
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/92511

>From 57cfb2b7791666022ee46201b5126ac610c19bdd Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Fri, 17 May 2024 14:25:53 +0800
Subject: [PATCH 1/2] [serialization] No transitive type change

---
 .../include/clang/Serialization/ASTBitCodes.h |  32 --
 clang/include/clang/Serialization/ASTReader.h |  23 ++--
 .../clang/Serialization/ASTRecordReader.h |   2 +-
 .../include/clang/Serialization/ModuleFile.h  |   3 -
 clang/lib/Serialization/ASTReader.cpp | 104 +-
 clang/lib/Serialization/ASTWriter.cpp |  31 +++---
 clang/lib/Serialization/ModuleFile.cpp|   1 -
 .../Modules/no-transitive-decls-change.cppm   |  12 +-
 .../no-transitive-identifier-change.cppm  |   3 -
 .../Modules/no-transitive-type-change.cppm|  68 
 clang/test/Modules/pr5.cppm   |  36 +++---
 11 files changed, 196 insertions(+), 119 deletions(-)
 create mode 100644 clang/test/Modules/no-transitive-type-change.cppm

diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index 03bac9be2bf3d..13ee1e4724f1e 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -26,6 +26,7 @@
 #include "clang/Serialization/SourceLocationEncoding.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/Bitstream/BitCodes.h"
+#include "llvm/Support/MathExtras.h"
 #include 
 #include 
 
@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict
-/// qualifiers (as with QualType) and the upper bits provide a
-/// type index. The type index values are partitioned into two
+/// qualifiers (as with QualType).
+/// - the upper 29 bits provide a type index in the corresponding
+/// module file.
+/// - the upper 32 bits provide a module file index.
+///
+/// The type index values are partitioned into two
 /// sets. The values below NUM_PREDEF_TYPE_IDs are predefined type
 /// IDs (based on the PREDEF_TYPE_*_ID constants), with 0 as a
 /// placeholder for "no type". Values from NUM_PREDEF_TYPE_IDs are
 /// other types that have serialized representations.
-using TypeID = uint32_t;
+using TypeID = uint64_t;
 
 /// A type index; the type ID with the qualifier bits removed.
+/// Keep structure alignment 32-bit since the blob is assumed as 32-bit
+/// aligned.
 class TypeIdx {
+  uint32_t ModuleFileIndex = 0;
   uint32_t Idx = 0;
 
 public:
   TypeIdx() = default;
-  explicit TypeIdx(uint32_t index) : Idx(index) {}
+  explicit TypeIdx(uint32_t Idx) : ModuleFileIndex(0), Idx(Idx) {}
+
+  explicit TypeIdx(uint32_t ModuleFileIdx, uint32_t Idx)
+  : ModuleFileIndex(ModuleFileIdx), Idx(Idx) {}
+
+  uint32_t getModuleFileIndex() const { return ModuleFileIndex; }
 
-  uint32_t getIndex() const { return Idx; }
+  uint64_t getValue() const { return ((uint64_t)ModuleFileIndex << 32) | Idx; }
 
   TypeID asTypeID(unsigned FastQuals) const {
 if (Idx == uint32_t(-1))
   return TypeID(-1);
 
-return (Idx << Qualifiers::FastWidth) | FastQuals;
+unsigned Index = (Idx << Qualifiers::FastWidth) | FastQuals;
+return ((uint64_t)ModuleFileIndex << 32) | Index;
   }
 
   static TypeIdx fromTypeID(TypeID ID) {
 if (ID == TypeID(-1))
   return TypeIdx(-1);
 
-return TypeIdx(ID >> Qualifiers::FastWidth);
+return TypeIdx(ID >> 32, (ID & llvm::maskTrailingOnes(32)) >>
+ Qualifiers::FastWidth);
   }
 };
 
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 3f38a08f0da3a..f4180984eaad3 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -490,14 +490,6 @@ class ASTReader
   /// ID = (I + 1) << FastQual::Width has already been loaded
   llvm::PagedVector TypesLoaded;
 
-  using GlobalTypeMapType =
-  ContinuousRangeMap;
-
-  /// Mapping from global type IDs to the module in which the
-  /// type resides along with the offset that should be added to the
-  /// global type ID to produce a local ID.
-  GlobalTypeMapType GlobalTypeMap;
-
   /// Declarations that have already been loaded from the chain.
   ///
   /// When the pointer at index I is non-NULL, the declaration with ID
@@ -1423,8 +1415,8 @@ class ASTReader
 RecordLocation(ModuleFile *M, uint64_t O) : F(M), Offset(O) {}
   };
 
-  QualType readTypeRecord(unsigned Index);
-  RecordLocation TypeCursorForIndex(unsigned Index);
+  QualType readTypeRecord(serialization::TypeID ID);
+  RecordLocation TypeCursorForIndex(serialization::TypeID ID);
   void LoadedDecl(unsigned Index, Decl *D);
   Decl *ReadDeclRecord(GlobalDe

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/92511

>From 57cfb2b7791666022ee46201b5126ac610c19bdd Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Fri, 17 May 2024 14:25:53 +0800
Subject: [PATCH 1/2] [serialization] No transitive type change

---
 .../include/clang/Serialization/ASTBitCodes.h |  32 --
 clang/include/clang/Serialization/ASTReader.h |  23 ++--
 .../clang/Serialization/ASTRecordReader.h |   2 +-
 .../include/clang/Serialization/ModuleFile.h  |   3 -
 clang/lib/Serialization/ASTReader.cpp | 104 +-
 clang/lib/Serialization/ASTWriter.cpp |  31 +++---
 clang/lib/Serialization/ModuleFile.cpp|   1 -
 .../Modules/no-transitive-decls-change.cppm   |  12 +-
 .../no-transitive-identifier-change.cppm  |   3 -
 .../Modules/no-transitive-type-change.cppm|  68 
 clang/test/Modules/pr5.cppm   |  36 +++---
 11 files changed, 196 insertions(+), 119 deletions(-)
 create mode 100644 clang/test/Modules/no-transitive-type-change.cppm

diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index 03bac9be2bf3d..13ee1e4724f1e 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -26,6 +26,7 @@
 #include "clang/Serialization/SourceLocationEncoding.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/Bitstream/BitCodes.h"
+#include "llvm/Support/MathExtras.h"
 #include 
 #include 
 
@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict
-/// qualifiers (as with QualType) and the upper bits provide a
-/// type index. The type index values are partitioned into two
+/// qualifiers (as with QualType).
+/// - the upper 29 bits provide a type index in the corresponding
+/// module file.
+/// - the upper 32 bits provide a module file index.
+///
+/// The type index values are partitioned into two
 /// sets. The values below NUM_PREDEF_TYPE_IDs are predefined type
 /// IDs (based on the PREDEF_TYPE_*_ID constants), with 0 as a
 /// placeholder for "no type". Values from NUM_PREDEF_TYPE_IDs are
 /// other types that have serialized representations.
-using TypeID = uint32_t;
+using TypeID = uint64_t;
 
 /// A type index; the type ID with the qualifier bits removed.
+/// Keep structure alignment 32-bit since the blob is assumed as 32-bit
+/// aligned.
 class TypeIdx {
+  uint32_t ModuleFileIndex = 0;
   uint32_t Idx = 0;
 
 public:
   TypeIdx() = default;
-  explicit TypeIdx(uint32_t index) : Idx(index) {}
+  explicit TypeIdx(uint32_t Idx) : ModuleFileIndex(0), Idx(Idx) {}
+
+  explicit TypeIdx(uint32_t ModuleFileIdx, uint32_t Idx)
+  : ModuleFileIndex(ModuleFileIdx), Idx(Idx) {}
+
+  uint32_t getModuleFileIndex() const { return ModuleFileIndex; }
 
-  uint32_t getIndex() const { return Idx; }
+  uint64_t getValue() const { return ((uint64_t)ModuleFileIndex << 32) | Idx; }
 
   TypeID asTypeID(unsigned FastQuals) const {
 if (Idx == uint32_t(-1))
   return TypeID(-1);
 
-return (Idx << Qualifiers::FastWidth) | FastQuals;
+unsigned Index = (Idx << Qualifiers::FastWidth) | FastQuals;
+return ((uint64_t)ModuleFileIndex << 32) | Index;
   }
 
   static TypeIdx fromTypeID(TypeID ID) {
 if (ID == TypeID(-1))
   return TypeIdx(-1);
 
-return TypeIdx(ID >> Qualifiers::FastWidth);
+return TypeIdx(ID >> 32, (ID & llvm::maskTrailingOnes(32)) >>
+ Qualifiers::FastWidth);
   }
 };
 
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 3f38a08f0da3a..f4180984eaad3 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -490,14 +490,6 @@ class ASTReader
   /// ID = (I + 1) << FastQual::Width has already been loaded
   llvm::PagedVector TypesLoaded;
 
-  using GlobalTypeMapType =
-  ContinuousRangeMap;
-
-  /// Mapping from global type IDs to the module in which the
-  /// type resides along with the offset that should be added to the
-  /// global type ID to produce a local ID.
-  GlobalTypeMapType GlobalTypeMap;
-
   /// Declarations that have already been loaded from the chain.
   ///
   /// When the pointer at index I is non-NULL, the declaration with ID
@@ -1423,8 +1415,8 @@ class ASTReader
 RecordLocation(ModuleFile *M, uint64_t O) : F(M), Offset(O) {}
   };
 
-  QualType readTypeRecord(unsigned Index);
-  RecordLocation TypeCursorForIndex(unsigned Index);
+  QualType readTypeRecord(serialization::TypeID ID);
+  RecordLocation TypeCursorForIndex(serialization::TypeID ID);
   void LoadedDecl(unsigned Index, Decl *D);
   Decl *ReadDeclRecord(GlobalDe

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -6659,13 +6655,22 @@ void ASTWriter::MacroRead(serialization::MacroID ID, 
MacroInfo *MI) {
 }
 
 void ASTWriter::TypeRead(TypeIdx Idx, QualType T) {
-  // Always take the highest-numbered type index. This copes with an 
interesting
+  // Always take the type index that comes in later module files.
+  // This copes with an interesting
   // case for chained AST writing where we schedule writing the type and then,
   // later, deserialize the type from another AST. In this case, we want to
-  // keep the higher-numbered entry so that we can properly write it out to
+  // keep the just writing entry so that we can properly write it out to

ChuanqiXu9 wrote:

Done

https://github.com/llvm/llvm-project/pull/92511
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -7650,6 +7647,16 @@ ModuleFile *ASTReader::getOwningModuleFile(GlobalDeclID 
ID) const {
   return &getModuleManager()[ModuleFileIndex - 1];
 }
 
+ModuleFile *ASTReader::getOwningModuleFile(TypeID ID) const {
+  if (ID < NUM_PREDEF_TYPE_IDS)
+return nullptr;
+
+  uint64_t ModuleFileIndex = ID >> 32;

ChuanqiXu9 wrote:

Done

https://github.com/llvm/llvm-project/pull/92511
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -3262,17 +3262,18 @@ void ASTWriter::WritePragmaDiagnosticMappings(const 
DiagnosticsEngine &Diag,
 /// Write the representation of a type to the AST stream.
 void ASTWriter::WriteType(QualType T) {
   TypeIdx &IdxRef = TypeIdxs[T];
-  if (IdxRef.getIndex() == 0) // we haven't seen this type before.
+  if (IdxRef.getValue() == 0) // we haven't seen this type before.

ChuanqiXu9 wrote:

I feel it might not be needed. Since the type `TypeIdx` shows the information 
that this is index from serialization to me.

https://github.com/llvm/llvm-project/pull/92511
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict

ChuanqiXu9 wrote:

Done

https://github.com/llvm/llvm-project/pull/92511
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict
-/// qualifiers (as with QualType) and the upper bits provide a
-/// type index. The type index values are partitioned into two
+/// qualifiers (as with QualType).
+/// - the upper 29 bits provide a type index in the corresponding

ChuanqiXu9 wrote:

Done

https://github.com/llvm/llvm-project/pull/92511
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [libc] [lld] [lldb] [llvm] [Serialization] Introduce OnDiskHashTable for specializations (PR #83233)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/83233

error: too big or took too long to generate
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

Sorry for bothering, I tried a new manner to update the patch but it shows I 
should better : (

https://github.com/llvm/llvm-project/pull/83237
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Introduce OnDiskHashTable for specializations (PR #83233)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/83233

>From 2bf5a6ca8bde003b7acf0a9ab7c6e69cc3109e02 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 28 Feb 2024 11:41:53 +0800
Subject: [PATCH] [Serialization] Introduce OnDiskHashTable for specializations

Following up for https://github.com/llvm/llvm-project/pull/83108

This follows the suggestion literally from
https://github.com/llvm/llvm-project/pull/76774#issuecomment-1951172457

which introduces OnDiskHashTable for specializations based on
D41416.

Note that I didn't polish this patch to reduce the diff from D41416
to it easier to review. I'll make the polishing patch later. So that we
can focus what we're doing in this patch and focus on the style in the
next patch.
---
 clang/include/clang/AST/ExternalASTSource.h   |  11 +
 .../clang/Sema/MultiplexExternalSemaSource.h  |   6 +
 .../include/clang/Serialization/ASTBitCodes.h |   9 +
 clang/include/clang/Serialization/ASTReader.h |  34 ++-
 clang/include/clang/Serialization/ASTWriter.h |  15 +
 clang/lib/AST/DeclTemplate.cpp|  17 ++
 clang/lib/AST/ExternalASTSource.cpp   |   5 +
 .../lib/Sema/MultiplexExternalSemaSource.cpp  |  12 +
 clang/lib/Serialization/ASTReader.cpp | 145 +-
 clang/lib/Serialization/ASTReaderDecl.cpp |  27 ++
 clang/lib/Serialization/ASTReaderInternals.h  | 124 +
 clang/lib/Serialization/ASTWriter.cpp | 174 +++-
 clang/lib/Serialization/ASTWriterDecl.cpp |  32 ++-
 clang/unittests/Serialization/CMakeLists.txt  |   1 +
 .../Serialization/LoadSpecLazilyTest.cpp  | 260 ++
 15 files changed, 859 insertions(+), 13 deletions(-)
 create mode 100644 clang/unittests/Serialization/LoadSpecLazilyTest.cpp

diff --git a/clang/include/clang/AST/ExternalASTSource.h 
b/clang/include/clang/AST/ExternalASTSource.h
index 385c32edbae0f..24c4490d160f3 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -150,6 +150,17 @@ class ExternalASTSource : public 
RefCountedBase {
   virtual bool
   FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name);
 
+  /// Load all the external specializations for the Decl \param D if \param
+  /// OnlyPartial is false. Otherwise, load all the external **partial**
+  /// specializations for the \param D.
+  virtual void LoadExternalSpecializations(const Decl *D, bool OnlyPartial);
+
+  /// Load all the specializations for the Decl \param D with the same template
+  /// args specified by \param TemplateArgs.
+  virtual void
+  LoadExternalSpecializations(const Decl *D,
+  ArrayRef TemplateArgs);
+
   /// Ensures that the table of all visible declarations inside this
   /// context is up to date.
   ///
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h 
b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
index 238fb398b7d12..f81b70daa4b3d 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -97,6 +97,12 @@ class MultiplexExternalSemaSource : public 
ExternalSemaSource {
   bool FindExternalVisibleDeclsByName(const DeclContext *DC,
   DeclarationName Name) override;
 
+  void LoadExternalSpecializations(const Decl *D, bool OnlyPartial) override;
+
+  void
+  LoadExternalSpecializations(const Decl *D,
+  ArrayRef TemplateArgs) 
override;
+
   /// Ensures that the table of all visible declarations inside this
   /// context is up to date.
   void completeVisibleDeclsMap(const DeclContext *DC) override;
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index 488994c05dc12..eefc0e5218d7d 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -721,6 +721,12 @@ enum ASTRecordTypes {
 
   /// Record code for \#pragma clang unsafe_buffer_usage begin/end
   PP_UNSAFE_BUFFER_USAGE = 69,
+
+  /// Record code for vtables to emit.
+  VTABLES_TO_EMIT = 70,
+
+  /// Record code for updated specialization
+  UPDATE_SPECIALIZATION = 71,
 };
 
 /// Record types used within a source manager block.
@@ -1484,6 +1490,9 @@ enum DeclCode {
   /// A HLSLBufferDecl record.
   DECL_HLSL_BUFFER,
 
+  // A decls specilization record.
+  DECL_SPECIALIZATIONS,
+
   /// An ImplicitConceptSpecializationDecl record.
   DECL_IMPLICIT_CONCEPT_SPECIALIZATION,
 
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 76e51ac7ab979..0356fd14d9fd6 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -340,6 +340,9 @@ class ASTIdentifierLookupTrait;
 /// The on-disk hash table(s) used for DeclContext name lookup.
 struct DeclContextLookupTable;
 
+/// The on-disk hash table(s) u

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/83237

>From f2e53e44eebab4720a1dbade24fcb14d698fb03f Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 28 Feb 2024 11:41:53 +0800
Subject: [PATCH] [Serialization] Code cleanups and polish 83233

---
 clang/include/clang/AST/DeclTemplate.h|  39 +-
 clang/include/clang/AST/ExternalASTSource.h   |   8 +-
 .../clang/Sema/MultiplexExternalSemaSource.h  |   4 +-
 .../include/clang/Serialization/ASTBitCodes.h |   2 +-
 clang/include/clang/Serialization/ASTReader.h |   4 +-
 clang/lib/AST/DeclTemplate.cpp|  85 ++--
 clang/lib/AST/ExternalASTSource.cpp   |  10 +-
 clang/lib/AST/ODRHash.cpp |  10 -
 .../lib/Sema/MultiplexExternalSemaSource.cpp  |  13 +-
 clang/lib/Serialization/ASTCommon.h   |   1 -
 clang/lib/Serialization/ASTReader.cpp |  42 +-
 clang/lib/Serialization/ASTReaderDecl.cpp |  76 +---
 clang/lib/Serialization/ASTReaderInternals.h  |   1 -
 clang/lib/Serialization/ASTWriter.cpp |  27 +-
 clang/lib/Serialization/ASTWriterDecl.cpp |  52 +--
 clang/lib/Serialization/CMakeLists.txt|   1 +
 .../Serialization/TemplateArgumentHasher.cpp  | 423 ++
 .../Serialization/TemplateArgumentHasher.h|  34 ++
 clang/test/Modules/cxx-templates.cpp  |   8 +-
 .../Modules/recursive-instantiations.cppm |  40 ++
 .../test/OpenMP/target_parallel_ast_print.cpp |   4 -
 clang/test/OpenMP/target_teams_ast_print.cpp  |   4 -
 clang/test/OpenMP/task_ast_print.cpp  |   4 -
 clang/test/OpenMP/teams_ast_print.cpp |   4 -
 24 files changed, 610 insertions(+), 286 deletions(-)
 create mode 100644 clang/lib/Serialization/TemplateArgumentHasher.cpp
 create mode 100644 clang/lib/Serialization/TemplateArgumentHasher.h
 create mode 100644 clang/test/Modules/recursive-instantiations.cppm

diff --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
index 44f840d297465..7406252363d22 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -256,9 +256,6 @@ class TemplateArgumentList final
   TemplateArgumentList(const TemplateArgumentList &) = delete;
   TemplateArgumentList &operator=(const TemplateArgumentList &) = delete;
 
-  /// Create hash for the given arguments.
-  static unsigned ComputeODRHash(ArrayRef Args);
-
   /// Create a new template argument list that copies the given set of
   /// template arguments.
   static TemplateArgumentList *CreateCopy(ASTContext &Context,
@@ -732,25 +729,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
   }
 
   void anchor() override;
-  struct LazySpecializationInfo {
-GlobalDeclID DeclID = GlobalDeclID();
-unsigned ODRHash = ~0U;
-bool IsPartial = false;
-LazySpecializationInfo(GlobalDeclID ID, unsigned Hash = ~0U,
-   bool Partial = false)
-: DeclID(ID), ODRHash(Hash), IsPartial(Partial) {}
-LazySpecializationInfo() {}
-bool operator<(const LazySpecializationInfo &Other) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo &Other) const {
-  assert((DeclID != Other.DeclID || ODRHash == Other.ODRHash) &&
- "Hashes differ!");
-  assert((DeclID != Other.DeclID || IsPartial == Other.IsPartial) &&
- "Both must be the same kinds!");
-  return DeclID == Other.DeclID;
-}
-  };
 
 protected:
   template  struct SpecEntryTraits {
@@ -794,16 +772,20 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 
   void loadLazySpecializationsImpl(bool OnlyPartial = false) const;
 
-  void loadLazySpecializationsImpl(llvm::ArrayRef Args,
+  bool loadLazySpecializationsImpl(llvm::ArrayRef Args,
TemplateParameterList *TPL = nullptr) const;
 
-  Decl *loadLazySpecializationImpl(LazySpecializationInfo &LazySpecInfo) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector &Specs,
  void *&InsertPos, ProfileArguments &&...ProfileArgs);
 
+  template 
+  typename SpecEntryTraits::DeclType *
+  findSpecializationLocally(llvm::FoldingSetVector &Specs,
+void *&InsertPos,
+ProfileArguments &&...ProfileArgs);
+
   template 
   void addSpecializationImpl(llvm::FoldingSetVector &Specs,
  EntryType *Entry, void *InsertPos);
@@ -819,13 +801,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 llvm::PointerIntPair
   InstantiatedFromMember;
 
-/// If non-null, points to an array of specializations (including
-/// partial specializations) known only by their external declaration IDs.
-///
-/// The first value in the array is the number of specializations/partial
-/// specializations that follow.
-LazySpecializationInfo *LazySpecializations = nullptr

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

@ilya-biryukov I've fixed the crash occured in the reproducer. The root reason 
is that, previously, I wouldn't load all the specializations for paritial 
specializations. But this is not safe. I think you can test it again.

https://github.com/llvm/llvm-project/pull/83237
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

@vgvassilev In the newest version, I introduced a new hasher 
TemplateArgumentHasher and use this new hasher to calculate the hash value for 
template arguments. We thought this may be harder. But I just realized, we 
don't have to provide a very precise results at first. Since we are only 
required to give the same hash value for the same template arguments, but not 
required to give different hash value for different template arguments. So 
technically, it will still be a correct implementation if we return the same 
value for all template arguments.

So I introduced a bail out mechanism in the new hasher: 
https://github.com/llvm/llvm-project/pull/83237/files#diff-211ba0dca7d2f4bd0555dcfe173edd4658b9b4e045c49cb851b47654891c1627R32-R33
 which will return the default value  when it encounters the cases it can't 
handle.

It is safer and it is easier to maintain. We don't need to worry about the 
changes in ODRHash breaks the LazySpecializations. And we can improve it step 
by step. Although the performance, **theoretically**, may not be good as your 
first patch, I don't feel this is a problem. We can improve it later.

https://github.com/llvm/llvm-project/pull/83237
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/83237

>From f2e53e44eebab4720a1dbade24fcb14d698fb03f Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 28 Feb 2024 11:41:53 +0800
Subject: [PATCH 1/2] [Serialization] Code cleanups and polish 83233

---
 clang/include/clang/AST/DeclTemplate.h|  39 +-
 clang/include/clang/AST/ExternalASTSource.h   |   8 +-
 .../clang/Sema/MultiplexExternalSemaSource.h  |   4 +-
 .../include/clang/Serialization/ASTBitCodes.h |   2 +-
 clang/include/clang/Serialization/ASTReader.h |   4 +-
 clang/lib/AST/DeclTemplate.cpp|  85 ++--
 clang/lib/AST/ExternalASTSource.cpp   |  10 +-
 clang/lib/AST/ODRHash.cpp |  10 -
 .../lib/Sema/MultiplexExternalSemaSource.cpp  |  13 +-
 clang/lib/Serialization/ASTCommon.h   |   1 -
 clang/lib/Serialization/ASTReader.cpp |  42 +-
 clang/lib/Serialization/ASTReaderDecl.cpp |  76 +---
 clang/lib/Serialization/ASTReaderInternals.h  |   1 -
 clang/lib/Serialization/ASTWriter.cpp |  27 +-
 clang/lib/Serialization/ASTWriterDecl.cpp |  52 +--
 clang/lib/Serialization/CMakeLists.txt|   1 +
 .../Serialization/TemplateArgumentHasher.cpp  | 423 ++
 .../Serialization/TemplateArgumentHasher.h|  34 ++
 clang/test/Modules/cxx-templates.cpp  |   8 +-
 .../Modules/recursive-instantiations.cppm |  40 ++
 .../test/OpenMP/target_parallel_ast_print.cpp |   4 -
 clang/test/OpenMP/target_teams_ast_print.cpp  |   4 -
 clang/test/OpenMP/task_ast_print.cpp  |   4 -
 clang/test/OpenMP/teams_ast_print.cpp |   4 -
 24 files changed, 610 insertions(+), 286 deletions(-)
 create mode 100644 clang/lib/Serialization/TemplateArgumentHasher.cpp
 create mode 100644 clang/lib/Serialization/TemplateArgumentHasher.h
 create mode 100644 clang/test/Modules/recursive-instantiations.cppm

diff --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
index 44f840d297465..7406252363d22 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -256,9 +256,6 @@ class TemplateArgumentList final
   TemplateArgumentList(const TemplateArgumentList &) = delete;
   TemplateArgumentList &operator=(const TemplateArgumentList &) = delete;
 
-  /// Create hash for the given arguments.
-  static unsigned ComputeODRHash(ArrayRef Args);
-
   /// Create a new template argument list that copies the given set of
   /// template arguments.
   static TemplateArgumentList *CreateCopy(ASTContext &Context,
@@ -732,25 +729,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
   }
 
   void anchor() override;
-  struct LazySpecializationInfo {
-GlobalDeclID DeclID = GlobalDeclID();
-unsigned ODRHash = ~0U;
-bool IsPartial = false;
-LazySpecializationInfo(GlobalDeclID ID, unsigned Hash = ~0U,
-   bool Partial = false)
-: DeclID(ID), ODRHash(Hash), IsPartial(Partial) {}
-LazySpecializationInfo() {}
-bool operator<(const LazySpecializationInfo &Other) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo &Other) const {
-  assert((DeclID != Other.DeclID || ODRHash == Other.ODRHash) &&
- "Hashes differ!");
-  assert((DeclID != Other.DeclID || IsPartial == Other.IsPartial) &&
- "Both must be the same kinds!");
-  return DeclID == Other.DeclID;
-}
-  };
 
 protected:
   template  struct SpecEntryTraits {
@@ -794,16 +772,20 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 
   void loadLazySpecializationsImpl(bool OnlyPartial = false) const;
 
-  void loadLazySpecializationsImpl(llvm::ArrayRef Args,
+  bool loadLazySpecializationsImpl(llvm::ArrayRef Args,
TemplateParameterList *TPL = nullptr) const;
 
-  Decl *loadLazySpecializationImpl(LazySpecializationInfo &LazySpecInfo) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector &Specs,
  void *&InsertPos, ProfileArguments &&...ProfileArgs);
 
+  template 
+  typename SpecEntryTraits::DeclType *
+  findSpecializationLocally(llvm::FoldingSetVector &Specs,
+void *&InsertPos,
+ProfileArguments &&...ProfileArgs);
+
   template 
   void addSpecializationImpl(llvm::FoldingSetVector &Specs,
  EntryType *Entry, void *InsertPos);
@@ -819,13 +801,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 llvm::PointerIntPair
   InstantiatedFromMember;
 
-/// If non-null, points to an array of specializations (including
-/// partial specializations) known only by their external declaration IDs.
-///
-/// The first value in the array is the number of specializations/partial
-/// specializations that follow.
-LazySpecializationInfo *LazySpecializations = nul

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

> > > @ilya-biryukov, could we have another try of this PR on your end?
> > 
> > 
> > Sorry for missing this, we will rerun the testing and get back to you.
> 
> In the meantime, could you rebase the patch on top of current ToT?

Are you saying to rebase on your local branch? Since there is no conflicts in 
the trunk.



> I'm getting this error when trying to bootstrap Clang:
> 
> ```
> In file included from clang/lib/Serialization/ASTReaderDecl.cpp:15:
> clang/lib/Serialization/ASTReaderInternals.h:160:19: error: ISO C++20 
> considers use of overloaded operator '==' (with operand types 
> 'clang::serialization::reader::LazySpecializationInfo' and 
> 'LazySpecializationInfo') to be ambiguous despite there being a unique best 
> viable function [-Werror,-Wambiguous-reversed-operator]
>   160 | if (I == Info)
>   | ~ ^  
> clang/lib/Serialization/ASTReaderInternals.h:128:8: note: ambiguity is 
> between a regular call to this operator and a call with the argument order 
> reversed
>   128 |   bool operator==(const LazySpecializationInfo &Other) {
>   |^
> clang/lib/Serialization/ASTReaderInternals.h:128:8: note: mark 'operator==' 
> as const or add a matching 'operator!=' to resolve the ambiguity
> ```
> 
> I can mute it for testing, but this also needs to be fixed.

Oh, I know this pattern. This is potential breaking change in C++20. I didn't 
catch this since LLVM is C++17 based. Let's mute the warning in the test.

> clang/test/Modules/odr_hash.cpp fails with
> 
> ```
> error: 'expected-error' diagnostics seen but not expected: 
>   File /tmp/odr_hash.cpp/odr_hash.cpp.tmp/Inputs/second.h Line 5028: 
> 'DeclTemplateArguments::bar' has different definitions in different modules; 
> definition in module 'SecondModule' first difference is function body
> error: 'expected-note' diagnostics seen but not expected: 
>   File /tmp/odr_hash.cpp/odr_hash.cpp.tmp/Inputs/first.h Line 4144: but in 
> 'FirstModule' found a different body
> 2 errors generated.
> ```
> 
> coming from this invocation:
> 
> ```
> [ 28] // RUN: %clang_cc1 -triple x86_64-linux-gnu -x c++ -std=c++20 \
> [ 29] // RUN:   -fmodules -fimplicit-module-maps 
> -fmodules-cache-path=%t/cache \
> [ 30] // RUN:   -I%t/Inputs -verify %s
> ```

I've seen this issue in the past. It was about the order mismatch in the 
diagnostics since this patch changes the order to load specializations. But in 
the latest version, the mismatch disappeared. So it might be fine if the error 
you're seeing is only about the mismatches.

https://github.com/llvm/llvm-project/pull/83237
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

> We've hit an error during testing:
> 
> ```c++
> In module '...stl_cc_library':
> .../src/libcxx/include/__memory/allocator.h:128:60: error: 
> 'std::allocator>::deallocate' from 
> module '...stl_cc_library./cxx17/vector' is not present in definition of 
> 'std::allocator>' provided earlier
>   128 |   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void 
> deallocate(_Tp* __p, size_t __n) _NOEXCEPT {
>   |^
> .../src/libcxx/include/__memory/allocator.h:94:28: note: definition has no 
> member 'deallocate'
>94 | class _LIBCPP_TEMPLATE_VIS allocator : private 
> __non_trivial_if::value, allocator<_Tp> > {
>   |^
> ```
> 
> I will try to share a reproducer, but just like the last time, this might 
> take quite some time because it only shows up with code that builds many 
> modules before eventually producing this error.

Thank you very much!

https://github.com/llvm/llvm-project/pull/83237
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] release/19.x: [clang][driver][clang-cl] Support `--precompile` and `-fmodule-*` options in Clang-CL (#98761) (PR #102159)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

I feel this is good and no risks to bring in 19.x

https://github.com/llvm/llvm-project/pull/102159
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LLVM][Coroutines] Create `.noalloc` variant of switch ABI coroutine ramp functions during CoroSplit (PR #99283)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 edited 
https://github.com/llvm/llvm-project/pull/99283
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LLVM][Coroutines] Create `.noalloc` variant of switch ABI coroutine ramp functions during CoroSplit (PR #99283)

[Chuanqi Xu via llvm-branch-commits]

@@ -1455,6 +1462,62 @@ struct SwitchCoroutineSplitter {
 setCoroInfo(F, Shape, Clones);
   }
 
+  static Function *createNoAllocVariant(Function &F, coro::Shape &Shape,
+SmallVectorImpl &Clones) {
+auto *OrigFnTy = F.getFunctionType();

ChuanqiXu9 wrote:

If I am not mistaken, the identation looks not good here.

https://github.com/llvm/llvm-project/pull/99283
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LLVM][Coroutines] Create `.noalloc` variant of switch ABI coroutine ramp functions during CoroSplit (PR #99283)

[Chuanqi Xu via llvm-branch-commits]

@@ -1455,6 +1462,64 @@ struct SwitchCoroutineSplitter {
 setCoroInfo(F, Shape, Clones);
   }
 
+  static Function *createNoAllocVariant(Function &F, coro::Shape &Shape,
+SmallVectorImpl &Clones) {
+auto *OrigFnTy = F.getFunctionType();
+auto OldParams = OrigFnTy->params();
+
+SmallVector NewParams;
+NewParams.reserve(OldParams.size() + 1);
+for (Type *T : OldParams) {
+  NewParams.push_back(T);
+}
+NewParams.push_back(PointerType::getUnqual(Shape.FrameTy));
+
+auto *NewFnTy = FunctionType::get(OrigFnTy->getReturnType(), NewParams,
+  OrigFnTy->isVarArg());
+Function *NoAllocF =
+Function::Create(NewFnTy, F.getLinkage(), F.getName() + ".noalloc");

ChuanqiXu9 wrote:

I think this should be marked as internal like .resume and .destroy function 
does. While LTO can import them, I don't feel we can benefit a lot from that.

https://github.com/llvm/llvm-project/pull/99283
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LLVM][Coroutines] Create `.noalloc` variant of switch ABI coroutine ramp functions during CoroSplit (PR #99283)

[Chuanqi Xu via llvm-branch-commits]

@@ -63,6 +63,13 @@ suspend:
 ; CHECK-NOT: call void @free(
 ; CHECK: ret void
 
+; CHECK-LABEL: @f.noalloc({{.*}})

ChuanqiXu9 wrote:

I think it worth to list the arguments explicitly.

https://github.com/llvm/llvm-project/pull/99283
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LLVM][Coroutines] Create `.noalloc` variant of switch ABI coroutine ramp functions during CoroSplit (PR #99283)

[Chuanqi Xu via llvm-branch-commits]

@@ -1967,22 +2047,13 @@ splitCoroutine(Function &F, SmallVectorImpl 
&Clones,
   for (DbgVariableRecord *DVR : DbgVariableRecords)
 coro::salvageDebugInfo(ArgToAllocaMap, *DVR, Shape.OptimizeFrame,
false /*UseEntryValue*/);
-  return Shape;
-}
 
-/// Remove calls to llvm.coro.end in the original function.
-static void removeCoroEndsFromRampFunction(const coro::Shape &Shape) {
-  if (Shape.ABI != coro::ABI::Switch) {
-for (auto *End : Shape.CoroEnds) {
-  replaceCoroEnd(End, Shape, Shape.FramePtr, /*in resume*/ false, nullptr);
-}
-  } else {
-for (llvm::AnyCoroEndInst *End : Shape.CoroEnds) {
-  auto &Context = End->getContext();
-  End->replaceAllUsesWith(ConstantInt::getFalse(Context));
-  End->eraseFromParent();
-}
+  removeCoroEndsFromRampFunction(Shape);
+
+  if (!isNoSuspendCoroutine && Shape.ABI == coro::ABI::Switch) {

ChuanqiXu9 wrote:

Agreed.

https://github.com/llvm/llvm-project/pull/99283
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LLVM][Coroutines] Create `.noalloc` variant of switch ABI coroutine ramp functions during CoroSplit (PR #99283)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 commented:

We need to update coroutines.rst for such changes.

https://github.com/llvm/llvm-project/pull/99283
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LLVM][Coroutines] Create `.noalloc` variant of switch ABI coroutine ramp functions during CoroSplit (PR #99283)

[Chuanqi Xu via llvm-branch-commits]

@@ -5,7 +5,7 @@
 define nonnull ptr @f(i32 %n) presplitcoroutine {
 ; CHECK-LABEL: @f(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:[[ID:%.*]] = call token @llvm.coro.id(i32 0, ptr null, ptr 
null, ptr @f.resumers)
+; CHECK-NEXT:[[ID:%.*]] = call token @llvm.coro.id(i32 0, ptr null, ptr 
null, ptr @{{.*}})

ChuanqiXu9 wrote:

Are these changes to `@llvm.coro.id` related to this patch really?

https://github.com/llvm/llvm-project/pull/99283
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LLVM][Coroutines] Create `.noalloc` variant of switch ABI coroutine ramp functions during CoroSplit (PR #99283)

[Chuanqi Xu via llvm-branch-commits]

@@ -1455,6 +1462,62 @@ struct SwitchCoroutineSplitter {
 setCoroInfo(F, Shape, Clones);
   }
 
+  static Function *createNoAllocVariant(Function &F, coro::Shape &Shape,

ChuanqiXu9 wrote:

We need comment and an example for this function

https://github.com/llvm/llvm-project/pull/99283
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LLVM][Coroutines] Transform "coro_must_elide" calls to switch ABI coroutines to the `noalloc` variant (PR #99285)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 commented:

I did a quick scanning and I'll put the comment in 
https://github.com/llvm/llvm-project/pull/99283#pullrequestreview-822800

https://github.com/llvm/llvm-project/pull/99285
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LLVM][Coroutines] Transform "coro_must_elide" calls to switch ABI coroutines to the `noalloc` variant (PR #99285)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 edited 
https://github.com/llvm/llvm-project/pull/99285
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LLVM][Coroutines] Transform "coro_must_elide" calls to switch ABI coroutines to the `noalloc` variant (PR #99285)

[Chuanqi Xu via llvm-branch-commits]

@@ -0,0 +1,136 @@
+//===- CoroSplit.cpp - Converts a coroutine into a state machine 
--===//
+//
+// 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 "llvm/Transforms/Coroutines/CoroAnnotationElide.h"
+
+#include "llvm/Analysis/LazyCallGraph.h"
+#include "llvm/Analysis/OptimizationRemarkEmitter.h"
+#include "llvm/IR/Analysis.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/InstIterator.h"
+#include "llvm/IR/Instruction.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IR/PassManager.h"
+#include "llvm/Transforms/Utils/CallGraphUpdater.h"
+
+#include 
+
+using namespace llvm;
+
+#define DEBUG_TYPE "coro-annotation-elide"
+
+#define CORO_MUST_ELIDE_ANNOTATION "coro_must_elide"
+
+static Instruction *getFirstNonAllocaInTheEntryBlock(Function *F) {
+  for (Instruction &I : F->getEntryBlock())
+if (!isa(&I))
+  return &I;
+  llvm_unreachable("no terminator in the entry block");
+}
+
+static Value *allocateFrameInCaller(Function *Caller, uint64_t FrameSize,
+Align FrameAlign) {
+  LLVMContext &C = Caller->getContext();
+  BasicBlock::iterator InsertPt =
+  getFirstNonAllocaInTheEntryBlock(Caller)->getIterator();
+  const DataLayout &DL = Caller->getDataLayout();
+  auto FrameTy = ArrayType::get(Type::getInt8Ty(C), FrameSize);
+  auto *Frame = new AllocaInst(FrameTy, DL.getAllocaAddrSpace(), "", InsertPt);
+  Frame->setAlignment(FrameAlign);
+  return new BitCastInst(Frame, PointerType::getUnqual(C), "vFrame", InsertPt);
+}
+
+static void processCall(CallBase *CB, Function *Caller, Function *NewCallee,

ChuanqiXu9 wrote:

We really comments for the function we can't understand it by its name.

https://github.com/llvm/llvm-project/pull/99285
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LLVM][Coroutines] Create `.noalloc` variant of switch ABI coroutine ramp functions during CoroSplit (PR #99283)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

After I took a quick look at https://github.com/llvm/llvm-project/pull/99285, I 
feel this is not what I thought when I heard the idea. Correct me if I 
misunderstand it.

The problems are:
1. It looks like all the `.noalloc` variant are emitted all the time. This is 
absolutely not good. It emits duplicated code (especially currently its linkage 
is not internal). What I had in mind is to create `.noalloc` variant on need or 
lazily.
2. The original ramp function and the `.noalloc` variant shares a lot of codes 
if I am not mistaken. Then it is pretty bad for the code size. What I had in 
mind is, after we generate the `.noalloc` variant, we will rewrite the ramp 
function too and the ramp function will call the `.noalloc` function after it 
allocates the coroutine frame.

https://github.com/llvm/llvm-project/pull/99283
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LLVM][Coroutines] Create `.noalloc` variant of switch ABI coroutine ramp functions during CoroSplit (PR #99283)

[Chuanqi Xu via llvm-branch-commits]

@@ -1455,6 +1462,62 @@ struct SwitchCoroutineSplitter {
 setCoroInfo(F, Shape, Clones);
   }
 
+  static Function *createNoAllocVariant(Function &F, coro::Shape &Shape,
+SmallVectorImpl &Clones) {
+auto *OrigFnTy = F.getFunctionType();

ChuanqiXu9 wrote:

Never mind. I made a silly mistake.

https://github.com/llvm/llvm-project/pull/99283
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [llvm] [LLVM][Coroutines] Create `.noalloc` variant of switch ABI coroutine ramp functions during CoroSplit (PR #99283)

[Chuanqi Xu via llvm-branch-commits]

@@ -5,7 +5,7 @@
 define nonnull ptr @f(i32 %n) presplitcoroutine {
 ; CHECK-LABEL: @f(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:[[ID:%.*]] = call token @llvm.coro.id(i32 0, ptr null, ptr 
null, ptr @f.resumers)
+; CHECK-NEXT:[[ID:%.*]] = call token @llvm.coro.id(i32 0, ptr null, ptr 
null, ptr @{{.*}})

ChuanqiXu9 wrote:

I feel this is something need to be fixed. It looks odd to treat the  noalloc 
variant as resumers.

https://github.com/llvm/llvm-project/pull/99283
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] release/19.x: [C++20] [Modules] Don't diagnose duplicated implicit decl in multiple named modules (#102423) (PR #102425)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

I landed this directly as the owner of serialization. I feel this change is not 
riskful as it adds more conditions to generate a diagnose message we didn't do 
in 18.x and before. So nothing will be worse.

https://github.com/llvm/llvm-project/pull/102425
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] release/19.x: Reland [C++20] [Modules] [Itanium ABI] Generate the vtable in the mod… (#102287) (PR #102561)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

> @ChuanqiXu9 are we good to merge this? It seems like a pretty big patch but 
> reading the description it seems like some good fixes in there. Any risk 
> taking this on?

Yes, this is not small. And I am 100% sure it is fine. But given this is 
important, I still like to backport this. How about waiting 2 weeks to see if 
there is any regression reports? If no, I'll try to ping you again to merge 
this.

https://github.com/llvm/llvm-project/pull/102561
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] release/19.x: Reland [C++20] [Modules] [Itanium ABI] Generate the vtable in the mod… (#102287) (PR #102561)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

> > How about waiting 2 weeks to see if there is any regression reports? If no, 
> > I'll try to ping you again to merge this.
> 
> That seems counter-intuitive. Yes there are projects using LLVM main, but it 
> would appear much more likely to me to get regression reports from the 
> release candidates?

Yeah, but we may need to find a balance point otherwise we should backport all 
patches since that will make things get better tested but it might introduce 
worse user experience. And the balance point is not well defined and generally 
being introduced by feelings. Given my feelings for the issue reports of 
modules, I do feel a lot of them are using main branch.

https://github.com/llvm/llvm-project/pull/102561
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [clang][driver][clang-cl] Fix unused argument warning for `/std:c++20` for precompiled module inputs to `clang-cl` (PR #102438)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 milestoned 
https://github.com/llvm/llvm-project/pull/102438
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [clang][driver][clang-cl] Fix unused argument warning for `/std:c++20` for precompiled module inputs to `clang-cl` (PR #102438)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

> @ChuanqiXu9, is there anything else that needs to be done here? There's a 
> merge conflict; I could resolve that.

If there is merge conflict, we need to resolve it. For the merge request, we 
need to wait for the release manager to have a time to look at this. Maybe due 
to we forgot to mention this belongs to 19.x. CC @tru  manually.

https://github.com/llvm/llvm-project/pull/102438
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] release/18.x: [Serialization] Record whether the ODR is skipped (#82302) (PR #82309)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

Although we announced that the support for C++20 modules in clangd is broken, 
there are already a lot of people tried to use clangd with modules in some 
level. So it should be helpful to backport this to clang18 as well to improve 
the user experience as much as possible.

https://github.com/llvm/llvm-project/pull/82309
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Introduce OnDiskHashTable for specializations (PR #83233)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 created 
https://github.com/llvm/llvm-project/pull/83233

Following up for https://github.com/llvm/llvm-project/pull/83108

This follows the suggestion literally from
https://github.com/llvm/llvm-project/pull/76774#issuecomment-1951172457

which introduces OnDiskHashTable for specializations based on D41416.

Note that I didn't polish this patch to reduce the diff from D41416 to it 
easier to review. I'll make the polishing patch later. So that we can focus 
what we're doing in this patch and focus on the style in the next patch.

>From 9a88b07eaa2719d17db6c7e3548c897aac7da30d Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 28 Feb 2024 11:41:53 +0800
Subject: [PATCH] [Serialization] Introduce OnDiskHashTable for specializations

Following up for https://github.com/llvm/llvm-project/pull/83108

This follows the suggestion literally from
https://github.com/llvm/llvm-project/pull/76774#issuecomment-1951172457

which introduces OnDiskHashTable for specializations based on D41416.

Note that I didn't polish this patch to reduce the diff from D41416 to
it easier to review. I'll make the polishing patch later. So that we can
focus what we're doing in this patch and focus on the style in the next
patch.
---
 clang/include/clang/AST/ExternalASTSource.h   |  11 +
 .../clang/Sema/MultiplexExternalSemaSource.h  |   6 +
 .../include/clang/Serialization/ASTBitCodes.h |   5 +
 clang/include/clang/Serialization/ASTReader.h |  34 ++-
 clang/include/clang/Serialization/ASTWriter.h |  14 ++
 clang/lib/AST/DeclTemplate.cpp|  17 ++
 clang/lib/AST/ExternalASTSource.cpp   |   5 +
 .../lib/Sema/MultiplexExternalSemaSource.cpp  |  12 +
 clang/lib/Serialization/ASTReader.cpp | 146 +++-
 clang/lib/Serialization/ASTReaderDecl.cpp |  27 +++
 clang/lib/Serialization/ASTReaderInternals.h  | 121 ++
 clang/lib/Serialization/ASTWriter.cpp | 172 +-
 clang/lib/Serialization/ASTWriterDecl.cpp |  30 ++-
 clang/unittests/Serialization/CMakeLists.txt  |   1 +
 .../Serialization/LoadSpecLazilyTest.cpp  | 211 ++
 15 files changed, 800 insertions(+), 12 deletions(-)
 create mode 100644 clang/unittests/Serialization/LoadSpecLazilyTest.cpp

diff --git a/clang/include/clang/AST/ExternalASTSource.h 
b/clang/include/clang/AST/ExternalASTSource.h
index 8e573965b0a336..af476aa8c57824 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -150,6 +150,17 @@ class ExternalASTSource : public 
RefCountedBase {
   virtual bool
   FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name);
 
+  /// Load all the external specializations for the Decl \param D if \param
+  /// OnlyPartial is false. Otherwise, load all the external **partial**
+  /// specializations for the \param D.
+  virtual void LoadExternalSpecializations(const Decl *D, bool OnlyPartial);
+
+  /// Load all the specializations for the Decl \param D with the same template
+  /// args specified by \param TemplateArgs.
+  virtual void
+  LoadExternalSpecializations(const Decl *D,
+  ArrayRef TemplateArgs);
+
   /// Ensures that the table of all visible declarations inside this
   /// context is up to date.
   ///
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h 
b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
index 2bf91cb5212c5e..f09f037da0556a 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -97,6 +97,12 @@ class MultiplexExternalSemaSource : public 
ExternalSemaSource {
   bool FindExternalVisibleDeclsByName(const DeclContext *DC,
   DeclarationName Name) override;
 
+  void LoadExternalSpecializations(const Decl *D, bool OnlyPartial) override;
+
+  void
+  LoadExternalSpecializations(const Decl *D,
+  ArrayRef TemplateArgs) 
override;
+
   /// Ensures that the table of all visible declarations inside this
   /// context is up to date.
   void completeVisibleDeclsMap(const DeclContext *DC) override;
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index f31efa5117f0d1..15e7aef826a52a 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -698,6 +698,8 @@ enum ASTRecordTypes {
   /// Record code for an unterminated \#pragma clang assume_nonnull begin
   /// recorded in a preamble.
   PP_ASSUME_NONNULL_LOC = 67,
+
+  UPDATE_SPECIALIZATION = 68,
 };
 
 /// Record types used within a source manager block.
@@ -1523,6 +1525,9 @@ enum DeclCode {
   /// A HLSLBufferDecl record.
   DECL_HLSL_BUFFER,
 
+  // A decls specilization record.
+  DECL_SPECIALIZATIONS,
+
   /// An ImplicitConceptSpecializationDecl record.
   DECL_IMPLICIT_CONCEPT_SPECIALIZATION,
 
diff --git a/clang/inclu

[llvm-branch-commits] [clang] [Serialization] Introduce OnDiskHashTable for specializations (PR #83233)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 edited 
https://github.com/llvm/llvm-project/pull/83233
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Introduce OnDiskHashTable for specializations (PR #83233)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

> Can you rebase on top of #83108 ? That'd make it easier for me to review.

Weird. It should be on top of #83108 already.

https://github.com/llvm/llvm-project/pull/83233
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Introduce OnDiskHashTable for specializations (PR #83233)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

> > > Can you rebase on top of #83108 ? That'd make it easier for me to review.
> > 
> > 
> > Weird. It should be on top of #83108 already. 
> > https://github.com/llvm/llvm-project/commits/users/ChuanqiXu9/D41416_on_disk_hash_table/
> 
> Ah, it is a single commit that includes what's in the other PR?

Yes. This mimics the stacked review on phab. I assumed you're familiar with it.

https://github.com/llvm/llvm-project/pull/83233
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Introduce OnDiskHashTable for specializations (PR #83233)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

> If that's ready to give it a try on our infrastructure I can do it.

I'll try to update the polishing patch. While it might purely be removing 
codes, I think it may be better to test on that.

https://github.com/llvm/llvm-project/pull/83233
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 created 
https://github.com/llvm/llvm-project/pull/83237

This is the following of https://github.com/llvm/llvm-project/pull/83233. This 
simply removes  `LazySpecializationInfo` from `RedeclarableTemplateDecl`. I 
feel it can make the review process much more easier after splitting this.

>From ecdd0c35ea131b2cdc0943dfd2dfdcd59f973409 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 28 Feb 2024 16:51:15 +0800
Subject: [PATCH] [Serialization] Code cleanups and polish 83233

---
 clang/include/clang/AST/DeclTemplate.h| 32 +---
 .../include/clang/Serialization/ASTBitCodes.h |  2 +-
 clang/lib/AST/DeclTemplate.cpp| 49 
 clang/lib/Serialization/ASTCommon.h   |  1 -
 clang/lib/Serialization/ASTReader.cpp |  4 +-
 clang/lib/Serialization/ASTReaderDecl.cpp | 76 +--
 clang/lib/Serialization/ASTWriter.cpp | 26 +--
 clang/lib/Serialization/ASTWriterDecl.cpp | 52 +
 8 files changed, 30 insertions(+), 212 deletions(-)

diff --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
index 51caef54baac26..d25e10adb5453d 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -256,8 +256,8 @@ class TemplateArgumentList final
   TemplateArgumentList(const TemplateArgumentList &) = delete;
   TemplateArgumentList &operator=(const TemplateArgumentList &) = delete;
 
-  /// Create hash for the given arguments.
-  static unsigned ComputeODRHash(ArrayRef Args);
+  /// Create stable hash for the given arguments across compiler invocations.
+  static unsigned ComputeStableHash(ArrayRef Args);
 
   /// Create a new template argument list that copies the given set of
   /// template arguments.
@@ -733,25 +733,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
   }
 
   void anchor() override;
-  struct LazySpecializationInfo {
-uint32_t DeclID = ~0U;
-unsigned ODRHash = ~0U;
-bool IsPartial = false;
-LazySpecializationInfo(uint32_t ID, unsigned Hash = ~0U,
-   bool Partial = false)
-: DeclID(ID), ODRHash(Hash), IsPartial(Partial) {}
-LazySpecializationInfo() {}
-bool operator<(const LazySpecializationInfo &Other) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo &Other) const {
-  assert((DeclID != Other.DeclID || ODRHash == Other.ODRHash) &&
- "Hashes differ!");
-  assert((DeclID != Other.DeclID || IsPartial == Other.IsPartial) &&
- "Both must be the same kinds!");
-  return DeclID == Other.DeclID;
-}
-  };
 
 protected:
   template  struct SpecEntryTraits {
@@ -798,8 +779,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
   void loadLazySpecializationsImpl(llvm::ArrayRef Args,
TemplateParameterList *TPL = nullptr) const;
 
-  Decl *loadLazySpecializationImpl(LazySpecializationInfo &LazySpecInfo) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector &Specs,
@@ -820,13 +799,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 llvm::PointerIntPair
   InstantiatedFromMember;
 
-/// If non-null, points to an array of specializations (including
-/// partial specializations) known only by their external declaration IDs.
-///
-/// The first value in the array is the number of specializations/partial
-/// specializations that follow.
-LazySpecializationInfo *LazySpecializations = nullptr;
-
 /// The set of "injected" template arguments used within this
 /// template.
 ///
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index 15e7aef826a52a..827799ec7f0a3b 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -699,7 +699,7 @@ enum ASTRecordTypes {
   /// recorded in a preamble.
   PP_ASSUME_NONNULL_LOC = 67,
 
-  UPDATE_SPECIALIZATION = 68,
+  CXX_ADDED_TEMPLATE_SPECIALIZATION = 68,
 };
 
 /// Record types used within a source manager block.
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index 7b98b046d00725..4115f8ae4f0382 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -342,32 +342,6 @@ void RedeclarableTemplateDecl::loadLazySpecializationsImpl(
   ExternalSource->LoadExternalSpecializations(this->getCanonicalDecl(),
   OnlyPartial);
   return;
-
-  // Grab the most recent declaration to ensure we've loaded any lazy
-  // redeclarations of this template.
-  CommonBase *CommonBasePtr = getMostRecentDecl()->getCommonPtr();
-  if (auto *Specs = CommonBasePtr->LazySpecializations) {
-if (!OnlyPartial)
-  CommonBasePtr->LazySpecializations = nullptr;
-for (uint32_t I = 0, N = Specs[0]

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

@@ -257,22 +241,12 @@ namespace clang {
   // If we have any lazy specializations, and the external AST source is
   // our chained AST reader, we can just write out the DeclIDs. Otherwise,
   // we need to resolve them to actual declarations.
-  if (Writer.Chain != Writer.Context->getExternalSource() &&
-  Common->LazySpecializations) {
+  if (Writer.Chain != Writer.Context->getExternalSource() && Writer.Chain 
&&
+  Writer.Chain->getLoadedSpecializationsLookupTables(D)) {
 D->LoadLazySpecializations();
-assert(!Common->LazySpecializations);
+assert(!Writer.Chain->getLoadedSpecializationsLookupTables(D));
   }

ChuanqiXu9 wrote:

During the process of rewriting, I just realized that this may be a significant 
change between D41416 and my original patch. In my original patch, I feel this 
is redundant since I think 
https://github.com/llvm/llvm-project/pull/83233/files#diff-6fe53759e8d797c328c73ada5f3324c6248a8634ef36131c7eb2b9d89192bb64R4082-R4084
 can handle the specialization. But this is not true. The process of loading 
specializations has a strong side effect that it may trigger  more 
deserialization and probably loading other specializations. However, the 
process of re-inserting the OnDiskHashTable won't trigger deserialization. It 
simply moves stored hash value and the DeclID.

I guess this may be the problem why my original patch can't pass the workloads 
in google and ROOT. If it is the case, it shows that we lack in tree test cases 
for that.

https://github.com/llvm/llvm-project/pull/83237
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

@vgvassilev this may be ready to test.

https://github.com/llvm/llvm-project/pull/83237
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 edited 
https://github.com/llvm/llvm-project/pull/83237
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

The error message looks odd since the language options shouldn't be involved.

https://github.com/llvm/llvm-project/pull/83237
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

> > The error message looks odd since the language options shouldn't be 
> > involved.
> 
> Sorry, I did not push. Now you can take a look.

I mean your local results.

https://github.com/llvm/llvm-project/pull/83237
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Introduce OnDiskHashTable for specializations (PR #83233)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/83233

>From 03e7d56b79531d2e964d85e5bec52ccd6c6422e7 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 28 Feb 2024 11:41:53 +0800
Subject: [PATCH] [Serialization] Introduce OnDiskHashTable for specializations

Following up for https://github.com/llvm/llvm-project/pull/83108

This follows the suggestion literally from
https://github.com/llvm/llvm-project/pull/76774#issuecomment-1951172457

which introduces OnDiskHashTable for specializations based on D41416.

Note that I didn't polish this patch to reduce the diff from D41416 to
it easier to review. I'll make the polishing patch later. So that we can
focus what we're doing in this patch and focus on the style in the next
patch.
---
 clang/include/clang/AST/ExternalASTSource.h   |  11 +
 .../clang/Sema/MultiplexExternalSemaSource.h  |   6 +
 .../include/clang/Serialization/ASTBitCodes.h |   5 +
 clang/include/clang/Serialization/ASTReader.h |  34 ++-
 clang/include/clang/Serialization/ASTWriter.h |  13 +
 clang/lib/AST/DeclTemplate.cpp|  17 ++
 clang/lib/AST/ExternalASTSource.cpp   |   5 +
 .../lib/Sema/MultiplexExternalSemaSource.cpp  |  12 +
 clang/lib/Serialization/ASTReader.cpp | 145 +-
 clang/lib/Serialization/ASTReaderDecl.cpp |  27 ++
 clang/lib/Serialization/ASTReaderInternals.h  | 124 +
 clang/lib/Serialization/ASTWriter.cpp | 174 +++-
 clang/lib/Serialization/ASTWriterDecl.cpp |  32 ++-
 clang/unittests/Serialization/CMakeLists.txt  |   1 +
 .../Serialization/LoadSpecLazilyTest.cpp  | 260 ++
 15 files changed, 853 insertions(+), 13 deletions(-)
 create mode 100644 clang/unittests/Serialization/LoadSpecLazilyTest.cpp

diff --git a/clang/include/clang/AST/ExternalASTSource.h 
b/clang/include/clang/AST/ExternalASTSource.h
index 8e573965b0a336..af476aa8c57824 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -150,6 +150,17 @@ class ExternalASTSource : public 
RefCountedBase {
   virtual bool
   FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name);
 
+  /// Load all the external specializations for the Decl \param D if \param
+  /// OnlyPartial is false. Otherwise, load all the external **partial**
+  /// specializations for the \param D.
+  virtual void LoadExternalSpecializations(const Decl *D, bool OnlyPartial);
+
+  /// Load all the specializations for the Decl \param D with the same template
+  /// args specified by \param TemplateArgs.
+  virtual void
+  LoadExternalSpecializations(const Decl *D,
+  ArrayRef TemplateArgs);
+
   /// Ensures that the table of all visible declarations inside this
   /// context is up to date.
   ///
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h 
b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
index 2bf91cb5212c5e..f09f037da0556a 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -97,6 +97,12 @@ class MultiplexExternalSemaSource : public 
ExternalSemaSource {
   bool FindExternalVisibleDeclsByName(const DeclContext *DC,
   DeclarationName Name) override;
 
+  void LoadExternalSpecializations(const Decl *D, bool OnlyPartial) override;
+
+  void
+  LoadExternalSpecializations(const Decl *D,
+  ArrayRef TemplateArgs) 
override;
+
   /// Ensures that the table of all visible declarations inside this
   /// context is up to date.
   void completeVisibleDeclsMap(const DeclContext *DC) override;
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index f31efa5117f0d1..15e7aef826a52a 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -698,6 +698,8 @@ enum ASTRecordTypes {
   /// Record code for an unterminated \#pragma clang assume_nonnull begin
   /// recorded in a preamble.
   PP_ASSUME_NONNULL_LOC = 67,
+
+  UPDATE_SPECIALIZATION = 68,
 };
 
 /// Record types used within a source manager block.
@@ -1523,6 +1525,9 @@ enum DeclCode {
   /// A HLSLBufferDecl record.
   DECL_HLSL_BUFFER,
 
+  // A decls specilization record.
+  DECL_SPECIALIZATIONS,
+
   /// An ImplicitConceptSpecializationDecl record.
   DECL_IMPLICIT_CONCEPT_SPECIALIZATION,
 
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 2002bf23c9595f..f1edd2fae645d4 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -340,6 +340,9 @@ class ASTIdentifierLookupTrait;
 /// The on-disk hash table(s) used for DeclContext name lookup.
 struct DeclContextLookupTable;
 
+/// The on-disk hash table(s) used for specialization decls.
+struct LazySpecializationInfoLookupTable

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/83237

>From 19617bbdd5b83076140af087d3da0b46d4fe0208 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 28 Feb 2024 11:41:53 +0800
Subject: [PATCH] [Serialization] Code cleanups and polish 83233

---
 clang/include/clang/AST/DeclTemplate.h| 34 +---
 clang/include/clang/AST/ExternalASTSource.h   |  4 +-
 .../clang/Sema/MultiplexExternalSemaSource.h  |  2 +-
 .../include/clang/Serialization/ASTBitCodes.h |  2 +-
 clang/include/clang/Serialization/ASTReader.h |  2 +-
 clang/lib/AST/DeclTemplate.cpp| 85 +--
 clang/lib/AST/ExternalASTSource.cpp   |  4 +-
 .../lib/Sema/MultiplexExternalSemaSource.cpp  |  6 +-
 clang/lib/Serialization/ASTCommon.h   |  1 -
 clang/lib/Serialization/ASTReader.cpp | 17 ++--
 clang/lib/Serialization/ASTReaderDecl.cpp | 76 +
 clang/lib/Serialization/ASTReaderInternals.h  |  1 -
 clang/lib/Serialization/ASTWriter.cpp | 26 +-
 clang/lib/Serialization/ASTWriterDecl.cpp | 52 +---
 14 files changed, 78 insertions(+), 234 deletions(-)

diff --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
index 51caef54baac26..537fca1ab9dd77 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -256,8 +256,8 @@ class TemplateArgumentList final
   TemplateArgumentList(const TemplateArgumentList &) = delete;
   TemplateArgumentList &operator=(const TemplateArgumentList &) = delete;
 
-  /// Create hash for the given arguments.
-  static unsigned ComputeODRHash(ArrayRef Args);
+  /// Create stable hash for the given arguments across compiler invocations.
+  static unsigned ComputeStableHash(ArrayRef Args);
 
   /// Create a new template argument list that copies the given set of
   /// template arguments.
@@ -733,25 +733,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
   }
 
   void anchor() override;
-  struct LazySpecializationInfo {
-uint32_t DeclID = ~0U;
-unsigned ODRHash = ~0U;
-bool IsPartial = false;
-LazySpecializationInfo(uint32_t ID, unsigned Hash = ~0U,
-   bool Partial = false)
-: DeclID(ID), ODRHash(Hash), IsPartial(Partial) {}
-LazySpecializationInfo() {}
-bool operator<(const LazySpecializationInfo &Other) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo &Other) const {
-  assert((DeclID != Other.DeclID || ODRHash == Other.ODRHash) &&
- "Hashes differ!");
-  assert((DeclID != Other.DeclID || IsPartial == Other.IsPartial) &&
- "Both must be the same kinds!");
-  return DeclID == Other.DeclID;
-}
-  };
 
 protected:
   template  struct SpecEntryTraits {
@@ -795,11 +776,9 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 
   void loadLazySpecializationsImpl(bool OnlyPartial = false) const;
 
-  void loadLazySpecializationsImpl(llvm::ArrayRef Args,
+  bool loadLazySpecializationsImpl(llvm::ArrayRef Args,
TemplateParameterList *TPL = nullptr) const;
 
-  Decl *loadLazySpecializationImpl(LazySpecializationInfo &LazySpecInfo) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector &Specs,
@@ -820,13 +799,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 llvm::PointerIntPair
   InstantiatedFromMember;
 
-/// If non-null, points to an array of specializations (including
-/// partial specializations) known only by their external declaration IDs.
-///
-/// The first value in the array is the number of specializations/partial
-/// specializations that follow.
-LazySpecializationInfo *LazySpecializations = nullptr;
-
 /// The set of "injected" template arguments used within this
 /// template.
 ///
diff --git a/clang/include/clang/AST/ExternalASTSource.h 
b/clang/include/clang/AST/ExternalASTSource.h
index af476aa8c57824..769abf44ecd430 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -157,7 +157,9 @@ class ExternalASTSource : public 
RefCountedBase {
 
   /// Load all the specializations for the Decl \param D with the same template
   /// args specified by \param TemplateArgs.
-  virtual void
+  ///
+  /// Return true if any new specializations get loaded. Return false 
otherwise.
+  virtual bool
   LoadExternalSpecializations(const Decl *D,
   ArrayRef TemplateArgs);
 
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h 
b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
index f09f037da0556a..2ac8f606ae9574 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -99,7 +99,7 @@ class MultiplexExternalSemaSource : public ExternalSemaSource 

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/83237

>From 1d288e76b213a25d15fa6abdf633488838ed100a Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 28 Feb 2024 11:41:53 +0800
Subject: [PATCH] [Serialization] Code cleanups and polish 83233

---
 clang/include/clang/AST/DeclTemplate.h| 40 ++---
 clang/include/clang/AST/ExternalASTSource.h   |  4 +-
 .../clang/Sema/MultiplexExternalSemaSource.h  |  2 +-
 .../include/clang/Serialization/ASTBitCodes.h |  2 +-
 clang/include/clang/Serialization/ASTReader.h |  2 +-
 clang/lib/AST/DeclTemplate.cpp| 85 +--
 clang/lib/AST/ExternalASTSource.cpp   |  6 +-
 .../lib/Sema/MultiplexExternalSemaSource.cpp  |  7 +-
 clang/lib/Serialization/ASTCommon.h   |  1 -
 clang/lib/Serialization/ASTReader.cpp | 17 ++--
 clang/lib/Serialization/ASTReaderDecl.cpp | 76 +
 clang/lib/Serialization/ASTReaderInternals.h  |  1 -
 clang/lib/Serialization/ASTWriter.cpp | 26 +-
 clang/lib/Serialization/ASTWriterDecl.cpp | 52 +---
 14 files changed, 87 insertions(+), 234 deletions(-)

diff --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
index 51caef54baac26..870c0d1bdd4610 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -256,8 +256,8 @@ class TemplateArgumentList final
   TemplateArgumentList(const TemplateArgumentList &) = delete;
   TemplateArgumentList &operator=(const TemplateArgumentList &) = delete;
 
-  /// Create hash for the given arguments.
-  static unsigned ComputeODRHash(ArrayRef Args);
+  /// Create stable hash for the given arguments across compiler invocations.
+  static unsigned ComputeStableHash(ArrayRef Args);
 
   /// Create a new template argument list that copies the given set of
   /// template arguments.
@@ -733,25 +733,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
   }
 
   void anchor() override;
-  struct LazySpecializationInfo {
-uint32_t DeclID = ~0U;
-unsigned ODRHash = ~0U;
-bool IsPartial = false;
-LazySpecializationInfo(uint32_t ID, unsigned Hash = ~0U,
-   bool Partial = false)
-: DeclID(ID), ODRHash(Hash), IsPartial(Partial) {}
-LazySpecializationInfo() {}
-bool operator<(const LazySpecializationInfo &Other) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo &Other) const {
-  assert((DeclID != Other.DeclID || ODRHash == Other.ODRHash) &&
- "Hashes differ!");
-  assert((DeclID != Other.DeclID || IsPartial == Other.IsPartial) &&
- "Both must be the same kinds!");
-  return DeclID == Other.DeclID;
-}
-  };
 
 protected:
   template  struct SpecEntryTraits {
@@ -795,16 +776,20 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 
   void loadLazySpecializationsImpl(bool OnlyPartial = false) const;
 
-  void loadLazySpecializationsImpl(llvm::ArrayRef Args,
+  bool loadLazySpecializationsImpl(llvm::ArrayRef Args,
TemplateParameterList *TPL = nullptr) const;
 
-  Decl *loadLazySpecializationImpl(LazySpecializationInfo &LazySpecInfo) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector &Specs,
  void *&InsertPos, ProfileArguments &&...ProfileArgs);
 
+  template 
+  typename SpecEntryTraits::DeclType *
+  findSpecializationLocally(llvm::FoldingSetVector &Specs,
+void *&InsertPos,
+ProfileArguments &&...ProfileArgs);
+
   template 
   void addSpecializationImpl(llvm::FoldingSetVector &Specs,
  EntryType *Entry, void *InsertPos);
@@ -820,13 +805,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 llvm::PointerIntPair
   InstantiatedFromMember;
 
-/// If non-null, points to an array of specializations (including
-/// partial specializations) known only by their external declaration IDs.
-///
-/// The first value in the array is the number of specializations/partial
-/// specializations that follow.
-LazySpecializationInfo *LazySpecializations = nullptr;
-
 /// The set of "injected" template arguments used within this
 /// template.
 ///
diff --git a/clang/include/clang/AST/ExternalASTSource.h 
b/clang/include/clang/AST/ExternalASTSource.h
index af476aa8c57824..769abf44ecd430 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -157,7 +157,9 @@ class ExternalASTSource : public 
RefCountedBase {
 
   /// Load all the specializations for the Decl \param D with the same template
   /// args specified by \param TemplateArgs.
-  virtual void
+  ///
+  /// Return true if any new specializations get loaded. Return false 
otherwise.
+  virtual bool
   LoadExternalSpecializ

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

@ilya-biryukov hi, the functional and performance test on the root side looks 
good: https://github.com/root-project/root/pull/14495#issuecomment-1980589145 

Could you try to test this within google internals?

Also if your projects implements new ExternalASTSource, you need to handle that 
like I did in root: 
https://github.com/ChuanqiXu9/root/commit/570fd783875671d346c7cdc0d98a8a9afcad05a4

https://github.com/llvm/llvm-project/pull/83237
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] release/18.x: backport [C++20] [Moduls] Avoid computing odr hash for functions from comparing constraint expression (PR #84723)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 milestoned 
https://github.com/llvm/llvm-project/pull/84723
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] release/18.x: backport [C++20] [Moduls] Avoid computing odr hash for functions from comparing constraint expression (PR #84723)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 created 
https://github.com/llvm/llvm-project/pull/84723

Backport 3f6bc1adf805681293c2ef0b93b708ff52244c00

This fixes a surprising regression introduced in 
https://github.com/llvm/llvm-project/issues/79240. Given we've backported that 
to 18.x. We need to backport this fix to 18.x too.

>From 9139ca67d7afc1838562ea73d14d693c59c929ff Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Mon, 11 Mar 2024 11:14:40 +0800
Subject: [PATCH] [C++20] [Moduls] Avoid computing odr hash for functions from
 comparing constraint expression

Previously we disabled to compute ODR hash for declarations from the
global module fragment. However, we missed the case that the functions
lives in the concept requiments (see the attached the test files for
example). And the mismatch causes the potential crashment.

Due to we will set the function body as lazy after we deserialize it and
we will only take its body when needed. However, we don't allow to take
the body during deserializing. So it is actually potentially problematic
if we set the body as lazy first and computing the hash value of the
function, which requires to deserialize its body. So we will meet a
crash here.

This patch tries to solve the issue by not taking the body of the
function from GMF. Note that we can't skip comparing the constraint
expression from the GMF directly since it is an key part of the
function selecting and it may be the reason why we can't return 0
directly for `FunctionDecl::getODRHash()` from the GMF.
---
 clang/include/clang/AST/DeclBase.h| 10 +++
 clang/include/clang/Serialization/ASTReader.h |  7 --
 clang/lib/AST/Decl.cpp|  2 +-
 clang/lib/AST/DeclBase.cpp|  5 ++
 clang/lib/Serialization/ASTReader.cpp |  2 +-
 clang/lib/Serialization/ASTReaderDecl.cpp |  8 +--
 clang/lib/Serialization/ASTWriter.cpp |  2 +-
 clang/lib/Serialization/ASTWriterDecl.cpp |  8 +--
 .../hashing-decls-in-exprs-from-gmf.cppm  | 67 +++
 9 files changed, 93 insertions(+), 18 deletions(-)
 create mode 100644 clang/test/Modules/hashing-decls-in-exprs-from-gmf.cppm

diff --git a/clang/include/clang/AST/DeclBase.h 
b/clang/include/clang/AST/DeclBase.h
index 9a4736019d1b1b..eb7a1a32060077 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -673,6 +673,16 @@ class alignas(8) Decl {
   /// fragment. See [module.global.frag]p3,4 for details.
   bool isDiscardedInGlobalModuleFragment() const { return false; }
 
+  /// Check if we should skip checking ODRHash for declaration \param D.
+  ///
+  /// The existing ODRHash mechanism seems to be not stable enough and
+  /// the false positive ODR violation reports are annoying and we rarely see
+  /// true ODR violation reports. Also we learned that MSVC disabled ODR checks
+  /// for declarations in GMF. So we try to disable ODR checks in the GMF to
+  /// get better user experiences before we make the ODR violation checks 
stable
+  /// enough.
+  bool shouldSkipCheckingODR() const;
+
   /// Return true if this declaration has an attribute which acts as
   /// definition of the entity, such as 'alias' or 'ifunc'.
   bool hasDefiningAttr() const;
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index cd28226c295b32..62c25f5b7a0df8 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2451,13 +2451,6 @@ class BitsUnpacker {
   uint32_t Value;
   uint32_t CurrentBitsIndex = ~0;
 };
-
-inline bool shouldSkipCheckingODR(const Decl *D) {
-  return D->getOwningModule() &&
- D->getASTContext().getLangOpts().SkipODRCheckInGMF &&
- D->getOwningModule()->isExplicitGlobalModule();
-}
-
 } // namespace clang
 
 #endif // LLVM_CLANG_SERIALIZATION_ASTREADER_H
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 26fdfa040796ed..1ee33fd7576d7d 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4476,7 +4476,7 @@ unsigned FunctionDecl::getODRHash() {
   }
 
   class ODRHash Hash;
-  Hash.AddFunctionDecl(this);
+  Hash.AddFunctionDecl(this, /*SkipBody=*/shouldSkipCheckingODR());
   setHasODRHash(true);
   ODRHash = Hash.CalculateHash();
   return ODRHash;
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index 8163f9bdaf8d97..6b3c13ff206d23 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1102,6 +1102,11 @@ bool Decl::isInAnotherModuleUnit() const {
   return M != getASTContext().getCurrentNamedModule();
 }
 
+bool Decl::shouldSkipCheckingODR() const {
+  return getASTContext().getLangOpts().SkipODRCheckInGMF && getOwningModule() 
&&
+ getOwningModule()->isExplicitGlobalModule();
+}
+
 static Decl::Kind getKind(const Decl *D) { return D->getKind(); }
 static Decl::Kind getKind(const DeclContext *DC) { return DC->getDeclKind(); }
 
diff --git a/clang/lib/Serializati

[llvm-branch-commits] [clang] release/18.x: backport [C++20] [Moduls] Avoid computing odr hash for functions from comparing constraint expression (PR #84723)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

@tstellar Could we backport this to 18.x?

https://github.com/llvm/llvm-project/pull/84723
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] release/18.x: Reland Print library module manifest path again (#84881) (PR #85637)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 closed 
https://github.com/llvm/llvm-project/pull/85637
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] release/18.x: Reland Print library module manifest path again (#84881) (PR #85637)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

Yeah, agreed.

https://github.com/llvm/llvm-project/pull/85637
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Introduce OnDiskHashTable for specializations (PR #83233)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/83233

>From f87a54e8ae2afc807cb275f5763691e06562ab0a Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 28 Feb 2024 11:41:53 +0800
Subject: [PATCH] [Serialization] Introduce OnDiskHashTable for specializations

Following up for https://github.com/llvm/llvm-project/pull/83108

This follows the suggestion literally from
https://github.com/llvm/llvm-project/pull/76774#issuecomment-1951172457

which introduces OnDiskHashTable for specializations based on D41416.

Note that I didn't polish this patch to reduce the diff from D41416 to
it easier to review. I'll make the polishing patch later. So that we can
focus what we're doing in this patch and focus on the style in the next
patch.
---
 clang/include/clang/AST/ExternalASTSource.h   |  11 +
 .../clang/Sema/MultiplexExternalSemaSource.h  |   6 +
 .../include/clang/Serialization/ASTBitCodes.h |   5 +
 clang/include/clang/Serialization/ASTReader.h |  34 ++-
 clang/include/clang/Serialization/ASTWriter.h |  13 +
 clang/lib/AST/DeclTemplate.cpp|  17 ++
 clang/lib/AST/ExternalASTSource.cpp   |   5 +
 .../lib/Sema/MultiplexExternalSemaSource.cpp  |  12 +
 clang/lib/Serialization/ASTReader.cpp | 145 +-
 clang/lib/Serialization/ASTReaderDecl.cpp |  27 ++
 clang/lib/Serialization/ASTReaderInternals.h  | 124 +
 clang/lib/Serialization/ASTWriter.cpp | 173 +++-
 clang/lib/Serialization/ASTWriterDecl.cpp |  32 ++-
 clang/unittests/Serialization/CMakeLists.txt  |   1 +
 .../Serialization/LoadSpecLazilyTest.cpp  | 260 ++
 15 files changed, 852 insertions(+), 13 deletions(-)
 create mode 100644 clang/unittests/Serialization/LoadSpecLazilyTest.cpp

diff --git a/clang/include/clang/AST/ExternalASTSource.h 
b/clang/include/clang/AST/ExternalASTSource.h
index 8e573965b0a336..af476aa8c57824 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -150,6 +150,17 @@ class ExternalASTSource : public 
RefCountedBase {
   virtual bool
   FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name);
 
+  /// Load all the external specializations for the Decl \param D if \param
+  /// OnlyPartial is false. Otherwise, load all the external **partial**
+  /// specializations for the \param D.
+  virtual void LoadExternalSpecializations(const Decl *D, bool OnlyPartial);
+
+  /// Load all the specializations for the Decl \param D with the same template
+  /// args specified by \param TemplateArgs.
+  virtual void
+  LoadExternalSpecializations(const Decl *D,
+  ArrayRef TemplateArgs);
+
   /// Ensures that the table of all visible declarations inside this
   /// context is up to date.
   ///
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h 
b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
index 2bf91cb5212c5e..f09f037da0556a 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -97,6 +97,12 @@ class MultiplexExternalSemaSource : public 
ExternalSemaSource {
   bool FindExternalVisibleDeclsByName(const DeclContext *DC,
   DeclarationName Name) override;
 
+  void LoadExternalSpecializations(const Decl *D, bool OnlyPartial) override;
+
+  void
+  LoadExternalSpecializations(const Decl *D,
+  ArrayRef TemplateArgs) 
override;
+
   /// Ensures that the table of all visible declarations inside this
   /// context is up to date.
   void completeVisibleDeclsMap(const DeclContext *DC) override;
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index f762116fea956c..20ae6d4bbbcb3f 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -698,6 +698,8 @@ enum ASTRecordTypes {
   /// Record code for an unterminated \#pragma clang assume_nonnull begin
   /// recorded in a preamble.
   PP_ASSUME_NONNULL_LOC = 67,
+
+  UPDATE_SPECIALIZATION = 68,
 };
 
 /// Record types used within a source manager block.
@@ -1523,6 +1525,9 @@ enum DeclCode {
   /// A HLSLBufferDecl record.
   DECL_HLSL_BUFFER,
 
+  // A decls specilization record.
+  DECL_SPECIALIZATIONS,
+
   /// An ImplicitConceptSpecializationDecl record.
   DECL_IMPLICIT_CONCEPT_SPECIALIZATION,
 
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 370d8037a4da17..5640e1eed00388 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -340,6 +340,9 @@ class ASTIdentifierLookupTrait;
 /// The on-disk hash table(s) used for DeclContext name lookup.
 struct DeclContextLookupTable;
 
+/// The on-disk hash table(s) used for specialization decls.
+struct LazySpecializationInfoLookupTable

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/83237

>From 40012e175a6ab3420e2dcaa64538b210173d6950 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 28 Feb 2024 11:41:53 +0800
Subject: [PATCH 1/2] [Serialization] Code cleanups and polish 83233

---
 clang/include/clang/AST/DeclTemplate.h| 40 ++---
 clang/include/clang/AST/ExternalASTSource.h   |  4 +-
 .../clang/Sema/MultiplexExternalSemaSource.h  |  2 +-
 .../include/clang/Serialization/ASTBitCodes.h |  2 +-
 clang/include/clang/Serialization/ASTReader.h |  2 +-
 clang/lib/AST/DeclTemplate.cpp| 85 +--
 clang/lib/AST/ExternalASTSource.cpp   |  6 +-
 .../lib/Sema/MultiplexExternalSemaSource.cpp  |  7 +-
 clang/lib/Serialization/ASTCommon.h   |  1 -
 clang/lib/Serialization/ASTReader.cpp | 17 ++--
 clang/lib/Serialization/ASTReaderDecl.cpp | 76 +
 clang/lib/Serialization/ASTReaderInternals.h  |  1 -
 clang/lib/Serialization/ASTWriter.cpp | 26 +-
 clang/lib/Serialization/ASTWriterDecl.cpp | 52 +---
 14 files changed, 87 insertions(+), 234 deletions(-)

diff --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
index 51caef54baac26..870c0d1bdd4610 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -256,8 +256,8 @@ class TemplateArgumentList final
   TemplateArgumentList(const TemplateArgumentList &) = delete;
   TemplateArgumentList &operator=(const TemplateArgumentList &) = delete;
 
-  /// Create hash for the given arguments.
-  static unsigned ComputeODRHash(ArrayRef Args);
+  /// Create stable hash for the given arguments across compiler invocations.
+  static unsigned ComputeStableHash(ArrayRef Args);
 
   /// Create a new template argument list that copies the given set of
   /// template arguments.
@@ -733,25 +733,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
   }
 
   void anchor() override;
-  struct LazySpecializationInfo {
-uint32_t DeclID = ~0U;
-unsigned ODRHash = ~0U;
-bool IsPartial = false;
-LazySpecializationInfo(uint32_t ID, unsigned Hash = ~0U,
-   bool Partial = false)
-: DeclID(ID), ODRHash(Hash), IsPartial(Partial) {}
-LazySpecializationInfo() {}
-bool operator<(const LazySpecializationInfo &Other) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo &Other) const {
-  assert((DeclID != Other.DeclID || ODRHash == Other.ODRHash) &&
- "Hashes differ!");
-  assert((DeclID != Other.DeclID || IsPartial == Other.IsPartial) &&
- "Both must be the same kinds!");
-  return DeclID == Other.DeclID;
-}
-  };
 
 protected:
   template  struct SpecEntryTraits {
@@ -795,16 +776,20 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 
   void loadLazySpecializationsImpl(bool OnlyPartial = false) const;
 
-  void loadLazySpecializationsImpl(llvm::ArrayRef Args,
+  bool loadLazySpecializationsImpl(llvm::ArrayRef Args,
TemplateParameterList *TPL = nullptr) const;
 
-  Decl *loadLazySpecializationImpl(LazySpecializationInfo &LazySpecInfo) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector &Specs,
  void *&InsertPos, ProfileArguments &&...ProfileArgs);
 
+  template 
+  typename SpecEntryTraits::DeclType *
+  findSpecializationLocally(llvm::FoldingSetVector &Specs,
+void *&InsertPos,
+ProfileArguments &&...ProfileArgs);
+
   template 
   void addSpecializationImpl(llvm::FoldingSetVector &Specs,
  EntryType *Entry, void *InsertPos);
@@ -820,13 +805,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 llvm::PointerIntPair
   InstantiatedFromMember;
 
-/// If non-null, points to an array of specializations (including
-/// partial specializations) known only by their external declaration IDs.
-///
-/// The first value in the array is the number of specializations/partial
-/// specializations that follow.
-LazySpecializationInfo *LazySpecializations = nullptr;
-
 /// The set of "injected" template arguments used within this
 /// template.
 ///
diff --git a/clang/include/clang/AST/ExternalASTSource.h 
b/clang/include/clang/AST/ExternalASTSource.h
index af476aa8c57824..769abf44ecd430 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -157,7 +157,9 @@ class ExternalASTSource : public 
RefCountedBase {
 
   /// Load all the specializations for the Decl \param D with the same template
   /// args specified by \param TemplateArgs.
-  virtual void
+  ///
+  /// Return true if any new specializations get loaded. Return false 
otherwise.
+  virtual bool
   LoadExternalSpeci

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

Thanks for the reduced test case. It is pretty helpful. I've added it to the 
current patch.

The root cause of the issue comes from an oversight in 
https://github.com/llvm/llvm-project/pull/83108/files#diff-dffd10772d07fb8c737cdf812839afa0173447c4812698c0c19618c34d92daddR806-R829.
 That we calculate the ODR Hash for class template specialization twice. Then 
for the example you described, the linear recursive computation becomes to 
somewhat similar to fibonacci computation:

```
wrap<40> -> wrap<39>  -> wrap<38> -> ...
  |-> wrap<38>
   -> wrap<39> -> wrap<38>
   -> wrap<38>
```

This test case passes quickly now after I remove the duplicated ODR computation.

https://github.com/llvm/llvm-project/pull/83237
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

> Sorry, still struggling to get a small repro. The build graphs we have are 
> quite large, unfortunately. Did any of the stack traces or error message I 
> posted help to find certain problems? Or is there no hope until we get a 
> smaller repro?

I tried to review the patch purely but it looks like not easy to fix the 
failures without reproducers... for performances, I am wondering if we can 
improve it by caching the hash results. But it will be helpful if we can have 
profile data for that.

A lot thanks for testing this.

https://github.com/llvm/llvm-project/pull/83237
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Introduce OnDiskHashTable for specializations (PR #83233)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/83233

>From 80c9ab1d56e1e69407af75444f276df446008fed Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 28 Feb 2024 11:41:53 +0800
Subject: [PATCH] [Serialization] Introduce OnDiskHashTable for specializations

Following up for https://github.com/llvm/llvm-project/pull/83108

This follows the suggestion literally from
https://github.com/llvm/llvm-project/pull/76774#issuecomment-1951172457

which introduces OnDiskHashTable for specializations based on
D41416.

Note that I didn't polish this patch to reduce the diff from D41416
to it easier to review. I'll make the polishing patch later. So that we
can focus what we're doing in this patch and focus on the style in the
next patch.
---
 clang/include/clang/AST/ExternalASTSource.h   |  11 +
 .../clang/Sema/MultiplexExternalSemaSource.h  |   6 +
 .../include/clang/Serialization/ASTBitCodes.h |   6 +
 clang/include/clang/Serialization/ASTReader.h |  34 ++-
 clang/include/clang/Serialization/ASTWriter.h |  15 +
 clang/lib/AST/DeclTemplate.cpp|  17 ++
 clang/lib/AST/ExternalASTSource.cpp   |   5 +
 .../lib/Sema/MultiplexExternalSemaSource.cpp  |  12 +
 clang/lib/Serialization/ASTReader.cpp | 145 +-
 clang/lib/Serialization/ASTReaderDecl.cpp |  27 ++
 clang/lib/Serialization/ASTReaderInternals.h  | 124 +
 clang/lib/Serialization/ASTWriter.cpp | 174 +++-
 clang/lib/Serialization/ASTWriterDecl.cpp |  32 ++-
 clang/unittests/Serialization/CMakeLists.txt  |   1 +
 .../Serialization/LoadSpecLazilyTest.cpp  | 260 ++
 15 files changed, 856 insertions(+), 13 deletions(-)
 create mode 100644 clang/unittests/Serialization/LoadSpecLazilyTest.cpp

diff --git a/clang/include/clang/AST/ExternalASTSource.h 
b/clang/include/clang/AST/ExternalASTSource.h
index 385c32edbae0fd..24c4490d160f3f 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -150,6 +150,17 @@ class ExternalASTSource : public 
RefCountedBase {
   virtual bool
   FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name);
 
+  /// Load all the external specializations for the Decl \param D if \param
+  /// OnlyPartial is false. Otherwise, load all the external **partial**
+  /// specializations for the \param D.
+  virtual void LoadExternalSpecializations(const Decl *D, bool OnlyPartial);
+
+  /// Load all the specializations for the Decl \param D with the same template
+  /// args specified by \param TemplateArgs.
+  virtual void
+  LoadExternalSpecializations(const Decl *D,
+  ArrayRef TemplateArgs);
+
   /// Ensures that the table of all visible declarations inside this
   /// context is up to date.
   ///
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h 
b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
index 238fb398b7d129..f81b70daa4b3d0 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -97,6 +97,12 @@ class MultiplexExternalSemaSource : public 
ExternalSemaSource {
   bool FindExternalVisibleDeclsByName(const DeclContext *DC,
   DeclarationName Name) override;
 
+  void LoadExternalSpecializations(const Decl *D, bool OnlyPartial) override;
+
+  void
+  LoadExternalSpecializations(const Decl *D,
+  ArrayRef TemplateArgs) 
override;
+
   /// Ensures that the table of all visible declarations inside this
   /// context is up to date.
   void completeVisibleDeclsMap(const DeclContext *DC) override;
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index 186c3b722ced16..b20a59ae8a2e9b 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -694,6 +694,9 @@ enum ASTRecordTypes {
   /// Record code for lexical and visible block for delayed namespace in
   /// reduced BMI.
   DELAYED_NAMESPACE_LEXICAL_VISIBLE_RECORD = 68,
+
+  /// Record code for updated specialization
+  UPDATE_SPECIALIZATION = 69,
 };
 
 /// Record types used within a source manager block.
@@ -1451,6 +1454,9 @@ enum DeclCode {
   /// A HLSLBufferDecl record.
   DECL_HLSL_BUFFER,
 
+  // A decls specilization record.
+  DECL_SPECIALIZATIONS,
+
   /// An ImplicitConceptSpecializationDecl record.
   DECL_IMPLICIT_CONCEPT_SPECIALIZATION,
 
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 64f1ebc117b327..d1ec72f8475435 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -340,6 +340,9 @@ class ASTIdentifierLookupTrait;
 /// The on-disk hash table(s) used for DeclContext name lookup.
 struct DeclContextLookupTable;
 
+/// The on-disk hash table(s) used for specializa

[llvm-branch-commits] [clang] [Serialization] Introduce OnDiskHashTable for specializations (PR #83233)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

Rebased with main

https://github.com/llvm/llvm-project/pull/83233
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/83237

>From f4edc5b1cde1735d1c9c9f6c43ef4f50066965b0 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 28 Feb 2024 11:41:53 +0800
Subject: [PATCH 1/2] [Serialization] Code cleanups and polish 83233

---
 clang/include/clang/AST/DeclTemplate.h| 40 ++---
 clang/include/clang/AST/ExternalASTSource.h   |  4 +-
 .../clang/Sema/MultiplexExternalSemaSource.h  |  2 +-
 .../include/clang/Serialization/ASTBitCodes.h |  2 +-
 clang/include/clang/Serialization/ASTReader.h |  2 +-
 clang/lib/AST/DeclTemplate.cpp| 88 +--
 clang/lib/AST/ExternalASTSource.cpp   |  6 +-
 .../lib/Sema/MultiplexExternalSemaSource.cpp  |  7 +-
 clang/lib/Serialization/ASTCommon.h   |  1 -
 clang/lib/Serialization/ASTReader.cpp | 17 ++--
 clang/lib/Serialization/ASTReaderDecl.cpp | 76 +---
 clang/lib/Serialization/ASTReaderInternals.h  |  1 -
 clang/lib/Serialization/ASTWriter.cpp | 26 +-
 clang/lib/Serialization/ASTWriterDecl.cpp | 52 +--
 14 files changed, 87 insertions(+), 237 deletions(-)

diff --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
index fb32639b86351c..7b1966e0c19c83 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -256,8 +256,8 @@ class TemplateArgumentList final
   TemplateArgumentList(const TemplateArgumentList &) = delete;
   TemplateArgumentList &operator=(const TemplateArgumentList &) = delete;
 
-  /// Create hash for the given arguments.
-  static unsigned ComputeODRHash(ArrayRef Args);
+  /// Create stable hash for the given arguments across compiler invocations.
+  static unsigned ComputeStableHash(ArrayRef Args);
 
   /// Create a new template argument list that copies the given set of
   /// template arguments.
@@ -733,25 +733,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
   }
 
   void anchor() override;
-  struct LazySpecializationInfo {
-GlobalDeclID DeclID = GlobalDeclID();
-unsigned ODRHash = ~0U;
-bool IsPartial = false;
-LazySpecializationInfo(GlobalDeclID ID, unsigned Hash = ~0U,
-   bool Partial = false)
-: DeclID(ID), ODRHash(Hash), IsPartial(Partial) {}
-LazySpecializationInfo() {}
-bool operator<(const LazySpecializationInfo &Other) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo &Other) const {
-  assert((DeclID != Other.DeclID || ODRHash == Other.ODRHash) &&
- "Hashes differ!");
-  assert((DeclID != Other.DeclID || IsPartial == Other.IsPartial) &&
- "Both must be the same kinds!");
-  return DeclID == Other.DeclID;
-}
-  };
 
 protected:
   template  struct SpecEntryTraits {
@@ -795,16 +776,20 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 
   void loadLazySpecializationsImpl(bool OnlyPartial = false) const;
 
-  void loadLazySpecializationsImpl(llvm::ArrayRef Args,
+  bool loadLazySpecializationsImpl(llvm::ArrayRef Args,
TemplateParameterList *TPL = nullptr) const;
 
-  Decl *loadLazySpecializationImpl(LazySpecializationInfo &LazySpecInfo) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector &Specs,
  void *&InsertPos, ProfileArguments &&...ProfileArgs);
 
+  template 
+  typename SpecEntryTraits::DeclType *
+  findSpecializationLocally(llvm::FoldingSetVector &Specs,
+void *&InsertPos,
+ProfileArguments &&...ProfileArgs);
+
   template 
   void addSpecializationImpl(llvm::FoldingSetVector &Specs,
  EntryType *Entry, void *InsertPos);
@@ -820,13 +805,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 llvm::PointerIntPair
   InstantiatedFromMember;
 
-/// If non-null, points to an array of specializations (including
-/// partial specializations) known only by their external declaration IDs.
-///
-/// The first value in the array is the number of specializations/partial
-/// specializations that follow.
-LazySpecializationInfo *LazySpecializations = nullptr;
-
 /// The set of "injected" template arguments used within this
 /// template.
 ///
diff --git a/clang/include/clang/AST/ExternalASTSource.h 
b/clang/include/clang/AST/ExternalASTSource.h
index 24c4490d160f3f..2f71d6030a12c9 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -157,7 +157,9 @@ class ExternalASTSource : public 
RefCountedBase {
 
   /// Load all the specializations for the Decl \param D with the same template
   /// args specified by \param TemplateArgs.
-  virtual void
+  ///
+  /// Return true if any new specializations get loaded. Return false 
otherwise.
+  virtual bool
   

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

Rebased with main

https://github.com/llvm/llvm-project/pull/83237
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] No transitive identifier change (PR #92085)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 created 
https://github.com/llvm/llvm-project/pull/92085

Following of https://github.com/llvm/llvm-project/pull/92083

The motivation is still cutting of the unnecessary change in the dependency 
chain. See the above link (recursively) for details.

After this patch, (and the above patch), we can already do something pretty 
interesting. For example,

 Motivation example

```

//--- m-partA.cppm
export module m:partA;

export inline int getA() {
return 43;
}

export class A {
public:
int getMem();
};

export template 
class ATempl {
public:
T getT();
};

//--- m-partA.v1.cppm
export module m:partA;

export inline int getA() {
return 43;
}

// Now we add a new declaration without introducing a new type.
// The consuming module which didn't use m:partA completely is expected to be
// not changed.
export inline int getA2() {
return 88;
}

export class A {
public:
int getMem();
// Now we add a new declaration without introducing a new type.
// The consuming module which didn't use m:partA completely is expected to 
be
// not changed.
int getMem2();
};

export template 
class ATempl {
public:
T getT();
// Add a new declaration without introducing a new type.
T getT2();
};

//--- m-partB.cppm
export module m:partB;

export inline int getB() {
return 430;
}

//--- m.cppm
export module m;
export import :partA;
export import :partB;

//--- useBOnly.cppm
export module useBOnly;
import m;

export inline int get() {
return getB();
}
```

In this example, module `m` exports two partitions `:partA` and `:partB`. And a 
consumer `useBOnly` only consumes the entities from `:partB`. So we don't hope 
the BMI of `useBOnly` changes if only `:partA` changes. After this patch, we 
can make it if the change of `:partA` doesn't introduce new types. (And we can 
get rid of this if we make no-transitive-type-change).

As the example shows, when we change the implementation of `:partA` from 
`m-partA.cppm` to `m-partA.v1.cppm`, we add new function declaration `getA2()` 
at the global namespace, add a new member function `getMem2()` to class `A` and 
add a new member function to  `getT2()` to class template `ATempl`. And since 
`:partA` is not used by `useBOnly` completely, the BMI of  `useBOnly`  won't 
change after we made above changes.

 Design details

Method used in this patch is similar with 
https://github.com/llvm/llvm-project/pull/92083 and 
https://github.com/llvm/llvm-project/pull/86912. It extends the 32 bit 
IdentifierID to 64 bits and use the higher 32 bits to store the module file 
index. So that the encoding of the identifier won't get affected by other 
modules.

 Overhead

Similar with https://github.com/llvm/llvm-project/pull/92083 and 
https://github.com/llvm/llvm-project/pull/86912. The change is only expected to 
increase the size of the on-disk .pcm files and not affect the compile-time 
performances. And from my experiment, the size of the on-disk change only 
increase 1%+ and observe no compile-time impacts.

 Future Plans

I'll try to do the same thing for type ids. IIRC, it won't change the 
dependency graph if we add a new type in an unused units. I do think this is a 
significant win. And this will be a pretty good answer to "why modules are 
better than headers."




>From d32a410d4847333870dff5cefb0b93c0d7facc80 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|   2 +-
 clang/include/clang/Lex/HeaderSearch.h|   2 +-
 .../include/clang/Serialization/ASTBitCodes.h |   2 +-
 clang/include/clang/Serialization/ASTReader.h |  19 ++--
 .../include/clang/Serialization/ModuleFile.h  |   3 -
 clang/lib/Serialization/ASTReader.cpp |  96 +
 clang/lib/Serialization/ASTWriter.cpp |  61 +++
 clang/lib/Serialization/GlobalModuleIndex.cpp |   3 +-
 clang/lib/Serialization/ModuleFile.cpp|   1 -
 .../no-transitive-identifier-change.cppm  | 102 ++
 10 files changed, 206 insertions(+), 85 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..bd5c11a4577f5 100644
--- a/clang/include/clang/Lex/ExternalPreprocessorSource.h
+++ b/clang/include/clang/Lex/ExternalPreprocessorSource.h
@@ -36,7 +36,7 @@ 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;
diff --git a/clang/include/clang/Lex/HeaderSearch.h 
b/clan

[llvm-branch-commits] [clang] [Serialization] No transitive identifier change (PR #92085)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/92085

>From c612b56dec8bfc7c1612e94be8876316f14ea8ea 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|   2 +-
 clang/include/clang/Lex/HeaderSearch.h|   2 +-
 .../include/clang/Serialization/ASTBitCodes.h |   2 +-
 clang/include/clang/Serialization/ASTReader.h |  19 ++-
 .../include/clang/Serialization/ModuleFile.h  |   3 -
 clang/lib/Serialization/ASTReader.cpp |  96 ---
 clang/lib/Serialization/ASTWriter.cpp |  61 ++
 clang/lib/Serialization/GlobalModuleIndex.cpp |   3 +-
 clang/lib/Serialization/ModuleFile.cpp|   1 -
 .../no-transitive-identifier-change.cppm  | 110 ++
 10 files changed, 214 insertions(+), 85 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..bd5c11a4577f5 100644
--- a/clang/include/clang/Lex/ExternalPreprocessorSource.h
+++ b/clang/include/clang/Lex/ExternalPreprocessorSource.h
@@ -36,7 +36,7 @@ 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;
diff --git a/clang/include/clang/Lex/HeaderSearch.h 
b/clang/include/clang/Lex/HeaderSearch.h
index 5ac634d4e..cb75dd429c448 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -124,7 +124,7 @@ struct HeaderFileInfo {
   /// 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;
+  uint64_t ControllingMacroID = 0;
 
   /// If this file has a \#ifndef XXX (or equivalent) guard that
   /// protects the entire contents of the file, this is the identifier
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index 772452e3afc55..1fd482b5aff0e 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -59,7 +59,7 @@ const unsigned VERSION_MINOR = 1;
 ///
 /// The ID numbers of identifiers are consecutive (in order of discovery)
 /// and start at 1. 0 is reserved for NULL.
-using IdentifierID = uint32_t;
+using IdentifierID = uint64_t;
 
 /// The number of predefined identifier IDs.
 const unsigned int NUM_PREDEF_IDENT_IDS = 1;
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index d028d52fc5ef1..1da9123280f26 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -657,14 +657,6 @@ class ASTReader
   /// been loaded.
   std::vector IdentifiersLoaded;
 
-  using GlobalIdentifierMapType =
-  ContinuousRangeMap;
-
-  /// Mapping from global identifier IDs to the module in which the
-  /// identifier resides along with the offset that should be added to the
-  /// global identifier ID to produce a local ID.
-  GlobalIdentifierMapType GlobalIdentifierMap;
-
   /// A vector containing macros that have already been
   /// loaded.
   ///
@@ -1536,6 +1528,11 @@ class ASTReader
   /// Translate a \param GlobalDeclID to the index of DeclsLoaded array.
   unsigned translateGlobalDeclIDToIndex(GlobalDeclID ID) const;
 
+  /// Translate an \param IdentifierID ID to the index of IdentifiersLoaded
+  /// array and the corresponding module file.
+  std::pair
+  translateIdentifierIDToIndex(serialization::IdentifierID ID) const;
+
 public:
   /// Load the AST file and validate its contents against the given
   /// Preprocessor.
@@ -2120,7 +2117,7 @@ class ASTReader
   /// Load a selector from disk, registering its ID if it exists.
   void LoadSelector(Selector Sel);
 
-  void SetIdentifierInfo(unsigned ID, IdentifierInfo *II);
+  void SetIdentifierInfo(serialization::IdentifierID ID, IdentifierInfo *II);
   void SetGloballyVisibleDecls(IdentifierInfo *II,
const SmallVectorImpl &DeclIDs,
SmallVectorImpl *Decls = nullptr);
@@ -2145,10 +2142,10 @@ class ASTReader
 return DecodeIdentifierInfo(ID);
   }
 
-  IdentifierInfo *getLocalIdentifier(ModuleFile &M, unsigned LocalID);
+  IdentifierInfo *getLocalIdentifier(ModuleFile &M, uint64_t LocalID);
 
   serialization::IdentifierID getGlobalIdentifierID(ModuleFile &M,
-unsigned LocalID);
+  

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 created 
https://github.com/llvm/llvm-project/pull/92511

Following of https://github.com/llvm/llvm-project/pull/92085. 

 motivation

The motivation is still cutting of the unnecessary change in the dependency 
chain. See the above link (recursively) for details.

And this will be the last patch of the `no-transitive-*-change` series. If 
there are any following patches, they might be C++20 Named modules specific to 
handle special grammars like `ADL` (See the reply in 
https://discourse.llvm.org/t/rfc-c-20-modules-introduce-thin-bmi-and-decls-hash/74755/53
 for example). So they won't affect the whole serialization part as the series 
patch did.

 example

After this patch, finally we are able to cut of unnecessary change of types. 
For example, 

```

//--- m-partA.cppm
export module m:partA;

//--- m-partA.v1.cppm
export module m:partA;

namespace NS {
class A {
public:
int getValue() {
return 43;
}
};
}

//--- m-partB.cppm
export module m:partB;

export inline int getB() {
return 430;
}

//--- m.cppm
export module m;
export import :partA;
export import :partB;

//--- useBOnly.cppm
export module useBOnly;
import m;

export inline int get() {
return getB();
}
```

The BMI of `useBOnly.cppm` is expected to not change if we only add a new class 
in `m:partA`. This will be pretty useful in practice.

 implementation details

The key idea of this patch is similar with the previous patches: extend the 
32bits type ID to 64bits so that we can store the module file index in the 
higher bits. Then the encoding of the type ID is independent on the imported 
modules.

But there are two differences from the previous patches:
- TypeID is not completely an index of serialized types. We used the lower 3 
bits to store the qualifiers.
- TypeID won't take part in any lookup process. So the uses of TypeID is much 
less than the previous patches.

The first difference make we have some more slightly complex bit operations. 
And the second difference makes the patch much simpler than the previous ones.


>From 2265f12343f929cc81f2b4fe6d27cc4ff3f31ec2 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Fri, 17 May 2024 14:25:53 +0800
Subject: [PATCH] [serialization] No transitive type change

---
 .../include/clang/Serialization/ASTBitCodes.h |  32 --
 clang/include/clang/Serialization/ASTReader.h |  23 ++--
 .../clang/Serialization/ASTRecordReader.h |   2 +-
 .../include/clang/Serialization/ModuleFile.h  |   3 -
 clang/lib/Serialization/ASTReader.cpp | 104 +-
 clang/lib/Serialization/ASTWriter.cpp |  31 +++---
 clang/lib/Serialization/ModuleFile.cpp|   1 -
 .../Modules/no-transitive-decls-change.cppm   |  12 +-
 .../no-transitive-identifier-change.cppm  |   3 -
 .../Modules/no-transitive-type-change.cppm|  68 
 clang/test/Modules/pr5.cppm   |  36 +++---
 11 files changed, 196 insertions(+), 119 deletions(-)
 create mode 100644 clang/test/Modules/no-transitive-type-change.cppm

diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index 1fd482b5aff0e..486d5f4042c61 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -26,6 +26,7 @@
 #include "clang/Serialization/SourceLocationEncoding.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/Bitstream/BitCodes.h"
+#include "llvm/Support/MathExtras.h"
 #include 
 #include 
 
@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict
-/// qualifiers (as with QualType) and the upper bits provide a
-/// type index. The type index values are partitioned into two
+/// qualifiers (as with QualType).
+/// - the upper 29 bits provide a type index in the corresponding
+/// module file.
+/// - the upper 32 bits provide a module file index.
+///
+/// The type index values are partitioned into two
 /// sets. The values below NUM_PREDEF_TYPE_IDs are predefined type
 /// IDs (based on the PREDEF_TYPE_*_ID constants), with 0 as a
 /// placeholder for "no type". Values from NUM_PREDEF_TYPE_IDs are
 /// other types that have serialized representations.
-using TypeID = uint32_t;
+using TypeID = uint64_t;
 
 /// A type index; the type ID with the qualifier bits removed.
+/// Keep structure alignment 32-bit since the blob is assumed as 32-bit
+/// aligned.
 class TypeIdx {
+  uint32_t ModuleFileIndex = 0;
   uint32_t Idx = 0;
 
 public:
   TypeIdx() = default;
-  explicit TypeIdx(uint32_t index) : Idx(index) {}
+  explicit TypeIdx(uint32_t Idx) : ModuleFileIndex(0), Idx(Idx) {}
+
+  explicit TypeIdx(uint32_t ModuleFileIdx, uint32_t Idx)
+  

[llvm-branch-commits] [clang] [Serialization] No transitive identifier change (PR #92085)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

@jansvoboda11 @Bigcheese gentle ping

https://github.com/llvm/llvm-project/pull/92085
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

@jansvoboda11 @Bigcheese gentle ping

https://github.com/llvm/llvm-project/pull/92511
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] No transitive identifier change (PR #92085)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/92085

>From a2fb7f50161932a9557a22a4ba23f827e80a4d6b 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 &operator=(const IdentifierInfo *Ptr) {
+this->Ptr = reinterpret_cast(Ptr);
+return *this;
+  }
+
+  LazyIdentifierInfoPtr &operator=(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;
   }
 

[llvm-branch-commits] [clang] [Serialization] No transitive identifier change (PR #92085)

[Chuanqi Xu via llvm-branch-commits]

@@ -124,7 +124,7 @@ struct HeaderFileInfo {
   /// 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;
+  uint64_t ControllingMacroID = 0;

ChuanqiXu9 wrote:

Done by adding `LazyIdentifierInfoPtr` to `ExternalPreprocessorSource.h`.

https://github.com/llvm/llvm-project/pull/92085
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] No transitive identifier change (PR #92085)

[Chuanqi Xu via llvm-branch-commits]

@@ -918,7 +918,7 @@ ASTSelectorLookupTrait::ReadKey(const unsigned char* d, 
unsigned) {
   SelectorTable &SelTable = Reader.getContext().Selectors;
   unsigned N = endian::readNext(d);
   const IdentifierInfo *FirstII = Reader.getLocalIdentifier(
-  F, endian::readNext(d));
+  F, endian::readNext(d));

ChuanqiXu9 wrote:

If `IdentifierID` is not integral type, the code can't compile 
(`endian::readNext` won't accept that). So I feel it might not be so useful.

https://github.com/llvm/llvm-project/pull/92085
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] No transitive identifier change (PR #92085)

[Chuanqi Xu via llvm-branch-commits]

@@ -3896,7 +3903,7 @@ void ASTWriter::WriteIdentifierTable(Preprocessor &PP,
 
   // 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:

Maybe it is because the name `FirstIdentID` is confusing. The proper name may 
be `FirstLocalIdentID`.

Previously, before this patch, `FirstIdentID` will be `Number of identifier ID` 
+ 1. This is the reason why the encoding of Identifier IDs may be affected by 
imported modules. This is also the major motivation of the patch. 

So all the ID bigger or equal to FirstIdentID must be the ID for local 
identifiers previously. But it won't be the case we make this patch.

https://github.com/llvm/llvm-project/pull/92085
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/92511

>From 1a83e2b0f00183c61e7a5303ea7eeb0b279c7e91 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Fri, 17 May 2024 14:25:53 +0800
Subject: [PATCH] [serialization] No transitive type change

---
 .../include/clang/Serialization/ASTBitCodes.h |  32 --
 clang/include/clang/Serialization/ASTReader.h |  23 ++--
 .../clang/Serialization/ASTRecordReader.h |   2 +-
 .../include/clang/Serialization/ModuleFile.h  |   3 -
 clang/lib/Serialization/ASTReader.cpp | 104 +-
 clang/lib/Serialization/ASTWriter.cpp |  31 +++---
 clang/lib/Serialization/ModuleFile.cpp|   1 -
 .../Modules/no-transitive-decls-change.cppm   |  12 +-
 .../no-transitive-identifier-change.cppm  |   3 -
 .../Modules/no-transitive-type-change.cppm|  68 
 clang/test/Modules/pr5.cppm   |  36 +++---
 11 files changed, 196 insertions(+), 119 deletions(-)
 create mode 100644 clang/test/Modules/no-transitive-type-change.cppm

diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index 99ad8524bde80..092d46b41e2cf 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -26,6 +26,7 @@
 #include "clang/Serialization/SourceLocationEncoding.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/Bitstream/BitCodes.h"
+#include "llvm/Support/MathExtras.h"
 #include 
 #include 
 
@@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID;
 
 /// An ID number that refers to a type in an AST file.
 ///
-/// The ID of a type is partitioned into two parts: the lower
+/// The ID of a type is partitioned into three parts:
+/// - the lower
 /// three bits are used to store the const/volatile/restrict
-/// qualifiers (as with QualType) and the upper bits provide a
-/// type index. The type index values are partitioned into two
+/// qualifiers (as with QualType).
+/// - the upper 29 bits provide a type index in the corresponding
+/// module file.
+/// - the upper 32 bits provide a module file index.
+///
+/// The type index values are partitioned into two
 /// sets. The values below NUM_PREDEF_TYPE_IDs are predefined type
 /// IDs (based on the PREDEF_TYPE_*_ID constants), with 0 as a
 /// placeholder for "no type". Values from NUM_PREDEF_TYPE_IDs are
 /// other types that have serialized representations.
-using TypeID = uint32_t;
+using TypeID = uint64_t;
 
 /// A type index; the type ID with the qualifier bits removed.
+/// Keep structure alignment 32-bit since the blob is assumed as 32-bit
+/// aligned.
 class TypeIdx {
+  uint32_t ModuleFileIndex = 0;
   uint32_t Idx = 0;
 
 public:
   TypeIdx() = default;
-  explicit TypeIdx(uint32_t index) : Idx(index) {}
+  explicit TypeIdx(uint32_t Idx) : ModuleFileIndex(0), Idx(Idx) {}
+
+  explicit TypeIdx(uint32_t ModuleFileIdx, uint32_t Idx)
+  : ModuleFileIndex(ModuleFileIdx), Idx(Idx) {}
+
+  uint32_t getModuleFileIndex() const { return ModuleFileIndex; }
 
-  uint32_t getIndex() const { return Idx; }
+  uint64_t getValue() const { return ((uint64_t)ModuleFileIndex << 32) | Idx; }
 
   TypeID asTypeID(unsigned FastQuals) const {
 if (Idx == uint32_t(-1))
   return TypeID(-1);
 
-return (Idx << Qualifiers::FastWidth) | FastQuals;
+unsigned Index = (Idx << Qualifiers::FastWidth) | FastQuals;
+return ((uint64_t)ModuleFileIndex << 32) | Index;
   }
 
   static TypeIdx fromTypeID(TypeID ID) {
 if (ID == TypeID(-1))
   return TypeIdx(-1);
 
-return TypeIdx(ID >> Qualifiers::FastWidth);
+return TypeIdx(ID >> 32, (ID & llvm::maskTrailingOnes(32)) >>
+ Qualifiers::FastWidth);
   }
 };
 
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index ef98a49cf491f..aec4e01ea07c0 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -487,14 +487,6 @@ class ASTReader
   /// ID = (I + 1) << FastQual::Width has already been loaded
   llvm::PagedVector TypesLoaded;
 
-  using GlobalTypeMapType =
-  ContinuousRangeMap;
-
-  /// Mapping from global type IDs to the module in which the
-  /// type resides along with the offset that should be added to the
-  /// global type ID to produce a local ID.
-  GlobalTypeMapType GlobalTypeMap;
-
   /// Declarations that have already been loaded from the chain.
   ///
   /// When the pointer at index I is non-NULL, the declaration with ID
@@ -1420,8 +1412,8 @@ class ASTReader
 RecordLocation(ModuleFile *M, uint64_t O) : F(M), Offset(O) {}
   };
 
-  QualType readTypeRecord(unsigned Index);
-  RecordLocation TypeCursorForIndex(unsigned Index);
+  QualType readTypeRecord(serialization::TypeID ID);
+  RecordLocation TypeCursorForIndex(serialization::TypeID ID);
   void LoadedDecl(unsigned Index, Decl *D);
   Decl *ReadDeclRecord(GlobalDeclID

[llvm-branch-commits] [clang] 88bf774 - Revert "[C++20] [Coroutines] Mark await_suspend as noinline if the awaiter is not empty"

[Chuanqi Xu via llvm-branch-commits]

Author: Chuanqi Xu
Date: 2023-09-18T15:09:00+08:00
New Revision: 88bf774c565080e30e0a073676c316ab175303af

URL: 
https://github.com/llvm/llvm-project/commit/88bf774c565080e30e0a073676c316ab175303af
DIFF: 
https://github.com/llvm/llvm-project/commit/88bf774c565080e30e0a073676c316ab175303af.diff

LOG: Revert "[C++20] [Coroutines] Mark await_suspend as noinline if the awaiter 
is not empty"

This reverts commit f05226d7e38c36efe029a0eb4201b0843f81b5e8.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGCoroutine.cpp
clang/lib/CodeGen/CodeGenFunction.h

Removed: 
clang/test/CodeGenCoroutines/coro-awaiter-noinline-suspend.cpp
clang/test/CodeGenCoroutines/pr56301.cpp



diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8ab7e065d218412..a4b61b9074d91e0 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -711,10 +711,6 @@ Bug Fixes in This Version
 - Fix a hang on valid C code passing a function type as an argument to
   ``typeof`` to form a function declaration.
   (`#64713 _`)
-- Fixed an issue where accesses to the local variables of a coroutine during
-  ``await_suspend`` could be misoptimized, including accesses to the awaiter
-  object itself.
-  (`#56301 `_)
 - Clang now correctly diagnoses ``function_needs_feature`` when always_inline
   callee has incompatible target features with caller.
 - Removed the linking of libraries when ``-r`` is passed to the driver on AIX.

diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 0d1e9ad439b7dc9..6b8af9bf18c1fff 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5487,30 +5487,6 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
&CallInfo,
 Attrs.addFnAttribute(getLLVMContext(), llvm::Attribute::AlwaysInline);
   }
 
-  // The await_suspend call performed by co_await is essentially asynchronous
-  // to the execution of the coroutine. Inlining it normally into an unsplit
-  // coroutine can cause miscompilation because the coroutine CFG misrepresents
-  // the true control flow of the program: things that happen in the
-  // await_suspend are not guaranteed to happen prior to the resumption of the
-  // coroutine, and things that happen after the resumption of the coroutine
-  // (including its exit and the potential deallocation of the coroutine frame)
-  // are not guaranteed to happen only after the end of await_suspend.
-  //
-  // The short-term solution to this problem is to mark the call as 
uninlinable.
-  // But we don't want to do this if the call is known to be trivial, which is
-  // very common.
-  //
-  // The long-term solution may introduce patterns like:
-  //
-  //  call @llvm.coro.await_suspend(ptr %awaiter, ptr %handle,
-  //ptr @awaitSuspendFn)
-  //
-  // Then it is much easier to perform the safety analysis in the middle end.
-  // If it is safe to inline the call to awaitSuspend, we can replace it in the
-  // CoroEarly pass. Otherwise we could replace it in the CoroSplit pass.
-  if (inSuspendBlock() && mayCoroHandleEscape())
-Attrs = Attrs.addFnAttribute(getLLVMContext(), llvm::Attribute::NoInline);
-
   // Disable inlining inside SEH __try blocks.
   if (isSEHTryScope()) {
 Attrs = Attrs.addFnAttribute(getLLVMContext(), llvm::Attribute::NoInline);

diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index 810ae7d51ec10c2..8437cda79beb2a7 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -139,36 +139,6 @@ static bool memberCallExpressionCanThrow(const Expr *E) {
   return true;
 }
 
-/// Return true when the coroutine handle may escape from the await-suspend
-/// (`awaiter.await_suspend(std::coroutine_handle)` expression).
-/// Return false only when the coroutine wouldn't escape in the await-suspend
-/// for sure.
-///
-/// While it is always safe to return true, return falses can bring better
-/// performances.
-///
-/// See https://github.com/llvm/llvm-project/issues/56301 and
-/// https://reviews.llvm.org/D157070 for the example and the full discussion.
-///
-/// FIXME: It will be much better to perform such analysis in the middle end.
-/// See the comments in `CodeGenFunction::EmitCall` for example.
-static bool MayCoroHandleEscape(CoroutineSuspendExpr const &S) {
-  CXXRecordDecl *Awaiter =
-  S.getCommonExpr()->getType().getNonReferenceType()->getAsCXXRecordDecl();
-
-  // Return true conservatively if the awaiter type is not a record type.
-  if (!Awaiter)
-return true;
-
-  // In case the awaiter type is empty, the suspend wouldn't leak the coroutine
-  // handle.
-  //
-  // TODO: We can improve this by looking into the implementation

[llvm-branch-commits] [clang] 2cfdebd - Revert "[NFC] [C++20] [Coroutines] Mention the side effect of a fix may bring regressions"

[Chuanqi Xu via llvm-branch-commits]

Author: Chuanqi Xu
Date: 2023-09-18T15:05:07+08:00
New Revision: 2cfdebdb7e5e430471ea833b33eee72c1938eb98

URL: 
https://github.com/llvm/llvm-project/commit/2cfdebdb7e5e430471ea833b33eee72c1938eb98
DIFF: 
https://github.com/llvm/llvm-project/commit/2cfdebdb7e5e430471ea833b33eee72c1938eb98.diff

LOG: Revert "[NFC] [C++20] [Coroutines] Mention the side effect of a fix may 
bring regressions"

This reverts commit 6998ecd330f2b028bf4678edd4f53b5489c5e6df.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 62a3c841730b97d..8ab7e065d218412 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -715,9 +715,6 @@ Bug Fixes in This Version
   ``await_suspend`` could be misoptimized, including accesses to the awaiter
   object itself.
   (`#56301 `_)
-  The current solution may bring performance regressions if the awaiters have
-  non-static data members. See
-  `#64945 `_ for details.
 - Clang now correctly diagnoses ``function_needs_feature`` when always_inline
   callee has incompatible target features with caller.
 - Removed the linking of libraries when ``-r`` is passed to the driver on AIX.



___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [libcxx] [libc++][modules] Improves std.compat module. (PR #76330)

[Chuanqi Xu via llvm-branch-commits]

@@ -17,38 +17,17 @@ module;
 
 // The headers of Table 24: C++ library headers [tab:headers.cpp]
 // and the headers of Table 25: C++ headers for C library facilities 
[tab:headers.cpp.c]
-#include 
-#include 
-#include 
-#if !defined(_LIBCPP_HAS_NO_ATOMIC_HEADER)
-#  include 
-#endif
-#if !defined(_LIBCPP_HAS_NO_THREADS)
-#  include 
-#endif
-#include 
-#include 
 #include 
 #include 

ChuanqiXu9 wrote:

Let's try to remove the duplicated headers as much as possible. Maybe we can 
improve it by something like:

```
export module std.compat;
export import std;

export using double_t = std::double_t;
```

https://github.com/llvm/llvm-project/pull/76330
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [libcxx] [libc++][modules] Improves std.compat module. (PR #76330)

[Chuanqi Xu via llvm-branch-commits]

@@ -17,38 +17,17 @@ module;
 
 // The headers of Table 24: C++ library headers [tab:headers.cpp]
 // and the headers of Table 25: C++ headers for C library facilities 
[tab:headers.cpp.c]
-#include 
-#include 
-#include 
-#if !defined(_LIBCPP_HAS_NO_ATOMIC_HEADER)
-#  include 
-#endif
-#if !defined(_LIBCPP_HAS_NO_THREADS)
-#  include 
-#endif
-#include 
-#include 
 #include 
 #include 

ChuanqiXu9 wrote:

Fair enough.

https://github.com/llvm/llvm-project/pull/76330
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Load Specialization Lazily (2/2) (PR #77417)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 created 
https://github.com/llvm/llvm-project/pull/77417

This is the successor of https://github.com/llvm/llvm-project/pull/76774.

I meant to use spr but I failed. So I created the stacked review here manually. 
Hope this won't be too bad.

The core idea of the patch is: when we see the new specialization for template 
decl in other modules, we don't add the specialization to the update list (so 
it'll be loaded at once if needed). But we add these specializations and the 
corresponding hashed values to a hash table and we wrote the hash table to disk 
instead. So when we read that template decl in other modules, we will associate 
the hash table with that template decl.

In fact, the whole process is exactly the same with how we update the name 
lookup table.

>From 538f9098a49cec659793ba0c679a8e70b6af867c Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Thu, 4 Jan 2024 16:19:05 +0800
Subject: [PATCH] Load Specialization Updates Lazily

---
 .../include/clang/Serialization/ASTBitCodes.h |  2 +
 clang/include/clang/Serialization/ASTReader.h | 11 +++--
 clang/include/clang/Serialization/ASTWriter.h |  7 
 clang/lib/Serialization/ASTCommon.h   |  2 +-
 clang/lib/Serialization/ASTReader.cpp | 24 +--
 clang/lib/Serialization/ASTReaderDecl.cpp | 37 +---
 clang/lib/Serialization/ASTWriter.cpp | 42 +++
 clang/lib/Serialization/ASTWriterDecl.cpp |  7 +++-
 clang/test/Modules/cxx-templates.cpp  |  9 ++--
 .../Serialization/LoadSpecLazily.cpp  | 34 +++
 10 files changed, 140 insertions(+), 35 deletions(-)

diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index 23a279de96ab15..212ae7db30faa0 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -695,6 +695,8 @@ enum ASTRecordTypes {
   /// Record code for an unterminated \#pragma clang assume_nonnull begin
   /// recorded in a preamble.
   PP_ASSUME_NONNULL_LOC = 67,
+
+  UPDATE_SPECIALIZATION = 68,
 };
 
 /// Record types used within a source manager block.
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 293d6495d164ef..ecd4932c16a401 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -610,18 +610,22 @@ class ASTReader
   // Updates for visible decls can occur for other contexts than just the
   // TU, and when we read those update records, the actual context may not
   // be available yet, so have this pending map using the ID as a key. It
-  // will be realized when the context is actually loaded.
-  struct PendingVisibleUpdate {
+  // will be realized when the data is actually loaded.
+  struct UpdateData {
 ModuleFile *Mod;
 const unsigned char *Data;
   };
-  using DeclContextVisibleUpdates = SmallVector;
+  using DeclContextVisibleUpdates = SmallVector;
 
   /// Updates to the visible declarations of declaration contexts that
   /// haven't been loaded yet.
   llvm::DenseMap
   PendingVisibleUpdates;
 
+  using SpecializationsUpdate = SmallVector;
+  llvm::DenseMap
+  PendingSpecializationsUpdates;
+
   /// The set of C++ or Objective-C classes that have forward
   /// declarations that have not yet been linked to their definitions.
   llvm::SmallPtrSet PendingDefinitions;
@@ -650,6 +654,7 @@ class ASTReader
 
   bool ReadSpecializations(ModuleFile &M, llvm::BitstreamCursor &Cursor,
uint64_t Offset, Decl *D);
+  void AddSpecializations(const Decl *D, const unsigned char *Data, ModuleFile 
&M);
 
   /// A vector containing identifiers that have already been
   /// loaded.
diff --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index 09806b87590766..9f689b18652589 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -385,6 +385,9 @@ class ASTWriter : public ASTDeserializationListener,
   /// record containing modifications to them.
   DeclUpdateMap DeclUpdates;
 
+  using SpecializationUpdateMap = llvm::MapVector>;
+  SpecializationUpdateMap SpecializationsUpdates;
+
   using FirstLatestDeclMap = llvm::DenseMap;
 
   /// Map of first declarations from a chained PCH that point to the
@@ -527,6 +530,9 @@ class ASTWriter : public ASTDeserializationListener,
   bool isLookupResultExternal(StoredDeclsList &Result, DeclContext *DC);
   bool isLookupResultEntirelyExternal(StoredDeclsList &Result, DeclContext 
*DC);
 
+  void GenerateSpecializationsLookupTable(const NamedDecl *D,
+llvm::SmallVectorImpl 
&Specs,
+llvm::SmallVectorImpl &LookupTable);
   uint64_t WriteSpecializationsLookupTable(
   const NamedDecl *D,
   llvm::SmallVectorImpl &Specializations);
@@ -5

[llvm-branch-commits] [clang] [Serialization] Load Specialization Lazily (2/2) (PR #77417)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/77417

>From 23184b26934c5a3ba833fb3d7a12adcafb6fad8e Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Thu, 4 Jan 2024 16:19:05 +0800
Subject: [PATCH] Load Specialization Updates Lazily

---
 .../include/clang/Serialization/ASTBitCodes.h |  2 +
 clang/include/clang/Serialization/ASTReader.h | 12 --
 clang/include/clang/Serialization/ASTWriter.h |  8 
 clang/lib/Serialization/ASTCommon.h   |  2 +-
 clang/lib/Serialization/ASTReader.cpp | 26 +--
 clang/lib/Serialization/ASTReaderDecl.cpp | 41 --
 clang/lib/Serialization/ASTWriter.cpp | 43 ---
 clang/lib/Serialization/ASTWriterDecl.cpp |  9 +++-
 clang/test/Modules/cxx-templates.cpp  |  9 ++--
 .../Serialization/LoadSpecLazily.cpp  | 34 +++
 10 files changed, 152 insertions(+), 34 deletions(-)

diff --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index 23a279de96ab15..212ae7db30faa0 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -695,6 +695,8 @@ enum ASTRecordTypes {
   /// Record code for an unterminated \#pragma clang assume_nonnull begin
   /// recorded in a preamble.
   PP_ASSUME_NONNULL_LOC = 67,
+
+  UPDATE_SPECIALIZATION = 68,
 };
 
 /// Record types used within a source manager block.
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 293d6495d164ef..10726b440de515 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -610,18 +610,22 @@ class ASTReader
   // Updates for visible decls can occur for other contexts than just the
   // TU, and when we read those update records, the actual context may not
   // be available yet, so have this pending map using the ID as a key. It
-  // will be realized when the context is actually loaded.
-  struct PendingVisibleUpdate {
+  // will be realized when the data is actually loaded.
+  struct UpdateData {
 ModuleFile *Mod;
 const unsigned char *Data;
   };
-  using DeclContextVisibleUpdates = SmallVector;
+  using DeclContextVisibleUpdates = SmallVector;
 
   /// Updates to the visible declarations of declaration contexts that
   /// haven't been loaded yet.
   llvm::DenseMap
   PendingVisibleUpdates;
 
+  using SpecializationsUpdate = SmallVector;
+  llvm::DenseMap
+  PendingSpecializationsUpdates;
+
   /// The set of C++ or Objective-C classes that have forward
   /// declarations that have not yet been linked to their definitions.
   llvm::SmallPtrSet PendingDefinitions;
@@ -650,6 +654,8 @@ class ASTReader
 
   bool ReadSpecializations(ModuleFile &M, llvm::BitstreamCursor &Cursor,
uint64_t Offset, Decl *D);
+  void AddSpecializations(const Decl *D, const unsigned char *Data,
+  ModuleFile &M);
 
   /// A vector containing identifiers that have already been
   /// loaded.
diff --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index 09806b87590766..0d39f8ace87843 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -385,6 +385,10 @@ class ASTWriter : public ASTDeserializationListener,
   /// record containing modifications to them.
   DeclUpdateMap DeclUpdates;
 
+  using SpecializationUpdateMap =
+  llvm::MapVector>;
+  SpecializationUpdateMap SpecializationsUpdates;
+
   using FirstLatestDeclMap = llvm::DenseMap;
 
   /// Map of first declarations from a chained PCH that point to the
@@ -527,6 +531,9 @@ class ASTWriter : public ASTDeserializationListener,
   bool isLookupResultExternal(StoredDeclsList &Result, DeclContext *DC);
   bool isLookupResultEntirelyExternal(StoredDeclsList &Result, DeclContext 
*DC);
 
+  void GenerateSpecializationsLookupTable(
+  const NamedDecl *D, llvm::SmallVectorImpl &Specs,
+  llvm::SmallVectorImpl &LookupTable);
   uint64_t WriteSpecializationsLookupTable(
   const NamedDecl *D,
   llvm::SmallVectorImpl &Specializations);
@@ -542,6 +549,7 @@ class ASTWriter : public ASTDeserializationListener,
   void WriteReferencedSelectorsPool(Sema &SemaRef);
   void WriteIdentifierTable(Preprocessor &PP, IdentifierResolver &IdResolver,
 bool IsModule);
+  void WriteSpecializationsUpdates();
   void WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord);
   void WriteDeclContextVisibleUpdate(const DeclContext *DC);
   void WriteFPPragmaOptions(const FPOptionsOverride &Opts);
diff --git a/clang/lib/Serialization/ASTCommon.h 
b/clang/lib/Serialization/ASTCommon.h
index 296642e3674a49..9d190c05062444 100644
--- a/clang/lib/Serialization/ASTCommon.h
+++ b/clang/lib/Serialization/ASTCommon.h
@@ -23,7 +23,7 @@ namespace serialization {
 

[llvm-branch-commits] [clang] [Serialization] Load Specialization Lazily (2/2) (PR #77417)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

> @ChuanqiXu9, I managed to push the commit here back to #76774 and we can 
> continue the discussion there. Would that be sufficient?

The intention of split the patch is to ease the reviewing. If you are glad to 
do that, yes, it will be pretty good : )

https://github.com/llvm/llvm-project/pull/77417
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Load Specialization Lazily (2/2) (PR #77417)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 closed 
https://github.com/llvm/llvm-project/pull/77417
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Load Specialization Lazily (2/2) (PR #77417)

[Chuanqi Xu via llvm-branch-commits]

ChuanqiXu9 wrote:

Then this is not needed as far as I understood.

https://github.com/llvm/llvm-project/pull/77417
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [libcxx] [libc++][modules] Adds module testing. (PR #76246)

[Chuanqi Xu via llvm-branch-commits]

@@ -131,13 +138,65 @@ def parseScript(test, preamble):
 script += preamble
 script += scriptInTest
 
+has_std_module = False
+has_std_compat_module = False
+for module in modules:
+if module == "std":
+has_std_module = True
+elif module == "std.compat":
+has_std_compat_module = True
+else:
+script.insert(
+0,
+f"echo \"The module '{module}' is not valid, use 'std' or 
'std.compat'\"",
+)
+script.insert(1, "false")
+return script
+
+if modules:
+# This flag is needed for both modules.
+moduleCompileFlags.append("-fprebuilt-module-path=%T")

ChuanqiXu9 wrote:

I didn't read the patch completely so I don't know the concatenated command 
line. But the behavior matches current expectation of clang. Now when clang 
sees an `import` for a named module, clang will only find that named module by 
`-fprebuilt-module-path` or 
`-fmodule-file==`.

I guess you're talking about the case about:

```
clang++ std.compat.pcm use.cc -o use
```

where use.cc uses std.compat module. 

This command line will be split into 3 processes like:

```
clang_cc1 std.compat.pcm  -c -o std.compat.o
clang_cc1 use.cc -c -o use.o
clang_cc1 std.compat.o use.o -o use
```

So the second step won't know about `std.compat.pcm`.

While it is technically possible to improve that, I feel it is not so useful 
since the major expected users of modules should use a build system instead of 
concatenating the command line manually.

https://github.com/llvm/llvm-project/pull/76246
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [backport] [C++20] [Modules] Backport the ability to skip ODR checks in GMF (PR #80249)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 created 
https://github.com/llvm/llvm-project/pull/80249

The backport follows the new practice suggested in 
https://discourse.llvm.org/t/release-18-x-branch-has-been-created/76480.

See https://github.com/llvm/llvm-project/issues/79240 and 
https://github.com/llvm/llvm-project/pull/79959 for the full context.

This is pretty helpful to improve the user experiences in modules given there 
are a lot of issue reports about false positive ODR violation diagnostics.

>From 85dc0ff79515cc439cc3e0d8c991709ad789a50b Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Mon, 29 Jan 2024 11:42:08 +0800
Subject: [PATCH 1/3] [C++20] [Modules] Don't perform ODR checks in GMF

Close https://github.com/llvm/llvm-project/issues/79240.

See the linked issue for details. Given the frequency of issue reporting
about false positive ODR checks (I received private issue reports too),
I'd like to backport this to 18.x too.
---
 clang/docs/ReleaseNotes.rst   |  5 ++
 clang/include/clang/Serialization/ASTReader.h |  4 ++
 clang/lib/Serialization/ASTReader.cpp |  3 ++
 clang/lib/Serialization/ASTReaderDecl.cpp | 37 +
 clang/lib/Serialization/ASTWriter.cpp |  8 ++-
 clang/lib/Serialization/ASTWriterDecl.cpp | 13 +++--
 clang/test/Modules/concept.cppm   | 14 ++---
 clang/test/Modules/no-eager-load.cppm | 53 ---
 clang/test/Modules/polluted-operator.cppm |  8 ++-
 clang/test/Modules/pr76638.cppm   |  6 +--
 10 files changed, 68 insertions(+), 83 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 060bc7669b72a..e8dfdfa63717c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -188,6 +188,11 @@ C++20 Feature Support
   This feature is still experimental. Accordingly, 
``__cpp_nontype_template_args`` was not updated.
   However, its support can be tested with 
``__has_extension(cxx_generalized_nttp)``.
 
+- Clang won't perform ODR checks for decls in the global module fragment any
+  more to ease the implementation and improve the user's using experience.
+  This follows the MSVC's behavior.
+  (`#79240 `_).
+
 C++23 Feature Support
 ^
 - Implemented `P0847R7: Deducing this `_. Some 
related core issues were also
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index dd1451bbf2d2c..ba06ab0cd4509 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2452,6 +2452,10 @@ class BitsUnpacker {
   uint32_t CurrentBitsIndex = ~0;
 };
 
+inline bool isFromExplicitGMF(const Decl *D) {
+  return D->getOwningModule() && 
D->getOwningModule()->isExplicitGlobalModule();
+}
+
 } // namespace clang
 
 #endif // LLVM_CLANG_SERIALIZATION_ASTREADER_H
diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index fecd94e875f67..e91c5fe08a043 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9743,6 +9743,9 @@ void ASTReader::finishPendingActions() {
 
 if (!FD->isLateTemplateParsed() &&
 !NonConstDefn->isLateTemplateParsed() &&
+// We only perform ODR checks for decls not in the explicit
+// global module fragment.
+!isFromExplicitGMF(FD) &&
 FD->getODRHash() != NonConstDefn->getODRHash()) {
   if (!isa(FD)) {
 PendingFunctionOdrMergeFailures[FD].push_back(NonConstDefn);
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
b/clang/lib/Serialization/ASTReaderDecl.cpp
index a149d82153037..7697f29b9054b 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -804,8 +804,10 @@ void ASTDeclReader::VisitEnumDecl(EnumDecl *ED) {
   ED->setScopedUsingClassTag(EnumDeclBits.getNextBit());
   ED->setFixed(EnumDeclBits.getNextBit());
 
-  ED->setHasODRHash(true);
-  ED->ODRHash = Record.readInt();
+  if (!isFromExplicitGMF(ED)) {
+ED->setHasODRHash(true);
+ED->ODRHash = Record.readInt();
+  }
 
   // If this is a definition subject to the ODR, and we already have a
   // definition, merge this one into it.
@@ -827,7 +829,9 @@ void ASTDeclReader::VisitEnumDecl(EnumDecl *ED) {
   Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef));
   ED->demoteThisDefinitionToDeclaration();
   Reader.mergeDefinitionVisibility(OldDef, ED);
-  if (OldDef->getODRHash() != ED->getODRHash())
+  // We don't want to check the ODR hash value for declarations from global
+  // module fragment.
+  if (!isFromExplicitGMF(ED) && OldDef->getODRHash() != ED->getODRHash())
 Reader.PendingEnumOdrMergeFailures[OldDef].push_back(ED);
 } else {
   OldDef = ED;
@@ -866,6 +870,9 @@ ASTDeclReader::VisitRecordDeclImpl(Rec

[llvm-branch-commits] [clang] [backport] [C++20] [Modules] Backport the ability to skip ODR checks in GMF (PR #80249)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 milestoned 
https://github.com/llvm/llvm-project/pull/80249
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [llvm] [backport] [C++20] [Modules] Backport the ability to skip ODR checks in GMF (PR #80249)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/80249

>From 85dc0ff79515cc439cc3e0d8c991709ad789a50b Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Mon, 29 Jan 2024 11:42:08 +0800
Subject: [PATCH 1/3] [C++20] [Modules] Don't perform ODR checks in GMF

Close https://github.com/llvm/llvm-project/issues/79240.

See the linked issue for details. Given the frequency of issue reporting
about false positive ODR checks (I received private issue reports too),
I'd like to backport this to 18.x too.
---
 clang/docs/ReleaseNotes.rst   |  5 ++
 clang/include/clang/Serialization/ASTReader.h |  4 ++
 clang/lib/Serialization/ASTReader.cpp |  3 ++
 clang/lib/Serialization/ASTReaderDecl.cpp | 37 +
 clang/lib/Serialization/ASTWriter.cpp |  8 ++-
 clang/lib/Serialization/ASTWriterDecl.cpp | 13 +++--
 clang/test/Modules/concept.cppm   | 14 ++---
 clang/test/Modules/no-eager-load.cppm | 53 ---
 clang/test/Modules/polluted-operator.cppm |  8 ++-
 clang/test/Modules/pr76638.cppm   |  6 +--
 10 files changed, 68 insertions(+), 83 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 060bc7669b72a..e8dfdfa63717c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -188,6 +188,11 @@ C++20 Feature Support
   This feature is still experimental. Accordingly, 
``__cpp_nontype_template_args`` was not updated.
   However, its support can be tested with 
``__has_extension(cxx_generalized_nttp)``.
 
+- Clang won't perform ODR checks for decls in the global module fragment any
+  more to ease the implementation and improve the user's using experience.
+  This follows the MSVC's behavior.
+  (`#79240 `_).
+
 C++23 Feature Support
 ^
 - Implemented `P0847R7: Deducing this `_. Some 
related core issues were also
diff --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index dd1451bbf2d2c..ba06ab0cd4509 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2452,6 +2452,10 @@ class BitsUnpacker {
   uint32_t CurrentBitsIndex = ~0;
 };
 
+inline bool isFromExplicitGMF(const Decl *D) {
+  return D->getOwningModule() && 
D->getOwningModule()->isExplicitGlobalModule();
+}
+
 } // namespace clang
 
 #endif // LLVM_CLANG_SERIALIZATION_ASTREADER_H
diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index fecd94e875f67..e91c5fe08a043 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9743,6 +9743,9 @@ void ASTReader::finishPendingActions() {
 
 if (!FD->isLateTemplateParsed() &&
 !NonConstDefn->isLateTemplateParsed() &&
+// We only perform ODR checks for decls not in the explicit
+// global module fragment.
+!isFromExplicitGMF(FD) &&
 FD->getODRHash() != NonConstDefn->getODRHash()) {
   if (!isa(FD)) {
 PendingFunctionOdrMergeFailures[FD].push_back(NonConstDefn);
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
b/clang/lib/Serialization/ASTReaderDecl.cpp
index a149d82153037..7697f29b9054b 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -804,8 +804,10 @@ void ASTDeclReader::VisitEnumDecl(EnumDecl *ED) {
   ED->setScopedUsingClassTag(EnumDeclBits.getNextBit());
   ED->setFixed(EnumDeclBits.getNextBit());
 
-  ED->setHasODRHash(true);
-  ED->ODRHash = Record.readInt();
+  if (!isFromExplicitGMF(ED)) {
+ED->setHasODRHash(true);
+ED->ODRHash = Record.readInt();
+  }
 
   // If this is a definition subject to the ODR, and we already have a
   // definition, merge this one into it.
@@ -827,7 +829,9 @@ void ASTDeclReader::VisitEnumDecl(EnumDecl *ED) {
   Reader.MergedDeclContexts.insert(std::make_pair(ED, OldDef));
   ED->demoteThisDefinitionToDeclaration();
   Reader.mergeDefinitionVisibility(OldDef, ED);
-  if (OldDef->getODRHash() != ED->getODRHash())
+  // We don't want to check the ODR hash value for declarations from global
+  // module fragment.
+  if (!isFromExplicitGMF(ED) && OldDef->getODRHash() != ED->getODRHash())
 Reader.PendingEnumOdrMergeFailures[OldDef].push_back(ED);
 } else {
   OldDef = ED;
@@ -866,6 +870,9 @@ ASTDeclReader::VisitRecordDeclImpl(RecordDecl *RD) {
 
 void ASTDeclReader::VisitRecordDecl(RecordDecl *RD) {
   VisitRecordDeclImpl(RD);
+  // We should only reach here if we're in C/Objective-C. There is no
+  // global module fragment.
+  assert(!isFromExplicitGMF(RD));
   RD->setODRHash(Record.readInt());
 
   // Maintain the invariant of a redeclaration chain containing only
@@ -1094,8 +1101,10 @@ void ASTDeclReader::VisitFunctionDecl(Funct

[llvm-branch-commits] [llvm] [clang] [backport] [C++20] [Modules] Backport the ability to skip ODR checks in GMF (PR #80249)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 edited 
https://github.com/llvm/llvm-project/pull/80249
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[llvm-branch-commits] [clang] [Serialization] Code cleanups and polish 83233 (PR #83237)

[Chuanqi Xu via llvm-branch-commits]

https://github.com/ChuanqiXu9 updated 
https://github.com/llvm/llvm-project/pull/83237

>From f2e53e44eebab4720a1dbade24fcb14d698fb03f Mon Sep 17 00:00:00 2001
From: Chuanqi Xu 
Date: Wed, 28 Feb 2024 11:41:53 +0800
Subject: [PATCH 1/3] [Serialization] Code cleanups and polish 83233

---
 clang/include/clang/AST/DeclTemplate.h|  39 +-
 clang/include/clang/AST/ExternalASTSource.h   |   8 +-
 .../clang/Sema/MultiplexExternalSemaSource.h  |   4 +-
 .../include/clang/Serialization/ASTBitCodes.h |   2 +-
 clang/include/clang/Serialization/ASTReader.h |   4 +-
 clang/lib/AST/DeclTemplate.cpp|  85 ++--
 clang/lib/AST/ExternalASTSource.cpp   |  10 +-
 clang/lib/AST/ODRHash.cpp |  10 -
 .../lib/Sema/MultiplexExternalSemaSource.cpp  |  13 +-
 clang/lib/Serialization/ASTCommon.h   |   1 -
 clang/lib/Serialization/ASTReader.cpp |  42 +-
 clang/lib/Serialization/ASTReaderDecl.cpp |  76 +---
 clang/lib/Serialization/ASTReaderInternals.h  |   1 -
 clang/lib/Serialization/ASTWriter.cpp |  27 +-
 clang/lib/Serialization/ASTWriterDecl.cpp |  52 +--
 clang/lib/Serialization/CMakeLists.txt|   1 +
 .../Serialization/TemplateArgumentHasher.cpp  | 423 ++
 .../Serialization/TemplateArgumentHasher.h|  34 ++
 clang/test/Modules/cxx-templates.cpp  |   8 +-
 .../Modules/recursive-instantiations.cppm |  40 ++
 .../test/OpenMP/target_parallel_ast_print.cpp |   4 -
 clang/test/OpenMP/target_teams_ast_print.cpp  |   4 -
 clang/test/OpenMP/task_ast_print.cpp  |   4 -
 clang/test/OpenMP/teams_ast_print.cpp |   4 -
 24 files changed, 610 insertions(+), 286 deletions(-)
 create mode 100644 clang/lib/Serialization/TemplateArgumentHasher.cpp
 create mode 100644 clang/lib/Serialization/TemplateArgumentHasher.h
 create mode 100644 clang/test/Modules/recursive-instantiations.cppm

diff --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
index 44f840d297465d..7406252363d223 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -256,9 +256,6 @@ class TemplateArgumentList final
   TemplateArgumentList(const TemplateArgumentList &) = delete;
   TemplateArgumentList &operator=(const TemplateArgumentList &) = delete;
 
-  /// Create hash for the given arguments.
-  static unsigned ComputeODRHash(ArrayRef Args);
-
   /// Create a new template argument list that copies the given set of
   /// template arguments.
   static TemplateArgumentList *CreateCopy(ASTContext &Context,
@@ -732,25 +729,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
   }
 
   void anchor() override;
-  struct LazySpecializationInfo {
-GlobalDeclID DeclID = GlobalDeclID();
-unsigned ODRHash = ~0U;
-bool IsPartial = false;
-LazySpecializationInfo(GlobalDeclID ID, unsigned Hash = ~0U,
-   bool Partial = false)
-: DeclID(ID), ODRHash(Hash), IsPartial(Partial) {}
-LazySpecializationInfo() {}
-bool operator<(const LazySpecializationInfo &Other) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo &Other) const {
-  assert((DeclID != Other.DeclID || ODRHash == Other.ODRHash) &&
- "Hashes differ!");
-  assert((DeclID != Other.DeclID || IsPartial == Other.IsPartial) &&
- "Both must be the same kinds!");
-  return DeclID == Other.DeclID;
-}
-  };
 
 protected:
   template  struct SpecEntryTraits {
@@ -794,16 +772,20 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 
   void loadLazySpecializationsImpl(bool OnlyPartial = false) const;
 
-  void loadLazySpecializationsImpl(llvm::ArrayRef Args,
+  bool loadLazySpecializationsImpl(llvm::ArrayRef Args,
TemplateParameterList *TPL = nullptr) const;
 
-  Decl *loadLazySpecializationImpl(LazySpecializationInfo &LazySpecInfo) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector &Specs,
  void *&InsertPos, ProfileArguments &&...ProfileArgs);
 
+  template 
+  typename SpecEntryTraits::DeclType *
+  findSpecializationLocally(llvm::FoldingSetVector &Specs,
+void *&InsertPos,
+ProfileArguments &&...ProfileArgs);
+
   template 
   void addSpecializationImpl(llvm::FoldingSetVector &Specs,
  EntryType *Entry, void *InsertPos);
@@ -819,13 +801,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
 llvm::PointerIntPair
   InstantiatedFromMember;
 
-/// If non-null, points to an array of specializations (including
-/// partial specializations) known only by their external declaration IDs.
-///
-/// The first value in the array is the number of specializations/partial
-/// specializations that follow.
-LazySpecializationInfo *LazySpecializations = n