Merge pull request #38794 from slavapestov/requirement-machine-trie-lhs-simplify

RequirementMachine: Use trie for left hand side simplification

Author: slavapestov

lib/AST/RequirementMachine/RequirementMachine.cpp

@@ -391,7 +391,10 @@ void RequirementMachine::computeCompletion() {
 
     // Simplify right hand sides in preparation for building the
     // property map.
-    System.simplifyRightHandSides();
+    System.simplifyRewriteSystem();
+
+    // Check invariants.
+    System.verify();
 
     // Build the property map, which also performs concrete term
     // unification; if this added any new rules, run the completion

lib/AST/RequirementMachine/RewriteSystem.cpp

@@ -200,18 +200,60 @@ bool RewriteSystem::simplify(MutableTerm &term) const {
   return changed;
 }
 
-void RewriteSystem::simplifyRightHandSides() {
-  for (auto &rule : Rules) {
+/// Delete any rules whose left hand sides can be reduced by other rules,
+/// and reduce the right hand sides of all remaining rules as much as
+/// possible.
+///
+/// Must be run after the completion procedure, since the deletion of
+/// rules is only valid to perform if the rewrite system is confluent.
+void RewriteSystem::simplifyRewriteSystem() {
+  for (auto ruleID : indices(Rules)) {
+    auto &rule = Rules[ruleID];
     if (rule.isDeleted())
       continue;
 
+    // First, see if the left hand side of this rule can be reduced using
+    // some other rule.
+    auto lhs = rule.getLHS();
+    auto begin = lhs.begin();
+    auto end = lhs.end();
+    while (begin < end) {
+      if (auto otherRuleID = Trie.find(begin++, end)) {
+        // A rule does not obsolete itself.
+        if (*otherRuleID == ruleID)
+          continue;
+
+        // Ignore other deleted rules.
+        if (Rules[*otherRuleID].isDeleted())
+          continue;
+
+        if (DebugCompletion) {
+          const auto &otherRule = Rules[ruleID];
+          llvm::dbgs() << "$ Deleting rule " << rule << " because "
+                       << "its left hand side contains " << otherRule
+                       << "\n";
+        }
+
+        rule.markDeleted();
+        break;
+      }
+    }
+
+    // If the rule was deleted above, skip the rest.
+    if (rule.isDeleted())
+      continue;
+
+    // Now, try to reduce the right hand side.
     MutableTerm rhs(rule.getRHS());
     if (!simplify(rhs))
       continue;
 
+    // If the right hand side was further reduced, update the rule.
     rule = Rule(rule.getLHS(), Term::get(rhs, Context));
   }
+}
 
+void RewriteSystem::verify() const {
 #ifndef NDEBUG
 
 #define ASSERT_RULE(expr) \

lib/AST/RequirementMachine/RewriteSystem.h

@@ -54,10 +54,6 @@ class Rule final {
     return LHS.checkForOverlap(other.LHS, t, v);
   }
 
-  bool canReduceLeftHandSide(const Rule &other) const {
-    return LHS.containsSubTerm(other.LHS);
-  }
-
   /// Returns if the rule was deleted.
   bool isDeleted() const {
     return deleted;
@@ -168,7 +164,9 @@ class RewriteSystem final {
   computeConfluentCompletion(unsigned maxIterations,
                              unsigned maxDepth);
 
-  void simplifyRightHandSides();
+  void simplifyRewriteSystem();
+
+  void verify() const;
 
   std::pair<CompletionResult, unsigned>
   buildPropertyMap(PropertyMap &map,

lib/AST/RequirementMachine/RewriteSystemCompletion.cpp

@@ -500,29 +500,6 @@ RewriteSystem::computeConfluentCompletion(unsigned maxIterations,
     if (newRule.getDepth() > maxDepth)
       return std::make_pair(CompletionResult::MaxDepth, steps);
 
-    // Check if the new rule X == Y obsoletes any existing rules.
-    for (unsigned j : indices(Rules)) {
-      // A rule does not obsolete itself.
-      if (i == j)
-        continue;
-
-      auto &rule = Rules[j];
-
-      // Ignore rules that have already been obsoleted.
-      if (rule.isDeleted())
-        continue;
-
-      // If this rule reduces some existing rule, delete the existing rule.
-      if (rule.canReduceLeftHandSide(newRule)) {
-        if (DebugCompletion) {
-          llvm::dbgs() << "$ Deleting rule " << rule << " because "
-                       << "its left hand side contains " << newRule
-                       << "\n";
-        }
-        rule.markDeleted();
-      }
-    }
-
     // If this new rule merges any associated types, process the merge now
     // before we continue with the completion procedure. This is important
     // to perform incrementally since merging is required to repair confluence

lib/AST/RequirementMachine/Term.cpp

@@ -106,15 +106,6 @@ Term Term::get(const MutableTerm &mutableTerm, RewriteContext &ctx) {
   return term;
 }
 
-/// Find the start of \p other in this term, returning end() if
-/// \p other does not occur as a subterm of this term.
-ArrayRef<Symbol>::iterator Term::findSubTerm(Term other) const {
-  if (other.size() > size())
-    return end();
-
-  return std::search(begin(), end(), other.begin(), other.end());
-}
-
 void Term::Storage::Profile(llvm::FoldingSetNodeID &id) const {
   id.AddInteger(Size);
 

lib/AST/RequirementMachine/Term.h

@@ -81,13 +81,6 @@ class Term final {
                               MutableTerm &t,
                               MutableTerm &v) const;
 
-  ArrayRef<Symbol>::iterator findSubTerm(Term other) const;
-
-  /// Returns true if this term contains, or is equal to, \p other.
-  bool containsSubTerm(Term other) const {
-    return findSubTerm(other) != end();
-  }
-
   ArrayRef<const ProtocolDecl *> getRootProtocols() const {
     return begin()->getRootProtocols();
   }