https://github.com/hahnjo created
https://github.com/llvm/llvm-project/pull/133057
* Hash inner template arguments: The code is applied from `ODRHash::AddDecl`
with the reasoning given in the comment, to reduce collisions. This was
particularly visible with STL types templated on `std::pair` where its template
arguments were not taken into account.
* Complete only needed partial specializations: It is unclear (to me) why this
needs to be done "for safety", but this change significantly improves the
effectiveness of lazy loading.
* Load only needed partial specializations: Similar as the last commit, it is
unclear why we need to load all specializations, including non-partial ones,
when we have a `TPL`.
* Remove bail-out logic in `TemplateArgumentHasher`: While it is correct to
assign a single fixed hash to all template
arguments, it can reduce the effectiveness of lazy loading and is not actually
needed: we are allowed to ignore parts that cannot be handled because they will
be analogously ignored by all hashings.
>From 670d16ce0e203624ad5f6f8428a03fb0af57fdb8 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld <jonas.hahnf...@cern.ch>
Date: Tue, 25 Mar 2025 11:27:59 +0100
Subject: [PATCH 1/4] [Serialization] Hash inner template arguments
The code is applied from ODRHash::AddDecl with the reasoning given
in the comment, to reduce collisions. This was particularly visible
with STL types templated on std::pair where its template arguments
were not taken into account.
---
.../lib/Serialization/TemplateArgumentHasher.cpp | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/clang/lib/Serialization/TemplateArgumentHasher.cpp
b/clang/lib/Serialization/TemplateArgumentHasher.cpp
index 3c7177b83ba52..5f6c8e3af9267 100644
--- a/clang/lib/Serialization/TemplateArgumentHasher.cpp
+++ b/clang/lib/Serialization/TemplateArgumentHasher.cpp
@@ -199,6 +199,21 @@ void TemplateArgumentHasher::AddDecl(const Decl *D) {
}
AddDeclarationName(ND->getDeclName());
+
+ // If this was a specialization we should take into account its template
+ // arguments. This helps to reduce collisions coming when visiting template
+ // specialization types (eg. when processing type template arguments).
+ ArrayRef<TemplateArgument> Args;
+ if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D))
+ Args = CTSD->getTemplateArgs().asArray();
+ else if (auto *VTSD = dyn_cast<VarTemplateSpecializationDecl>(D))
+ Args = VTSD->getTemplateArgs().asArray();
+ else if (auto *FD = dyn_cast<FunctionDecl>(D))
+ if (FD->getTemplateSpecializationArgs())
+ Args = FD->getTemplateSpecializationArgs()->asArray();
+
+ for (auto &TA : Args)
+ AddTemplateArgument(TA);
}
void TemplateArgumentHasher::AddQualType(QualType T) {
>From 658d55ba1117a029f37f51a3072585a1fd9fc59f Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld <jonas.hahnf...@cern.ch>
Date: Tue, 25 Mar 2025 11:31:23 +0100
Subject: [PATCH 2/4] [Serialization] Complete only needed partial
specializations
It is unclear (to me) why this needs to be done "for safety", but
this change significantly improves the effectiveness of lazy loading.
---
clang/lib/Serialization/ASTReader.cpp | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 0cd2cedb48dd9..eb0496c97eb3b 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -7891,14 +7891,8 @@ void ASTReader::CompleteRedeclChain(const Decl *D) {
}
}
- if (Template) {
- // For partitial specialization, load all the specializations for safety.
- if (isa<ClassTemplatePartialSpecializationDecl,
- VarTemplatePartialSpecializationDecl>(D))
- Template->loadLazySpecializationsImpl();
- else
- Template->loadLazySpecializationsImpl(Args);
- }
+ if (Template)
+ Template->loadLazySpecializationsImpl(Args);
}
CXXCtorInitializer **
>From 48f50c7385d79da320cdf52d3aabc83cc403ff9c Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld <jonas.hahnf...@cern.ch>
Date: Tue, 25 Mar 2025 11:42:20 +0100
Subject: [PATCH 3/4] [Serialization] Load only needed partial specializations
Similar as the last commit, it is unclear why we need to load all
specializations, including non-partial ones, when we have a TPL.
---
clang/lib/AST/DeclTemplate.cpp | 6 ------
1 file changed, 6 deletions(-)
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index c0f5be51db5f3..8560c3928aa84 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -367,12 +367,6 @@ bool RedeclarableTemplateDecl::loadLazySpecializationsImpl(
if (!ExternalSource)
return false;
- // If TPL is not null, it implies that we're loading specializations for
- // partial templates. We need to load all specializations in such cases.
- if (TPL)
- return
ExternalSource->LoadExternalSpecializations(this->getCanonicalDecl(),
- /*OnlyPartial=*/false);
-
return ExternalSource->LoadExternalSpecializations(this->getCanonicalDecl(),
Args);
}
>From c8b3418a7b37372c1aa810d1d5aedea853daa5d6 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld <jonas.hahnf...@cern.ch>
Date: Tue, 25 Mar 2025 11:59:46 +0100
Subject: [PATCH 4/4] [Serialization] Remove bail-out logic in
TemplateArgumentHasher
While it is correct to assign a single fixed hash to all template
arguments, it can reduce the effectiveness of lazy loading and is
not actually needed: we are allowed to ignore parts that cannot be
handled because they will be analogously ignored by all hashings.
---
.../Serialization/TemplateArgumentHasher.cpp | 36 ++-----------------
1 file changed, 3 insertions(+), 33 deletions(-)
diff --git a/clang/lib/Serialization/TemplateArgumentHasher.cpp
b/clang/lib/Serialization/TemplateArgumentHasher.cpp
index 5f6c8e3af9267..5fb363c4ab148 100644
--- a/clang/lib/Serialization/TemplateArgumentHasher.cpp
+++ b/clang/lib/Serialization/TemplateArgumentHasher.cpp
@@ -21,17 +21,6 @@ using namespace clang;
namespace {
class TemplateArgumentHasher {
- // If we bail out during the process of calculating hash values for
- // template arguments for any reason. We're allowed to do it since
- // TemplateArgumentHasher 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 in the worst case, it is still a valid implementation to give all
- // inputs the same BailedOutValue as output.
- bool BailedOut = false;
- static constexpr unsigned BailedOutValue = 0x12345678;
-
llvm::FoldingSetNodeID ID;
public:
@@ -41,14 +30,7 @@ class TemplateArgumentHasher {
void AddInteger(unsigned V) { ID.AddInteger(V); }
- unsigned getValue() {
- if (BailedOut)
- return BailedOutValue;
-
- return ID.computeStableHash();
- }
-
- void setBailedOut() { BailedOut = true; }
+ unsigned getValue() { return ID.computeStableHash(); }
void AddType(const Type *T);
void AddQualType(QualType T);
@@ -92,8 +74,7 @@ void
TemplateArgumentHasher::AddTemplateArgument(TemplateArgument TA) {
case TemplateArgument::Expression:
// If we meet expression in template argument, it implies
// that the template is still dependent. It is meaningless
- // to get a stable hash for the template. Bail out simply.
- BailedOut = true;
+ // to get a stable hash for the template.
break;
case TemplateArgument::Pack:
AddInteger(TA.pack_size());
@@ -110,10 +91,9 @@ void TemplateArgumentHasher::AddStructuralValue(const
APValue &Value) {
// 'APValue::Profile' uses pointer values to make hash for LValue and
// MemberPointer, but they differ from one compiler invocation to another.
- // It may be difficult to handle such cases. Bail out simply.
+ // It may be difficult to handle such cases.
if (Kind == APValue::LValue || Kind == APValue::MemberPointer) {
- BailedOut = true;
return;
}
@@ -135,14 +115,11 @@ void TemplateArgumentHasher::AddTemplateName(TemplateName
Name) {
case TemplateName::DependentTemplate:
case TemplateName::SubstTemplateTemplateParm:
case TemplateName::SubstTemplateTemplateParmPack:
- BailedOut = true;
break;
case TemplateName::UsingTemplate: {
UsingShadowDecl *USD = Name.getAsUsingShadowDecl();
if (USD)
AddDecl(USD->getTargetDecl());
- else
- BailedOut = true;
break;
}
case TemplateName::DeducedTemplate:
@@ -167,7 +144,6 @@ void
TemplateArgumentHasher::AddDeclarationName(DeclarationName Name) {
case DeclarationName::ObjCZeroArgSelector:
case DeclarationName::ObjCOneArgSelector:
case DeclarationName::ObjCMultiArgSelector:
- BailedOut = true;
break;
case DeclarationName::CXXConstructorName:
case DeclarationName::CXXDestructorName:
@@ -194,7 +170,6 @@ void
TemplateArgumentHasher::AddDeclarationName(DeclarationName Name) {
void TemplateArgumentHasher::AddDecl(const Decl *D) {
const NamedDecl *ND = dyn_cast<NamedDecl>(D);
if (!ND) {
- BailedOut = true;
return;
}
@@ -218,7 +193,6 @@ void TemplateArgumentHasher::AddDecl(const Decl *D) {
void TemplateArgumentHasher::AddQualType(QualType T) {
if (T.isNull()) {
- BailedOut = true;
return;
}
SplitQualType split = T.split();
@@ -228,7 +202,6 @@ void TemplateArgumentHasher::AddQualType(QualType T) {
// Process a Type pointer. Add* methods call back into TemplateArgumentHasher
// while Visit* methods process the relevant parts of the Type.
-// Any unhandled type will make the hash computation bail out.
class TypeVisitorHelper : public TypeVisitor<TypeVisitorHelper> {
typedef TypeVisitor<TypeVisitorHelper> Inherited;
llvm::FoldingSetNodeID &ID;
@@ -260,9 +233,6 @@ class TypeVisitorHelper : public
TypeVisitor<TypeVisitorHelper> {
void Visit(const Type *T) { Inherited::Visit(T); }
- // Unhandled types. Bail out simply.
- void VisitType(const Type *T) { Hash.setBailedOut(); }
-
void VisitAdjustedType(const AdjustedType *T) {
AddQualType(T->getOriginalType());
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
