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

2024-08-01 Thread Ilya Biryukov via llvm-branch-commits

ilya-biryukov wrote:

We've hit an error during testing:
```cpp
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.

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)

2024-08-01 Thread Alexander Kornienko via llvm-branch-commits

alexfh wrote:

> bool operator==(const LazySpecializationInfo )

Making this `operator==` `const` helps with the warning in C++20. And it's the 
right thing to do anyway.

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)

2024-07-31 Thread 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 ) {
>   |^
> 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)

2024-07-31 Thread Alexander Kornienko via llvm-branch-commits

alexfh wrote:

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 ) {
  |^
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.

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)

2024-07-31 Thread Ilya Biryukov via llvm-branch-commits

ilya-biryukov wrote:

> In the meantime, could you rebase the patch on top of current ToT?

I thought that it should not have any merge conflicts and it turned out to be 
almost true, the only conflict I encountered was in `CMakeLists.txt`, feel free 
to use this squashed and rebased commit for testing from my fork: 
https://github.com/ilya-biryukov/llvm-project/commit/e6329b9390445369c537aa6aa5a5cd64ba27e19c


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)

2024-07-31 Thread Alexander Kornienko via llvm-branch-commits

alexfh 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?

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)

2024-07-31 Thread Ilya Biryukov via llvm-branch-commits

ilya-biryukov 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.

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)

2024-07-10 Thread 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 =(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 ,
@@ -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 ) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo ) 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 ) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector ,
  void *, ProfileArguments &&...ProfileArgs);
 
+  template 
+  typename SpecEntryTraits::DeclType *
+  findSpecializationLocally(llvm::FoldingSetVector ,
+void *,
+ProfileArguments &&...ProfileArgs);
+
   template 
   void addSpecializationImpl(llvm::FoldingSetVector ,
  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;
-
 /// The set of "injected" template arguments used within this
 

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

2024-07-10 Thread 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)

2024-07-10 Thread 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)

2024-07-10 Thread 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 =(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 ,
@@ -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 ) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo ) 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 ) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector ,
  void *, ProfileArguments &&...ProfileArgs);
 
+  template 
+  typename SpecEntryTraits::DeclType *
+  findSpecializationLocally(llvm::FoldingSetVector ,
+void *,
+ProfileArguments &&...ProfileArgs);
+
   template 
   void addSpecializationImpl(llvm::FoldingSetVector ,
  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;
-
 /// The set of "injected" template arguments used within this
 /// 

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

2024-07-05 Thread 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] Code cleanups and polish 83233 (PR #83237)

2024-05-06 Thread Ilya Biryukov via llvm-branch-commits

ilya-biryukov wrote:

After endless hours and recompilations, I finally have this to share.
It could probably be smaller, but I decided to share this as soon as it crashed 
outside of our monorepo. 

[module.tgz](https://github.com/llvm/llvm-project/files/15223606/module.tgz)
^^ If you run build.sh with Clang 16, it builds. However, it fails with the 
current patch. 
See out/errors for a list of errors I get on my machine. 
This breaks with libstdc++ 13, but also latest libc++ (which is what we use in 
our monorepo).

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)

2024-04-29 Thread via llvm-branch-commits

alexfh wrote:

Apologies for the delay with the reproducer. @ilya-biryukov is on vacation, but 
maybe @sam-mccall can help with an isolated test case. Either way it seems to 
be challenging.

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)

2024-04-25 Thread 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] Code cleanups and polish 83233 (PR #83237)

2024-04-25 Thread 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 =(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 ) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo ) 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 ) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector ,
  void *, ProfileArguments &&...ProfileArgs);
 
+  template 
+  typename SpecEntryTraits::DeclType *
+  findSpecializationLocally(llvm::FoldingSetVector ,
+void *,
+ProfileArguments &&...ProfileArgs);
+
   template 
   void addSpecializationImpl(llvm::FoldingSetVector ,
  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
   LoadExternalSpecializations(const Decl *D,
 

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

2024-04-15 Thread 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] Code cleanups and polish 83233 (PR #83237)

2024-04-15 Thread Vassil Vassilev via llvm-branch-commits

vgvassilev wrote:

> > Can you export all files into a standalone reproducer? I might be able to 
> > reduce an example.
> 
> Not really, this is why it's taking so long. Our infrastructure in that space 
> is lacking, the issue is that the root case is not in one compilation step, 
> but rather in some of the dependencies and the dependency graph for any of 
> those problems is really large.
> 
> We will get you a reproducer, please bear with us. Sorry for taking a long 
> time.

Back in the day when we were more active developing modules we wrote this tool: 
https://github.com/Teemperor/hippie It hooks to the system calls and copies all 
files involved in a crash/miscompilation under a common sysroot preserving the 
file structure. Maybe that'd help...

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)

2024-04-15 Thread Ilya Biryukov via llvm-branch-commits

ilya-biryukov wrote:

> Can you export all files into a standalone reproducer? I might be able to 
> reduce an example.

Not really, this is why it's taking so long. Our infrastructure in that space 
is lacking, the issue is that the root case is not in one compilation step, but 
rather in some of the dependencies and the dependency graph for any of those 
problems is really large.

We will get you a reproducer, please bear with us. Sorry for taking a long time.

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)

2024-04-12 Thread Vassil Vassilev via llvm-branch-commits

vgvassilev wrote:

Can you export all files into a standalone reproducer? I might be able to 
reduce an example. 

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)

2024-04-12 Thread Ilya Biryukov via llvm-branch-commits

ilya-biryukov 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?

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)

2024-04-09 Thread Ilya Biryukov via llvm-branch-commits

ilya-biryukov wrote:

We have hit quite a few issues when trying this out. I have spent a day trying 
to reduce to a small repro that I can share, but haven't not fully succeeded 
yet, I will continue tomorrow.
Still wanted to share the error descriptions in case that would allow to make 
progress in the meanwhile.

* Crash with the following assertion failure:

```
assert.h assertion failed at 
third_party/llvm/llvm-project/clang/include/clang/AST/DeclCXX.h:464 in struct 
DefinitionData ::CXXRecordDecl::data() const: DD && "queried property of 
class with no definition"
*** Check failure stack trace: ***
@ 0x55fb619aaea4  absl::log_internal::LogMessage::SendToLog()
@ 0x55fb619aad04  absl::log_internal::LogMessage::Flush()
@ 0x55fb619ab209  
absl::log_internal::LogMessageFatal::~LogMessageFatal()
@ 0x55fb6199a3e4  __assert_fail
@ 0x55fb5bd8d400  clang::CXXRecordDecl::data()
@ 0x55fb5e24de0d  clang::CXXBasePaths::lookupInBases()
@ 0x55fb5e24e388  clang::CXXBasePaths::lookupInBases()
@ 0x55fb5e24d55c  clang::CXXRecordDecl::lookupInBases()
@ 0x55fb5da8bd60  clang::Sema::LookupQualifiedName()
@ 0x55fb5dc747f3  clang::Sema::CheckTypenameType()
@ 0x55fb5de19e72  clang::TreeTransform<>::TransformDependentNameType()
@ 0x55fb5ddee35c  clang::TreeTransform<>::TransformType()
@ 0x55fb5ddedd3f  clang::TreeTransform<>::TransformType()
@ 0x55fb5dded94d  clang::Sema::SubstType()
@ 0x55fb5dc62c9f  SubstDefaultTemplateArgument()
@ 0x55fb5dc58761  clang::Sema::CheckTemplateArgumentList()
@ 0x55fb5dc5720b  clang::Sema::CheckTemplateIdType()
@ 0x55fb5de19086  
clang::TreeTransform<>::TransformTemplateSpecializationType()
@ 0x55fb5de2115d  
clang::TreeTransform<>::TransformTemplateSpecializationType()
@ 0x55fb5ddee280  clang::TreeTransform<>::TransformType()
@ 0x55fb5de1dc1a  clang::TreeTransform<>::TransformElaboratedType()
@ 0x55fb5ddee3c4  clang::TreeTransform<>::TransformType()
@ 0x55fb5ddedd3f  clang::TreeTransform<>::TransformType()
@ 0x55fb5dded94d  clang::Sema::SubstType()
@ 0x55fb5dc62c9f  SubstDefaultTemplateArgument()
@ 0x55fb5dc62632  clang::Sema::SubstDefaultTemplateArgumentIfAvailable()
@ 0x55fb5dd7056b  clang::Sema::FinishTemplateArgumentDeduction()
@ 0x55fb5dde5796  llvm::function_ref<>::callback_fn<>()
@ 0x55fb5d420dcf  clang::Sema::runWithSufficientStackSpace()
@ 0x55fb5dd729f4  clang::Sema::DeduceTemplateArguments()
@ 0x55fb5dbc00d5  clang::Sema::AddTemplateOverloadCandidate()
@ 0x55fb5da7065d  ResolveConstructorOverload()
@ 0x55fb5da52dd8  TryConstructorInitialization()
@ 0x55fb5da50e7a  TryDefaultInitialization()
@ 0x55fb5da4e6b1  clang::InitializationSequence::InitializeFrom()
@ 0x55fb5d72c3ed  CollectFieldInitializer()
@ 0x55fb5d72a972  clang::Sema::SetCtorInitializers()
@ 0x55fb5d72ef65  clang::Sema::ActOnDefaultCtorInitializers()
@ 0x55fb5d2401b0  clang::Parser::ParseLexedMethodDef()
@ 0x55fb5d23e77a  clang::Parser::ParseLexedMethodDefs()
@ 0x55fb5d1b50e0  clang::Parser::ParseCXXMemberSpecification()
@ 0x55fb5d1b2744  clang::Parser::ParseClassSpecifier()
@ 0x55fb5d1d8ef0  clang::Parser::ParseDeclarationSpecifiers()
@ 0x55fb5d15ea4a  clang::Parser::ParseDeclOrFunctionDefInternal()
@ 0x55fb5d15e5b6  clang::Parser::ParseDeclarationOrFunctionDefinition()
@ 0x55fb5d15d2ea  clang::Parser::ParseExternalDeclaration()
@ 0x55fb5d1a8c73  clang::Parser::ParseInnerNamespace()
@ 0x55fb5d1a7e4f  clang::Parser::ParseNamespace()
@ 0x55fb5d1d2bde  clang::Parser::ParseDeclaration()
@ 0x55fb5d15ceb6  clang::Parser::ParseExternalDeclaration()
@ 0x55fb5d1a8c73  clang::Parser::ParseInnerNamespace()
@ 0x55fb5d1a7e4f  clang::Parser::ParseNamespace()
@ 0x55fb5d1d2bde  clang::Parser::ParseDeclaration()
@ 0x55fb5d15ceb6  clang::Parser::ParseExternalDeclaration()
@ 0x55fb5d15b04b  clang::Parser::ParseTopLevelDecl()
@ 0x55fb5d154d2e  clang::ParseAST()
@ 0x55fb5cea31e3  clang::FrontendAction::Execute()
@ 0x55fb5ce1c62d  clang::CompilerInstance::ExecuteAction()
@ 0x55fb5bd4a20e  clang::ExecuteCompilerInvocation()
@ 0x55fb5bd3e106  cc1_main()
@ 0x55fb5bd3b9a6  ExecuteCC1Tool()
```

* Invalid errors about absence or presence of certain members in 
std::pointer_traits and other types:

```
third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__memory/pointer_traits.h:161:3:
 error: 'std::pointer_traits::pointer_to' 
from module '/
/third_party/crosstool/v18/stable:stl_cc_library.third_party/stl/cxx17/optional'
 is not present in definition of 'std::pointer_traits' provided earlier
  161 |   pointer_to(__conditional_t::value, __nat, 

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

2024-04-08 Thread Ilya Biryukov via llvm-branch-commits

ilya-biryukov wrote:

Thanks for fixing it quickly, we'll try the new patch and get back with the 
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] Code cleanups and polish 83233 (PR #83237)

2024-04-07 Thread 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)

2024-04-07 Thread 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 =(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 ) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo ) 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 ) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector ,
  void *, ProfileArguments &&...ProfileArgs);
 
+  template 
+  typename SpecEntryTraits::DeclType *
+  findSpecializationLocally(llvm::FoldingSetVector ,
+void *,
+ProfileArguments &&...ProfileArgs);
+
   template 
   void addSpecializationImpl(llvm::FoldingSetVector ,
  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
   LoadExternalSpecializations(const Decl *D,
   ArrayRef 

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

2024-04-05 Thread Ilya Biryukov via llvm-branch-commits

ilya-biryukov wrote:

Sorry for the delay, I had an emergency and took a week off.
We have hit some form of OOM (an infinite loop or some exponential computation 
allocating memory?)  on the following small example (ready as a test case, also 
in [this 
commit](https://github.com/ilya-biryukov/llvm-project/commit/6e2f55a1d1fa90b325ca2891c2a101bc79a32b9d)):

```cpp
// RUN: rm -rf %t
// RUN: mkdir -p %t
// RUN: split-file %s %t
//
// RUN: %clang_cc1 -std=c++20 %t/type_traits.cppm -emit-module-interface -o 
%t/type_traits.pcm
// RUN: %clang_cc1 -std=c++20 %t/test.cpp -fprebuilt-module-path=%t -verify

//--- type_traits.cppm
export module type_traits;

export template 
constexpr bool is_pod_v = __is_pod(T);

//--- test.cpp
import type_traits;
// Base is either void or wrapper.
template  struct wrapper : Base {};
template <> struct wrapper {};

// wrap<0>::type is wrapper, wrap<1>::type is wrapper>,
// and so on.
template 
struct wrap {
  template 
  using type = wrapper::template type>;
};

template <>
struct wrap<0> {
  template 
  using type = wrapper;
};

inline constexpr int kMaxRank = 40;
template 
using rank = typename wrap::template type;
using rank_selector_t = rank<40>;

static_assert(is_pod_v, "Must be POD");
```

There might be more problems as this one has been caught early and blocked 
further progress.

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)

2024-04-01 Thread Vassil Vassilev via llvm-branch-commits

vgvassilev wrote:

> Btw, if I don't respond timely to these requests and you need an urgent 
> reaction, feel free to ping me via email at 
> [ibiryu...@google.com](mailto:ibiryu...@google.com). We heavily use Clang 
> header modules internally at Google and we are really interested in helping 
> to discover and prevent the module bugs as early as possible. And we also 
> really appreciate your work on improving and optimizing the modules 
> implementation!

@ilya-biryukov, I was wondering if you have had a chance to test this PR?

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)

2024-03-22 Thread Ilya Biryukov via llvm-branch-commits

ilya-biryukov wrote:

Btw, if I don't respond timely to these requests and you need an urgent 
reaction, feel free to ping me via email at ibiryu...@google.com.
We heavily use Clang header modules internally at Google and we are really 
interested in helping to discover and prevent the module bugs as early as 
possible. And we also really appreciate your work on improving and optimizing 
the modules implementation!

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)

2024-03-22 Thread Ilya Biryukov via llvm-branch-commits

ilya-biryukov wrote:

We'll pick this up, I'll get back with the testing results.
(Sorry for the delayed response)

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)

2024-03-21 Thread Vassil Vassilev via llvm-branch-commits

vgvassilev wrote:

> @ilya-biryukov hi, the functional and performance test on the root side looks 
> good: [root-project/root#14495 
> (comment)](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: 
> [ChuanqiXu9/root@570fd78](https://github.com/ChuanqiXu9/root/commit/570fd783875671d346c7cdc0d98a8a9afcad05a4)

@ilya-biryukov, can you test this PR on your side. We'd like to move forward.

cc: @dwblaikie 

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)

2024-03-06 Thread 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] [Serialization] Code cleanups and polish 83233 (PR #83237)

2024-03-05 Thread 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 =(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 ) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo ) 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 ) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector ,
  void *, ProfileArguments &&...ProfileArgs);
 
+  template 
+  typename SpecEntryTraits::DeclType *
+  findSpecializationLocally(llvm::FoldingSetVector ,
+void *,
+ProfileArguments &&...ProfileArgs);
+
   template 
   void addSpecializationImpl(llvm::FoldingSetVector ,
  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
   LoadExternalSpecializations(const Decl *D,
   ArrayRef 

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

2024-03-04 Thread 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 =(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 ) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo ) 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 ) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector ,
@@ -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 
{
 
   void 

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

2024-03-01 Thread 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] Code cleanups and polish 83233 (PR #83237)

2024-03-01 Thread Vassil Vassilev via llvm-branch-commits

vgvassilev 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.

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)

2024-02-29 Thread 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)

2024-02-29 Thread Vassil Vassilev via llvm-branch-commits

vgvassilev wrote:

> @vgvassilev this may be ready to test.

I triggered a test here: https://github.com/root-project/root/pull/14495 

I still need to verify if I did not screw up bringing your changes back to our 
builds but locally it crashes quite early with:



```
0  rootcling_stage1 0x0001091d1c9d 
llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 61
1  rootcling_stage1 0x0001091d221b 
PrintStackTraceSignalHandler(void*) + 27
2  rootcling_stage1 0x0001091cff06 llvm::sys::RunSignalHandlers() + 
134
3  rootcling_stage1 0x0001091d480f SignalHandler(int) + 223
4  libsystem_platform.dylib 0x7ff800ad137d _sigtramp + 29
5  rootcling_stage1 0x000100059595 
llvm::SmallVectorTemplateCommon::assertSafeToReferenceAfterResize(void const*, unsigned long) + 37
6  libsystem_c.dylib0x7ff8009c1a49 abort + 126
7  libsystem_c.dylib0x7ff8009c0d30 err + 0
8  rootcling_stage1 0x0001019959f0 
llvm::SmallVectorTemplateCommon::operator[](unsigned 
long) const + 96
9  rootcling_stage1 0x0001017d43d9 
clang::ASTReader::ReadString(llvm::SmallVector const&, 
unsigned int&) + 57
10 rootcling_stage1 0x0001017e0f0b 
clang::ASTReader::ParseLanguageOptions(llvm::SmallVector const&, bool, clang::ASTReaderListener&, bool) + 27691
11 rootcling_stage1 0x0001017da096 
clang::ASTReader::ReadOptionsBlock(llvm::BitstreamCursor&, unsigned int, bool, 
clang::ASTReaderListener&, std::__1::basic_string, std::__1::allocator>&) + 758
12 rootcling_stage1 0x0001017e2537 
clang::ASTReader::ReadControlBlock(clang::serialization::ModuleFile&, 
llvm::SmallVectorImpl&, 
clang::serialization::ModuleFile const*, unsigned int) + 2487
13 rootcling_stage1 0x0001017e487e 
clang::ASTReader::ReadASTCore(llvm::StringRef, 
clang::serialization::ModuleKind, clang::SourceLocation, 
clang::serialization::ModuleFile*, 
llvm::SmallVectorImpl&, long long, long, 
clang::ASTFileSignature, unsigned int) + 2110
14 rootcling_stage1 0x0001017efae3 
clang::ASTReader::ReadAST(llvm::StringRef, clang::serialization::ModuleKind, 
clang::SourceLocation, unsigned int, 
llvm::SmallVectorImpl*) + 627
15 rootcling_stage1 0x000101249f53 
clang::CompilerInstance::findOrCompileModuleAndReadAST(llvm::StringRef, 
clang::SourceLocation, clang::SourceLocation, bool) + 1443
16 rootcling_stage1 0x00010124b4be 
clang::CompilerInstance::loadModule(clang::SourceLocation, 
llvm::ArrayRef>, 
clang::Module::NameVisibilityKind, bool) + 1006
17 rootcling_stage1 0x000103f6a144 
clang::Preprocessor::HandleHeaderIncludeOrImport(clang::SourceLocation, 
clang::Token&, clang::Token&, clang::SourceLocation, 
clang::detail::SearchDirIteratorImpl, clang::FileEntry const*) + 3348
18 rootcling_stage1 0x000103f648ec 
clang::Preprocessor::HandleIncludeDirective(clang::SourceLocation, 
clang::Token&, clang::detail::SearchDirIteratorImpl, clang::FileEntry 
const*) + 316
19 rootcling_stage1 0x000103f65064 
clang::Preprocessor::HandleDirective(clang::Token&) + 1412
20 rootcling_stage1 0x000103f07de8 
clang::Lexer::LexTokenInternal(clang::Token&, bool) + 10120
21 rootcling_stage1 0x000103f032ce clang::Lexer::Lex(clang::Token&) 
+ 270
22 rootcling_stage1 0x000103fd3cbf 
clang::Preprocessor::Lex(clang::Token&) + 111
23 rootcling_stage1 0x0001005e1669 
clang::Parser::ConsumeAnnotationToken() + 169
24 rootcling_stage1 0x0001017ab553 
clang::Parser::ParseTopLevelDecl(clang::OpaquePtr&, 
clang::Sema::ModuleImportState&) + 1475
25 rootcling_stage1 0x000100583b29 
clang::Parser::ParseTopLevelDecl() + 57
26 rootcling_stage1 0x0001016b5d67 
clang::Parser::ParseLinkage(clang::ParsingDeclSpec&, clang::DeclaratorContext) 
+ 1303
27 rootcling_stage1 0x0001017afa70 
clang::Parser::ParseDeclOrFunctionDefInternal(clang::ParsedAttributes&, 
clang::ParsedAttributes&, clang::ParsingDeclSpec&, clang::AccessSpecifier) + 
1792
28 rootcling_stage1 0x0001017aef1a 
clang::Parser::ParseDeclarationOrFunctionDefinition(clang::ParsedAttributes&, 
clang::ParsedAttributes&, clang::ParsingDeclSpec*, clang::AccessSpecifier) + 218
29 rootcling_stage1 0x0001017adeb0 
clang::Parser::ParseExternalDeclaration(clang::ParsedAttributes&, 
clang::ParsedAttributes&, clang::ParsingDeclSpec*) + 3920
30 rootcling_stage1 0x0001017ab8f3 
clang::Parser::ParseTopLevelDecl(clang::OpaquePtr&, 
clang::Sema::ModuleImportState&) + 2403
31 rootcling_stage1 0x0001017aaf00 
clang::Parser::ParseFirstTopLevelDecl(clang::OpaquePtr&, 
clang::Sema::ModuleImportState&) + 64
32 rootcling_stage1 0x00010167af3b clang::ParseAST(clang::Sema&, 
bool, bool) + 459
33 rootcling_stage1 0x00010135a5cc 
clang::ASTFrontendAction::ExecuteAction() + 300
34 rootcling_stage1 0x000101359dcc 

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

2024-02-28 Thread 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)

2024-02-28 Thread 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)

2024-02-28 Thread 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)

2024-02-28 Thread via llvm-branch-commits

llvmbot wrote:



@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Chuanqi Xu (ChuanqiXu9)


Changes

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.

---

Patch is 21.06 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/83237.diff


8 Files Affected:

- (modified) clang/include/clang/AST/DeclTemplate.h (+2-30) 
- (modified) clang/include/clang/Serialization/ASTBitCodes.h (+1-1) 
- (modified) clang/lib/AST/DeclTemplate.cpp (+14-35) 
- (modified) clang/lib/Serialization/ASTCommon.h (-1) 
- (modified) clang/lib/Serialization/ASTReader.cpp (+2-2) 
- (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+3-73) 
- (modified) clang/lib/Serialization/ASTWriter.cpp (+4-22) 
- (modified) clang/lib/Serialization/ASTWriterDecl.cpp (+4-48) 


``diff
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 =(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 ) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo ) 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 ) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector ,
@@ -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].DeclID; I != N; ++I) {
-  // Skip over already loaded specializations.
-  if 

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

2024-02-28 Thread 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 =(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 ) const {
-  return DeclID < Other.DeclID;
-}
-bool operator==(const LazySpecializationInfo ) 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 ) const;
-
   template 
   typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector ,
@@ -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].DeclID; I != N; ++I) {
-  // Skip