Skip to content

Commit cc12c4e

Browse files
authored
Merge pull request #40732 from slavapestov/rqm-layout-requirement-relations
RequirementMachine: Minimization understands 'relations' among layout requirements
2 parents cb0c527 + 66cf913 commit cc12c4e

File tree

9 files changed

+321
-125
lines changed

9 files changed

+321
-125
lines changed

lib/AST/RequirementMachine/HomotopyReduction.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,16 @@ RewriteLoop::findRulesAppearingOnceInEmptyContext(
8888
switch (step.Kind) {
8989
case RewriteStep::ApplyRewriteRule: {
9090
if (!step.isInContext() && !evaluator.isInContext())
91-
rulesInEmptyContext.insert(step.RuleID);
91+
rulesInEmptyContext.insert(step.getRuleID());
9292

93-
++ruleMultiplicity[step.RuleID];
93+
++ruleMultiplicity[step.getRuleID()];
9494
break;
9595
}
9696

9797
case RewriteStep::AdjustConcreteType:
9898
case RewriteStep::Shift:
9999
case RewriteStep::Decompose:
100+
case RewriteStep::Relation:
100101
case RewriteStep::ConcreteConformance:
101102
case RewriteStep::SuperclassConformance:
102103
case RewriteStep::ConcreteTypeWitness:
@@ -207,7 +208,7 @@ RewritePath RewritePath::splitCycleAtRule(unsigned ruleID) const {
207208
for (auto step : Steps) {
208209
switch (step.Kind) {
209210
case RewriteStep::ApplyRewriteRule: {
210-
if (step.RuleID != ruleID)
211+
if (step.getRuleID() != ruleID)
211212
break;
212213

213214
assert(!sawRule && "Rule appears more than once?");
@@ -220,6 +221,7 @@ RewritePath RewritePath::splitCycleAtRule(unsigned ruleID) const {
220221
case RewriteStep::AdjustConcreteType:
221222
case RewriteStep::Shift:
222223
case RewriteStep::Decompose:
224+
case RewriteStep::Relation:
223225
case RewriteStep::ConcreteConformance:
224226
case RewriteStep::SuperclassConformance:
225227
case RewriteStep::ConcreteTypeWitness:
@@ -262,7 +264,7 @@ bool RewritePath::replaceRuleWithPath(unsigned ruleID,
262264

263265
for (const auto &step : Steps) {
264266
if (step.Kind == RewriteStep::ApplyRewriteRule &&
265-
step.RuleID == ruleID) {
267+
step.getRuleID() == ruleID) {
266268
foundAny = true;
267269
break;
268270
}
@@ -282,7 +284,7 @@ bool RewritePath::replaceRuleWithPath(unsigned ruleID,
282284
for (const auto &step : Steps) {
283285
switch (step.Kind) {
284286
case RewriteStep::ApplyRewriteRule: {
285-
if (step.RuleID != ruleID) {
287+
if (step.getRuleID() != ruleID) {
286288
newSteps.push_back(step);
287289
break;
288290
}
@@ -321,6 +323,7 @@ bool RewritePath::replaceRuleWithPath(unsigned ruleID,
321323
case RewriteStep::AdjustConcreteType:
322324
case RewriteStep::Shift:
323325
case RewriteStep::Decompose:
326+
case RewriteStep::Relation:
324327
case RewriteStep::ConcreteConformance:
325328
case RewriteStep::SuperclassConformance:
326329
case RewriteStep::ConcreteTypeWitness:

lib/AST/RequirementMachine/MinimalConformances.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ void RewriteLoop::findProtocolConformanceRules(
113113
if (!evaluator.isInContext()) {
114114
switch (step.Kind) {
115115
case RewriteStep::ApplyRewriteRule: {
116-
const auto &rule = system.getRule(step.RuleID);
116+
const auto &rule = system.getRule(step.getRuleID());
117117

118118
if (rule.isIdentityConformanceRule())
119119
break;
@@ -127,7 +127,7 @@ void RewriteLoop::findProtocolConformanceRules(
127127
// the prefix term is 'X'.
128128
const auto &term = evaluator.getCurrentTerm();
129129
MutableTerm prefix(term.begin(), term.begin() + step.StartOffset);
130-
result[proto].RulesInContext.emplace_back(prefix, step.RuleID);
130+
result[proto].RulesInContext.emplace_back(prefix, step.getRuleID());
131131
}
132132
}
133133

@@ -137,6 +137,7 @@ void RewriteLoop::findProtocolConformanceRules(
137137
case RewriteStep::AdjustConcreteType:
138138
case RewriteStep::Shift:
139139
case RewriteStep::Decompose:
140+
case RewriteStep::Relation:
140141
case RewriteStep::ConcreteConformance:
141142
case RewriteStep::SuperclassConformance:
142143
case RewriteStep::ConcreteTypeWitness:
@@ -262,7 +263,7 @@ MinimalConformances::decomposeTermIntoConformanceRuleLeftHandSides(
262263
const auto &step = *steps.begin();
263264

264265
#ifndef NDEBUG
265-
const auto &rule = System.getRule(step.RuleID);
266+
const auto &rule = System.getRule(step.getRuleID());
266267
assert(rule.isAnyConformanceRule());
267268
assert(!rule.isIdentityConformanceRule());
268269
#endif
@@ -277,9 +278,10 @@ MinimalConformances::decomposeTermIntoConformanceRuleLeftHandSides(
277278
// Build the term U.
278279
MutableTerm prefix(term.begin(), term.begin() + step.StartOffset);
279280

280-
decomposeTermIntoConformanceRuleLeftHandSides(prefix, step.RuleID, result);
281+
decomposeTermIntoConformanceRuleLeftHandSides(
282+
prefix, step.getRuleID(), result);
281283
} else {
282-
result.push_back(step.RuleID);
284+
result.push_back(step.getRuleID());
283285
}
284286
}
285287

lib/AST/RequirementMachine/PropertyMap.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ Optional<unsigned> PropertyMap::addProperty(
319319
assert(*System.getRule(ruleID).isPropertyRule() == property);
320320
auto *props = getOrCreateProperties(key);
321321
bool debug = Debug.contains(DebugFlags::ConcreteUnification);
322-
return props->addProperty(property, ruleID, Context,
322+
return props->addProperty(property, ruleID, System,
323323
inducedRules, debug);
324324
}
325325

lib/AST/RequirementMachine/PropertyMap.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ class PropertyBag {
103103

104104
Optional<unsigned> addProperty(Symbol property,
105105
unsigned ruleID,
106-
RewriteContext &ctx,
106+
RewriteSystem &system,
107107
SmallVectorImpl<InducedRule> &inducedRules,
108108
bool debug);
109109
void copyPropertiesFrom(const PropertyBag *next,

lib/AST/RequirementMachine/PropertyUnification.cpp

Lines changed: 108 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,82 @@
3737
using namespace swift;
3838
using namespace rewriting;
3939

40+
unsigned RewriteSystem::recordRelation(Symbol lhs, Symbol rhs) {
41+
auto key = std::make_pair(lhs, rhs);
42+
auto found = RelationMap.find(key);
43+
if (found != RelationMap.end())
44+
return found->second;
45+
46+
unsigned index = Relations.size();
47+
Relations.push_back(key);
48+
auto inserted = RelationMap.insert(std::make_pair(key, index));
49+
assert(inserted.second);
50+
(void) inserted;
51+
52+
return index;
53+
}
54+
55+
RewriteSystem::Relation
56+
RewriteSystem::getRelation(unsigned index) const {
57+
return Relations[index];
58+
}
59+
60+
/// Given two property rules (T.[p1] => T) and (T.[p2] => T) where [p1] < [p2],
61+
/// record a rewrite loop that makes the second rule redundant from the first.
62+
static void recordRelation(unsigned lhsRuleID, unsigned rhsRuleID,
63+
RewriteSystem &system,
64+
SmallVectorImpl<InducedRule> &inducedRules,
65+
bool debug) {
66+
const auto &lhsRule = system.getRule(lhsRuleID);
67+
const auto &rhsRule = system.getRule(rhsRuleID);
68+
69+
auto lhsProperty = lhsRule.getLHS().back();
70+
auto rhsProperty = rhsRule.getLHS().back();
71+
72+
assert(lhsProperty.isProperty());
73+
assert(rhsProperty.isProperty());
74+
assert(lhsProperty.getKind() == rhsProperty.getKind());
75+
76+
if (debug) {
77+
llvm::dbgs() << "%% Recording relation: ";
78+
llvm::dbgs() << lhsRule.getLHS() << " < " << rhsProperty << "\n";
79+
}
80+
81+
unsigned relationID = system.recordRelation(lhsProperty, rhsProperty);
82+
83+
/// Build the following rewrite path:
84+
///
85+
/// (T => T.[p1]).[p2] ⊗ T.Relation([p1].[p2] => [p1]) ⊗ (T.[p1] => T).
86+
///
87+
RewritePath path;
88+
89+
/// Starting from T.[p2], the LHS rule in reverse to get T.[p1].[p2].
90+
path.add(RewriteStep::forRewriteRule(/*startOffset=*/0,
91+
/*endOffset=*/1,
92+
/*ruleID=*/lhsRuleID,
93+
/*inverse=*/true));
94+
95+
/// T.Relation([p1].[p2] => [p1]).
96+
path.add(RewriteStep::forRelation(relationID, /*inverse=*/false));
97+
98+
/// (T.[p1] => T).
99+
path.add(RewriteStep::forRewriteRule(/*startOffset=*/0,
100+
/*endOffset=*/0,
101+
/*ruleID=*/lhsRuleID,
102+
/*inverse=*/false));
103+
104+
/// Add the rule (T.[p2] => T) with the above rewrite path.
105+
///
106+
/// Since a rule (T.[p2] => T) *already exists*, both sides of the new
107+
/// rule will simplify down to T, and the rewrite path will become a loop.
108+
///
109+
/// This loop encodes the fact that (T.[p1] => T) makes (T.[p2] => T)
110+
/// redundant.
111+
inducedRules.emplace_back(MutableTerm(rhsRule.getLHS()),
112+
MutableTerm(rhsRule.getRHS()),
113+
path);
114+
}
115+
40116
/// This method takes a concrete type that was derived from a concrete type
41117
/// produced by RewriteSystemBuilder::getConcreteSubstitutionSchema(),
42118
/// either by extracting a structural sub-component or performing a (Swift AST)
@@ -310,7 +386,7 @@ static std::pair<Symbol, bool> unifySuperclasses(
310386
/// Returns the old conflicting rule ID if there was a conflict,
311387
/// otherwise returns None.
312388
Optional<unsigned> PropertyBag::addProperty(
313-
Symbol property, unsigned ruleID, RewriteContext &ctx,
389+
Symbol property, unsigned ruleID, RewriteSystem &system,
314390
SmallVectorImpl<InducedRule> &inducedRules,
315391
bool debug) {
316392

@@ -320,18 +396,41 @@ Optional<unsigned> PropertyBag::addProperty(
320396
ConformsToRules.push_back(ruleID);
321397
return None;
322398

323-
case Symbol::Kind::Layout:
399+
case Symbol::Kind::Layout: {
400+
auto newLayout = property.getLayoutConstraint();
401+
324402
if (!Layout) {
325-
Layout = property.getLayoutConstraint();
403+
// If we haven't seen a layout requirement before, just record it.
404+
Layout = newLayout;
326405
LayoutRule = ruleID;
327406
} else {
407+
// Otherwise, compute the intersection.
328408
assert(LayoutRule.hasValue());
329-
Layout = Layout.merge(property.getLayoutConstraint());
330-
if (!Layout->isKnownLayout())
409+
auto mergedLayout = Layout.merge(property.getLayoutConstraint());
410+
411+
// If the intersection is invalid, we have a conflict.
412+
if (!mergedLayout->isKnownLayout())
331413
return LayoutRule;
414+
415+
// If the intersection is equal to the existing layout requirement,
416+
// the new layout requirement is redundant.
417+
if (mergedLayout == Layout) {
418+
recordRelation(*LayoutRule, ruleID, system, inducedRules, debug);
419+
420+
// If the intersection is equal to the new layout requirement, the
421+
// existing layout requirement is redundant.
422+
} else if (mergedLayout == newLayout) {
423+
recordRelation(ruleID, *LayoutRule, system, inducedRules, debug);
424+
LayoutRule = ruleID;
425+
} else {
426+
llvm::errs() << "Arbitrary intersection of layout requirements is "
427+
<< "supported yet\n";
428+
abort();
429+
}
332430
}
333431

334432
return None;
433+
}
335434

336435
case Symbol::Kind::Superclass: {
337436
// FIXME: Also handle superclass vs concrete
@@ -342,7 +441,8 @@ Optional<unsigned> PropertyBag::addProperty(
342441
} else {
343442
assert(SuperclassRule.hasValue());
344443
auto pair = unifySuperclasses(*Superclass, property,
345-
ctx, inducedRules, debug);
444+
system.getRewriteContext(),
445+
inducedRules, debug);
346446
Superclass = pair.first;
347447
bool conflict = pair.second;
348448
if (conflict)
@@ -359,7 +459,8 @@ Optional<unsigned> PropertyBag::addProperty(
359459
} else {
360460
assert(ConcreteTypeRule.hasValue());
361461
bool conflict = unifyConcreteTypes(*ConcreteType, property,
362-
ctx, inducedRules, debug);
462+
system.getRewriteContext(),
463+
inducedRules, debug);
363464
if (conflict)
364465
return ConcreteTypeRule;
365466
}

0 commit comments

Comments
 (0)