[PATCH] D128299: [pseudo] Add a fast-path to GLR reduce when both pop and push are trivial

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:269
+  // bypass all these fancy queues and pop+push directly. This is very common.
+  auto PopAndPushTrivial = [&]() -> bool {
+    if (!Sequences.empty() || Heads.size() != PoppedHeads + 1)
----------------
IMO, it is exactly what the reduction implementation would look like for a linear LR parsing :)

maybe encode the linear in the name?


================
Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:270
+  auto PopAndPushTrivial = [&]() -> bool {
+    if (!Sequences.empty() || Heads.size() != PoppedHeads + 1)
+      return false;
----------------
The patch description is really nice document, and help to me to justify the code. I'd suggest adding them in the comment as well.

> there must be no pending pushes and only one head available to pop
> the head must have only one reduction rule
> the reduction path must be a straight line (no multiple parents)




================
Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:320
     for (; PoppedHeads < Heads.size(); ++PoppedHeads) {
-      // FIXME: if there's exactly one head in the queue, and the pop stage
-      // is trivial, we could pop + push without touching the expensive queues.
+      if (PopAndPushTrivial())
+        continue;
----------------
This seems very clever -- for trivial case, the main reduce loop is happening here.


================
Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:323
       for (const auto &A :
            Params.Table.getActions(Heads[PoppedHeads]->State, Lookahead)) {
         if (A.kind() != LRTable::Action::Reduce)
----------------
we could save an extra call of `Params.Table.getActions` -- we call it at the beginning of the loop body and store the results in a local var, and use it in PopAndPushTrivial and here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128299