[PATCH] D127283: [pseudo] Simplify the glrReduce implementation.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 435463.
hokein marked 3 inline comments as done.
hokein added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127283/new/

https://reviews.llvm.org/D127283

Files:
  clang-tools-extra/pseudo/lib/GLR.cpp


Index: clang-tools-extra/pseudo/lib/GLR.cpp
===================================================================
--- clang-tools-extra/pseudo/lib/GLR.cpp
+++ clang-tools-extra/pseudo/lib/GLR.cpp
@@ -273,18 +273,16 @@
     }
   };
 
-  // The base nodes are the heads after popping the GSS nodes we are reducing.
-  // We don't care which rule yielded each base. If Family.Symbol is S, the
-  // base includes an item X := ... • S ... and since the grammar is
-  // context-free, *all* parses of S are valid here.
-  // FIXME: reuse the queues across calls instead of reallocating.
-  KeyedQueue<Family, const GSS::Node *> Bases;
-
   // A sequence is the ForestNode payloads of the GSS nodes we are reducing.
   // These are the RHS of the rule, the RuleID is stored in the Family.
   // They specify a sequence ForestNode we may build (but we dedup first).
   using Sequence = llvm::SmallVector<const ForestNode *, Rule::MaxElements>;
-  KeyedQueue<Family, Sequence> Sequences;
+  struct PushSpec {
+    // A base node is the head after popping the GSS nodes we are reducing.
+    const GSS::Node* Base = nullptr;
+    Sequence Seq;
+  };
+  KeyedQueue<Family, PushSpec> Sequences;
 
   Sequence TempSequence;
   // Pop walks up the parent chain(s) for a reduction from Head by to Rule.
@@ -297,9 +295,8 @@
     auto DFS = [&](const GSS::Node *N, unsigned I, auto &DFS) {
       if (I == Rule.Size) {
         F.Start = TempSequence.front()->startTokenIndex();
-        Bases.emplace(F, N);
         LLVM_DEBUG(llvm::dbgs() << "    --> base at S" << N->State << "\n");
-        Sequences.emplace(F, TempSequence);
+        Sequences.emplace(F, PushSpec{N, TempSequence});
         return;
       }
       TempSequence[Rule.Size - 1 - I] = N->Payload;
@@ -325,20 +322,26 @@
   //  - process one family at a time, forming a forest node
   //  - produces new GSS heads which may enable more pops
   PopPending();
-  while (!Bases.empty()) {
-    // We should always have bases and sequences for the same families.
-    Family F = Bases.top().first;
-    assert(!Sequences.empty());
-    assert(Sequences.top().first == F);
+  while (!Sequences.empty()) {
+    Family F = Sequences.top().first;
 
     LLVM_DEBUG(llvm::dbgs() << "  Push " << Params.G.symbolName(F.Symbol)
                             << " from token " << F.Start << "\n");
 
-    // Grab the sequences for this family.
+    // Grab the sequences and bases for this family.
+    // We don't care which rule yielded each base. If Family.Symbol is S, the
+    // base includes an item X := ... • S ... and since the grammar is
+    // context-free, *all* parses of S are valid here.
     FamilySequences.clear();
+    FamilyBases.clear();
     do {
       FamilySequences.emplace_back(Sequences.top().first.Rule,
-                                   Sequences.top().second);
+                                   Sequences.top().second.Seq);
+      FamilyBases.emplace_back(
+          Params.Table.getGoToState(Sequences.top().second.Base->State,
+                                    F.Symbol),
+          Sequences.top().second.Base);
+
       Sequences.pop();
     } while (!Sequences.empty() && Sequences.top().first == F);
     // Build a forest node for each unique sequence.
@@ -355,15 +358,7 @@
             : &Params.Forest.createAmbiguous(F.Symbol, SequenceNodes);
     LLVM_DEBUG(llvm::dbgs() << "    --> " << Parsed->dump(Params.G) << "\n");
 
-    // Grab the bases for this family.
-    // As well as deduplicating them, we'll group by the goto state.
-    FamilyBases.clear();
-    do {
-      FamilyBases.emplace_back(
-          Params.Table.getGoToState(Bases.top().second->State, F.Symbol),
-          Bases.top().second);
-      Bases.pop();
-    } while (!Bases.empty() && Bases.top().first == F);
+    // Bases for this family, deduplicate them, and group by the goTo State.
     sortAndUnique(FamilyBases);
     // Create a GSS node for each unique goto state.
     llvm::ArrayRef<decltype(FamilyBases)::value_type> BasesLeft = FamilyBases;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D127283.435463.patch
Type: text/x-patch
Size: 4025 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220609/90bdd3d4/attachment-0001.bin>