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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
