-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[Clang] Avoid building unnecessary expressions when checking satisfaction #164611
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Clang] Avoid building unnecessary expressions when checking satisfaction #164611
Conversation
…ion. When establishing constraint satisfaction, we were building expressions even for compound constraint, This is unecessary extra work that acount for ~20% of the performance regression observed here llvm#161671 (comment)
|
@llvm/pr-subscribers-clang Author: Corentin Jabot (cor3ntin) ChangesWhen establishing constraint satisfaction, we were building expressions even for compound constraint, This is unnecessary extra work that accounts for ~20% of the performance regression observed here #161671 (comment) Full diff: https://github.com/llvm/llvm-project/pull/164611.diff 4 Files Affected:
diff --git a/clang/lib/AST/ASTConcept.cpp b/clang/lib/AST/ASTConcept.cpp
index 9ea104c4c3c9d..fd12bc4e83827 100644
--- a/clang/lib/AST/ASTConcept.cpp
+++ b/clang/lib/AST/ASTConcept.cpp
@@ -86,7 +86,7 @@ void ConstraintSatisfaction::Profile(llvm::FoldingSetNodeID &ID,
ID.AddPointer(ConstraintOwner);
ID.AddInteger(TemplateArgs.size());
for (auto &Arg : TemplateArgs)
- C.getCanonicalTemplateArgument(Arg).Profile(ID, C);
+ Arg.Profile(ID, C);
}
ConceptReference *
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 54cbfe46a6528..a1163e904dc1b 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -417,8 +417,8 @@ class ConstraintSatisfactionChecker {
const NamedDecl *Template;
SourceLocation TemplateNameLoc;
UnsignedOrNone PackSubstitutionIndex;
-
ConstraintSatisfaction &Satisfaction;
+ bool BuildExpression;
private:
ExprResult
@@ -461,10 +461,11 @@ class ConstraintSatisfactionChecker {
ConstraintSatisfactionChecker(Sema &SemaRef, const NamedDecl *Template,
SourceLocation TemplateNameLoc,
UnsignedOrNone PackSubstitutionIndex,
- ConstraintSatisfaction &Satisfaction)
+ ConstraintSatisfaction &Satisfaction,
+ bool BuildExpression)
: S(SemaRef), Template(Template), TemplateNameLoc(TemplateNameLoc),
PackSubstitutionIndex(PackSubstitutionIndex),
- Satisfaction(Satisfaction) {}
+ Satisfaction(Satisfaction), BuildExpression(BuildExpression) {}
ExprResult Evaluate(const NormalizedConstraint &Constraint,
const MultiLevelTemplateArgumentList &MLTAL);
@@ -821,9 +822,10 @@ ExprResult ConstraintSatisfactionChecker::EvaluateSlow(
Satisfaction.ContainsErrors = false;
ExprResult Expr =
ConstraintSatisfactionChecker(S, Template, TemplateNameLoc,
- UnsignedOrNone(I), Satisfaction)
+ UnsignedOrNone(I), Satisfaction,
+ /*BuildExpression=*/false)
.Evaluate(Constraint.getNormalizedPattern(), *SubstitutedArgs);
- if (Expr.isUsable()) {
+ if (BuildExpression && Expr.isUsable()) {
if (Out.isUnset())
Out = Expr;
else
@@ -834,7 +836,7 @@ ExprResult ConstraintSatisfactionChecker::EvaluateSlow(
Constraint.getBeginLoc(),
FPOptionsOverride{});
} else {
- assert(!Satisfaction.IsSatisfied);
+ assert(!BuildExpression || !Satisfaction.IsSatisfied);
}
if (!Conjunction && Satisfaction.IsSatisfied) {
Satisfaction.Details.erase(Satisfaction.Details.begin() +
@@ -985,7 +987,7 @@ ExprResult ConstraintSatisfactionChecker::Evaluate(
ExprResult E = Evaluate(Constraint.getNormalizedConstraint(), MLTAL);
- if (!E.isUsable()) {
+ if (E.isInvalid()) {
Satisfaction.Details.insert(Satisfaction.Details.begin() + Size, ConceptId);
return E;
}
@@ -1041,7 +1043,7 @@ ExprResult ConstraintSatisfactionChecker::Evaluate(
if (Conjunction && (!Satisfaction.IsSatisfied || Satisfaction.ContainsErrors))
return LHS;
- if (!Conjunction && LHS.isUsable() && Satisfaction.IsSatisfied &&
+ if (!Conjunction && !LHS.isInvalid() && Satisfaction.IsSatisfied &&
!Satisfaction.ContainsErrors)
return LHS;
@@ -1050,12 +1052,15 @@ ExprResult ConstraintSatisfactionChecker::Evaluate(
ExprResult RHS = Evaluate(Constraint.getRHS(), MLTAL);
- if (RHS.isUsable() && Satisfaction.IsSatisfied &&
+ if (!Conjunction && !RHS.isInvalid() && Satisfaction.IsSatisfied &&
!Satisfaction.ContainsErrors)
Satisfaction.Details.erase(Satisfaction.Details.begin() +
EffectiveDetailEndIndex,
Satisfaction.Details.end());
+ if (!BuildExpression)
+ return Satisfaction.ContainsErrors ? ExprError() : ExprEmpty();
+
if (!LHS.isUsable())
return RHS;
@@ -1136,10 +1141,11 @@ static bool CheckConstraintSatisfaction(
Template, /*CSE=*/nullptr,
S.ArgPackSubstIndex);
- ExprResult Res =
- ConstraintSatisfactionChecker(S, Template, TemplateIDRange.getBegin(),
- S.ArgPackSubstIndex, Satisfaction)
- .Evaluate(*C, TemplateArgsLists);
+ ExprResult Res = ConstraintSatisfactionChecker(
+ S, Template, TemplateIDRange.getBegin(),
+ S.ArgPackSubstIndex, Satisfaction,
+ /*BuildExpression=*/ConvertedExpr != nullptr)
+ .Evaluate(*C, TemplateArgsLists);
if (Res.isInvalid())
return true;
diff --git a/clang/test/SemaCXX/cxx2c-fold-exprs.cpp b/clang/test/SemaCXX/cxx2c-fold-exprs.cpp
index 137f46ee3dc01..289059ea86eb9 100644
--- a/clang/test/SemaCXX/cxx2c-fold-exprs.cpp
+++ b/clang/test/SemaCXX/cxx2c-fold-exprs.cpp
@@ -157,66 +157,55 @@ static_assert(And1<S, S>() == 1);
// FIXME: The diagnostics are not so great
static_assert(And1<int>() == 1); // expected-error {{no matching function for call to 'And1'}}
// expected-note@#and1 {{candidate template ignored: constraints not satisfied [with T = <int>]}}
- // expected-note@#and1 {{because 'typename T::type' does not satisfy 'C'}}
- // expected-note@#C {{because 'T' does not satisfy 'A'}}
+ // expected-note@#and1 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
static_assert(And1<S, int>() == 1); // expected-error {{no matching function for call to 'And1'}}
// expected-note@#and1 {{candidate template ignored: constraints not satisfied [with T = <S, int>]}}
- // expected-note@#and1 {{because 'typename T::type' does not satisfy 'C'}}
- // expected-note@#C {{because 'T' does not satisfy 'A'}}
+ // expected-note@#and1 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
static_assert(And1<int, S>() == 1); // expected-error {{no matching function for call to 'And1'}}
// expected-note@#and1 {{candidate template ignored: constraints not satisfied [with T = <int, S>]}}
- // expected-note@#and1 {{because 'typename T::type' does not satisfy 'C'}}
- // expected-note@#C {{because 'T' does not satisfy 'A'}}
+ // expected-note@#and1 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
static_assert(And2<S>() == 2);
static_assert(And2<S, S>() == 2);
static_assert(And2<int>() == 2); // expected-error {{no matching function for call to 'And2'}}
// expected-note@#and2 {{candidate template ignored: constraints not satisfied [with T = int, U = <>]}}
- // expected-note@#and2 {{because 'typename U::type' does not satisfy 'C'}}
- // expected-note@#C {{because 'T' does not satisfy 'A'}}
+ // expected-note@#and2 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
static_assert(And2<int, int>() == 2); // expected-error {{no matching function for call to 'And2'}}
// expected-note@#and2 {{candidate template ignored: constraints not satisfied [with T = S, U = <int>]}} \
- // expected-note@#and2 {{because 'typename U::type' does not satisfy 'C'}}
- // expected-note@#C {{because 'T' does not satisfy 'A'}}
+ // expected-note@#and2 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
static_assert(And2<S, int>() == 2); // expected-error {{no matching function for call to 'And2'}}
// expected-note@#and2 {{candidate template ignored: constraints not satisfied [with T = int, U = <S>]}}
- // expected-note@#and2 {{because 'typename T::type' does not satisfy 'C'}}
- // expected-note@#C {{because 'T' does not satisfy 'A'}}
+ // expected-note@#and2 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
static_assert(And2<int, S>() == 2); // expected-error {{no matching function for call to 'And2'}}
// expected-note@#and2 {{candidate template ignored: constraints not satisfied [with T = int, U = <int>]}}
- // expected-note@#and2 {{because 'typename T::type' does not satisfy 'C'}}
- // expected-note@#C {{because 'T' does not satisfy 'A'}}
+ // expected-note@#and2 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
static_assert(And3<S>() == 3);
static_assert(And3<S, S>() == 3);
static_assert(And3<int>() == 3); // expected-error {{no matching function for call to 'And3'}}
// expected-note@#and3 {{candidate template ignored: constraints not satisfied [with T = int, U = <>]}}
- // expected-note@#and3 {{because 'typename T::type' does not satisfy 'C'}}
- // expected-note@#C {{because 'T' does not satisfy 'A'}}
+ // expected-note@#and3 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
static_assert(And3<int, int>() == 3); // expected-error {{no matching function for call to 'And3'}}
// expected-note@#and3 {{candidate template ignored: constraints not satisfied [with T = int, U = <int>]}}
- // expected-note@#and3 {{because 'typename T::type' does not satisfy 'C'}}
- // expected-note@#C {{because 'T' does not satisfy 'A'}}
+ // expected-note@#and3 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
static_assert(And3<S, int>() == 3); // expected-error {{no matching function for call to 'And3'}}
// expected-note@#and3 {{candidate template ignored: constraints not satisfied [with T = S, U = <int>]}}
- // expected-note@#and3 {{because 'typename U::type' does not satisfy 'C'}}
- // expected-note@#C {{because 'T' does not satisfy 'A'}}
+ // expected-note@#and3 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
static_assert(And3<int, S>() == 3); // expected-error {{no matching function for call to 'And3'}}
// expected-note@#and3 {{candidate template ignored: constraints not satisfied [with T = int, U = <S>]}}
- // expected-note@#and3 {{because 'typename T::type' does not satisfy 'C'}}
- // expected-note@#C {{because 'T' does not satisfy 'A'}}
+ // expected-note@#and3 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
static_assert(Or1<>() == 1); // expected-error {{no matching function for call to 'Or1'}}
@@ -227,8 +216,7 @@ static_assert(Or1<S, int>() == 1);
static_assert(Or1<S, S>() == 1);
static_assert(Or1<int>() == 1); // expected-error {{no matching function for call to 'Or1'}}
// expected-note@#or1 {{candidate template ignored: constraints not satisfied}}
- // expected-note@#or1 {{because 'typename T::type' does not satisfy 'C'}}
- // expected-note@#C {{because 'T' does not satisfy 'A'}}
+ // expected-note@#or1 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
static_assert(Or2<S>() == 2);
static_assert(Or2<int, S>() == 2);
@@ -236,16 +224,14 @@ static_assert(Or2<S, int>() == 2);
static_assert(Or2<S, S>() == 2);
static_assert(Or2<int>() == 2); // expected-error {{no matching function for call to 'Or2'}}
// expected-note@#or2 {{candidate template ignored: constraints not satisfied [with T = int, U = <>]}}
- // expected-note@#or2 {{because 'typename T::type' does not satisfy 'C'}}
- // expected-note@#C {{because 'T' does not satisfy 'A'}}
+ // expected-note@#or2 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
static_assert(Or3<S>() == 3);
static_assert(Or3<int, S>() == 3);
static_assert(Or3<S, int>() == 3);
static_assert(Or3<S, S>() == 3);
static_assert(Or3<int>() == 3); // expected-error {{no matching function for call to 'Or3'}}
// expected-note@#or3 {{candidate template ignored: constraints not satisfied}}
- // expected-note@#or3 {{because 'typename T::type' does not satisfy 'C'}}
- // expected-note@#C {{because 'T' does not satisfy 'A'}}
+ // expected-note@#or3 {{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
}
namespace bool_conversion_break {
diff --git a/clang/test/SemaTemplate/concepts-recursive-inst.cpp b/clang/test/SemaTemplate/concepts-recursive-inst.cpp
index 73dce9317f383..5e1bce5f15684 100644
--- a/clang/test/SemaTemplate/concepts-recursive-inst.cpp
+++ b/clang/test/SemaTemplate/concepts-recursive-inst.cpp
@@ -82,7 +82,6 @@ auto it = begin(rng); // #BEGIN_CALL
// expected-error@#BEGIN_CALL {{no matching function for call to 'begin'}}
// expected-note@#NOTINF_BEGIN {{candidate function}}
// expected-note@#INF_BEGIN{{candidate template ignored: constraints not satisfied}}
-// expected-note@#INF_BEGIN{{because 'Inf auto' does not satisfy 'Inf}}
}
} // namespace DirectRecursiveCheck
|
zyn0217
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…tion (llvm#164611) When establishing constraint satisfaction, we were building expressions even for compound constraint, This is unnecessary extra work that accounts for ~20% of the performance regression observed here llvm#161671 (comment)
…tion (llvm#164611) When establishing constraint satisfaction, we were building expressions even for compound constraint, This is unnecessary extra work that accounts for ~20% of the performance regression observed here llvm#161671 (comment)
…tion (llvm#164611) When establishing constraint satisfaction, we were building expressions even for compound constraint, This is unnecessary extra work that accounts for ~20% of the performance regression observed here llvm#161671 (comment)
When establishing constraint satisfaction, we were building expressions even for compound constraint,
This is unnecessary extra work that accounts for ~20% of the performance regression observed here #161671 (comment)