Skip to content

Commit ae54f47

Browse files
authored
[Clang] Improve subsumption. (llvm#132849)
The main goal of this patch is to improve the performance of concept subsumption by - Making sure literal (atomic) clauses are de-duplicated (Whether 2 atomic constraint is established during the initial normal form production). - Eagerly removing duplicated clauses. This should minimize the risks of exponentially large formulas that can be produced by a naive {C,D}NF transformation. While at it, I restructured that part of the code to be a bit clearer. Subsumption of fold expanded constraint is also cached. --- Note that removing duplicated clauses seems to be necessary and sufficient to have acceptable performance on anything that could be construed as reasonable code. Ultimately, the number of clauses is always going to be fairly small (but $2^{fairly\ small}$ is quickly *fairly large*..). I went too far in the rabbit hole of Tseitin transformations etc, which was much faster but would then require to check satisfiabiliy to establish subsumption between some constraints (although it was good enough to pass all but ones of our tests...). It doesn't help that the C++ standard has a very specific definition of subsumption that is really more of an implication... While that sort of musing is fascinating, it was ultimately a fool's errand, at least until such time that there is more motivation for a SAT solver in clang (clang-tidy can after all use z3!). Here be dragons. Fixes llvm#122581
1 parent 00c43ae commit ae54f47

File tree

4 files changed

+686
-237
lines changed

4 files changed

+686
-237
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,8 @@ Bug Fixes to C++ Support
370370
- Fixed a Clang regression in C++20 mode where unresolved dependent call expressions were created inside non-dependent contexts (#GH122892)
371371
- Clang now emits the ``-Wunused-variable`` warning when some structured bindings are unused
372372
and the ``[[maybe_unused]]`` attribute is not applied. (#GH125810)
373-
- Clang now issues an error when placement new is used to modify a const-qualified variable
373+
- Clang no longer crashes when establishing subsumption between some constraint expressions. (#GH122581)
374+
- Clang now issues an error when placement new is used to modify a const-qualified variable
374375
in a ``constexpr`` function. (#GH131432)
375376

376377
Bug Fixes to AST Handling

clang/include/clang/Sema/SemaConcept.h

Lines changed: 96 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,14 @@
1414
#define LLVM_CLANG_SEMA_SEMACONCEPT_H
1515
#include "clang/AST/ASTConcept.h"
1616
#include "clang/AST/ASTContext.h"
17-
#include "clang/AST/Expr.h"
1817
#include "clang/AST/DeclTemplate.h"
18+
#include "clang/AST/Expr.h"
1919
#include "clang/Basic/SourceLocation.h"
20+
#include "llvm/ADT/FoldingSet.h"
2021
#include "llvm/ADT/PointerUnion.h"
22+
#include "llvm/ADT/STLFunctionalExtras.h"
2123
#include "llvm/ADT/SmallVector.h"
2224
#include <optional>
23-
#include <string>
2425
#include <utility>
2526

2627
namespace clang {
@@ -56,49 +57,10 @@ struct alignas(ConstraintAlignment) AtomicConstraint {
5657
}
5758
return true;
5859
}
59-
60-
bool subsumes(ASTContext &C, const AtomicConstraint &Other) const {
61-
// C++ [temp.constr.order] p2
62-
// - an atomic constraint A subsumes another atomic constraint B
63-
// if and only if the A and B are identical [...]
64-
//
65-
// C++ [temp.constr.atomic] p2
66-
// Two atomic constraints are identical if they are formed from the
67-
// same expression and the targets of the parameter mappings are
68-
// equivalent according to the rules for expressions [...]
69-
70-
// We do not actually substitute the parameter mappings into the
71-
// constraint expressions, therefore the constraint expressions are
72-
// the originals, and comparing them will suffice.
73-
if (ConstraintExpr != Other.ConstraintExpr)
74-
return false;
75-
76-
// Check that the parameter lists are identical
77-
return hasMatchingParameterMapping(C, Other);
78-
}
7960
};
8061

81-
struct alignas(ConstraintAlignment) FoldExpandedConstraint;
82-
83-
using NormalFormConstraint =
84-
llvm::PointerUnion<AtomicConstraint *, FoldExpandedConstraint *>;
85-
struct NormalizedConstraint;
86-
using NormalForm =
87-
llvm::SmallVector<llvm::SmallVector<NormalFormConstraint, 2>, 4>;
88-
89-
// A constraint is in conjunctive normal form when it is a conjunction of
90-
// clauses where each clause is a disjunction of atomic constraints. For atomic
91-
// constraints A, B, and C, the constraint A  ∧ (B  ∨ C) is in conjunctive
92-
// normal form.
93-
NormalForm makeCNF(const NormalizedConstraint &Normalized);
94-
95-
// A constraint is in disjunctive normal form when it is a disjunction of
96-
// clauses where each clause is a conjunction of atomic constraints. For atomic
97-
// constraints A, B, and C, the disjunctive normal form of the constraint A
98-
//  ∧ (B  ∨ C) is (A  ∧ B)  ∨ (A  ∧ C).
99-
NormalForm makeDNF(const NormalizedConstraint &Normalized);
100-
10162
struct alignas(ConstraintAlignment) NormalizedConstraintPair;
63+
struct alignas(ConstraintAlignment) FoldExpandedConstraint;
10264

10365
/// \brief A normalized constraint, as defined in C++ [temp.constr.normal], is
10466
/// either an atomic constraint, a conjunction of normalized constraints or a
@@ -170,10 +132,6 @@ struct alignas(ConstraintAlignment) FoldExpandedConstraint {
170132
const Expr *Pattern)
171133
: Kind(K), Constraint(std::move(C)), Pattern(Pattern) {};
172134

173-
template <typename AtomicSubsumptionEvaluator>
174-
bool subsumes(const FoldExpandedConstraint &Other,
175-
const AtomicSubsumptionEvaluator &E) const;
176-
177135
static bool AreCompatibleForSubsumption(const FoldExpandedConstraint &A,
178136
const FoldExpandedConstraint &B);
179137
};
@@ -182,90 +140,102 @@ const NormalizedConstraint *getNormalizedAssociatedConstraints(
182140
Sema &S, NamedDecl *ConstrainedDecl,
183141
ArrayRef<const Expr *> AssociatedConstraints);
184142

185-
template <typename AtomicSubsumptionEvaluator>
186-
bool subsumes(const NormalForm &PDNF, const NormalForm &QCNF,
187-
const AtomicSubsumptionEvaluator &E) {
188-
// C++ [temp.constr.order] p2
189-
// Then, P subsumes Q if and only if, for every disjunctive clause Pi in the
190-
// disjunctive normal form of P, Pi subsumes every conjunctive clause Qj in
191-
// the conjuctive normal form of Q, where [...]
192-
for (const auto &Pi : PDNF) {
193-
for (const auto &Qj : QCNF) {
194-
// C++ [temp.constr.order] p2
195-
// - [...] a disjunctive clause Pi subsumes a conjunctive clause Qj if
196-
// and only if there exists an atomic constraint Pia in Pi for which
197-
// there exists an atomic constraint, Qjb, in Qj such that Pia
198-
// subsumes Qjb.
199-
bool Found = false;
200-
for (NormalFormConstraint Pia : Pi) {
201-
for (NormalFormConstraint Qjb : Qj) {
202-
if (isa<FoldExpandedConstraint *>(Pia) &&
203-
isa<FoldExpandedConstraint *>(Qjb)) {
204-
if (cast<FoldExpandedConstraint *>(Pia)->subsumes(
205-
*cast<FoldExpandedConstraint *>(Qjb), E)) {
206-
Found = true;
207-
break;
208-
}
209-
} else if (isa<AtomicConstraint *>(Pia) &&
210-
isa<AtomicConstraint *>(Qjb)) {
211-
if (E(*cast<AtomicConstraint *>(Pia),
212-
*cast<AtomicConstraint *>(Qjb))) {
213-
Found = true;
214-
break;
215-
}
216-
}
217-
}
218-
if (Found)
219-
break;
220-
}
221-
if (!Found)
222-
return false;
223-
}
224-
}
225-
return true;
226-
}
227-
228-
template <typename AtomicSubsumptionEvaluator>
229-
bool subsumes(Sema &S, NamedDecl *DP, ArrayRef<const Expr *> P, NamedDecl *DQ,
230-
ArrayRef<const Expr *> Q, bool &Subsumes,
231-
const AtomicSubsumptionEvaluator &E) {
232-
// C++ [temp.constr.order] p2
233-
// In order to determine if a constraint P subsumes a constraint Q, P is
234-
// transformed into disjunctive normal form, and Q is transformed into
235-
// conjunctive normal form. [...]
236-
const NormalizedConstraint *PNormalized =
237-
getNormalizedAssociatedConstraints(S, DP, P);
238-
if (!PNormalized)
239-
return true;
240-
NormalForm PDNF = makeDNF(*PNormalized);
143+
/// \brief SubsumptionChecker establishes subsumption
144+
/// between two set of constraints.
145+
class SubsumptionChecker {
146+
public:
147+
using SubsumptionCallable = llvm::function_ref<bool(
148+
const AtomicConstraint &, const AtomicConstraint &)>;
241149

242-
const NormalizedConstraint *QNormalized =
243-
getNormalizedAssociatedConstraints(S, DQ, Q);
244-
if (!QNormalized)
245-
return true;
246-
NormalForm QCNF = makeCNF(*QNormalized);
247-
248-
Subsumes = subsumes(PDNF, QCNF, E);
249-
return false;
250-
}
251-
252-
template <typename AtomicSubsumptionEvaluator>
253-
bool FoldExpandedConstraint::subsumes(
254-
const FoldExpandedConstraint &Other,
255-
const AtomicSubsumptionEvaluator &E) const {
150+
SubsumptionChecker(Sema &SemaRef, SubsumptionCallable Callable = {});
256151

257-
// [C++26] [temp.constr.order]
258-
// a fold expanded constraint A subsumes another fold expanded constraint B if
259-
// they are compatible for subsumption, have the same fold-operator, and the
260-
// constraint of A subsumes that of B
152+
std::optional<bool> Subsumes(NamedDecl *DP, ArrayRef<const Expr *> P,
153+
NamedDecl *DQ, ArrayRef<const Expr *> Q);
261154

262-
if (Kind != Other.Kind || !AreCompatibleForSubsumption(*this, Other))
263-
return false;
155+
bool Subsumes(const NormalizedConstraint *P, const NormalizedConstraint *Q);
264156

265-
NormalForm PDNF = makeDNF(this->Constraint);
266-
NormalForm QCNF = makeCNF(Other.Constraint);
267-
return clang::subsumes(PDNF, QCNF, E);
268-
}
157+
private:
158+
Sema &SemaRef;
159+
SubsumptionCallable Callable;
160+
161+
// Each Literal has a unique value that is enough to establish
162+
// its identity.
163+
// Some constraints (fold expended) require special subsumption
164+
// handling logic beyond comparing values, so we store a flag
165+
// to let us quickly dispatch to each kind of variable.
166+
struct Literal {
167+
enum Kind { Atomic, FoldExpanded };
168+
169+
unsigned Value : 16;
170+
LLVM_PREFERRED_TYPE(Kind)
171+
unsigned Kind : 1;
172+
173+
bool operator==(const Literal &Other) const { return Value == Other.Value; }
174+
bool operator<(const Literal &Other) const { return Value < Other.Value; }
175+
};
176+
using Clause = llvm::SmallVector<Literal>;
177+
using Formula = llvm::SmallVector<Clause, 5>;
178+
179+
struct CNFFormula : Formula {
180+
static constexpr auto Kind = NormalizedConstraint::CCK_Conjunction;
181+
using Formula::Formula;
182+
};
183+
struct DNFFormula : Formula {
184+
static constexpr auto Kind = NormalizedConstraint::CCK_Disjunction;
185+
using Formula::Formula;
186+
};
187+
188+
struct MappedAtomicConstraint {
189+
AtomicConstraint *Constraint;
190+
Literal ID;
191+
};
192+
193+
struct FoldExpendedConstraintKey {
194+
FoldExpandedConstraint::FoldOperatorKind Kind;
195+
AtomicConstraint *Constraint;
196+
Literal ID;
197+
};
198+
199+
llvm::DenseMap<const Expr *, llvm::SmallDenseMap<llvm::FoldingSetNodeID,
200+
MappedAtomicConstraint>>
201+
AtomicMap;
202+
203+
llvm::DenseMap<const Expr *, std::vector<FoldExpendedConstraintKey>> FoldMap;
204+
205+
// A map from a literal to a corresponding associated constraint.
206+
// We do not have enough bits left for a pointer union here :(
207+
llvm::DenseMap<uint16_t, void *> ReverseMap;
208+
209+
// Fold expanded constraints ask us to recursively establish subsumption.
210+
// This caches the result.
211+
llvm::SmallDenseMap<
212+
std::pair<const FoldExpandedConstraint *, const FoldExpandedConstraint *>,
213+
bool>
214+
FoldSubsumptionCache;
215+
216+
// Each <atomic, fold expanded constraint> is represented as a single ID.
217+
// This is intentionally kept small we can't handle a large number of
218+
// constraints anyway.
219+
uint16_t NextID;
220+
221+
bool Subsumes(const DNFFormula &P, const CNFFormula &Q);
222+
bool Subsumes(Literal A, Literal B);
223+
bool Subsumes(const FoldExpandedConstraint *A,
224+
const FoldExpandedConstraint *B);
225+
bool DNFSubsumes(const Clause &P, const Clause &Q);
226+
227+
CNFFormula CNF(const NormalizedConstraint &C);
228+
DNFFormula DNF(const NormalizedConstraint &C);
229+
230+
template <typename FormulaType>
231+
FormulaType Normalize(const NormalizedConstraint &C);
232+
void AddUniqueClauseToFormula(Formula &F, Clause C);
233+
234+
Literal find(AtomicConstraint *);
235+
Literal find(FoldExpandedConstraint *);
236+
237+
uint16_t getNewLiteralId();
238+
};
269239

270240
} // clang
271241

0 commit comments

Comments
 (0)