Skip to content

Commit

Permalink
[pseudo] Store last node popped in the queue, not its parent(s). NFC
Browse files Browse the repository at this point in the history
We have to walk up to the last node to find the start token, but no need
to go even one node further.

This is one node fewer to store, but more importantly if the last node
happens to have multiple parents we avoid storing the sequence multiple times.

This saves ~5% on glrParse.
Based on a comment by hokein@ on https://reviews.llvm.org/D128307
  • Loading branch information
sam-mccall committed Jun 23, 2022
1 parent c078e46 commit 466eae6
Showing 1 changed file with 14 additions and 11 deletions.
25 changes: 14 additions & 11 deletions clang-tools-extra/pseudo/lib/GLR.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,9 @@ class GLRReduce {
// They specify a sequence ForestNode we may build (but we dedup first).
// (The RuleID is not stored here, but rather in the Family).
struct PushSpec {
// A base node is the head after popping the GSS nodes we are reducing.
const GSS::Node* Base = nullptr;
// The last node popped before pushing. Its parent is the reduction base(s).
// (Base is more fundamental, but this is cheaper to store).
const GSS::Node* LastPop = nullptr;
Sequence *Seq = nullptr;
};
KeyedQueue<Family, PushSpec> Sequences; // FIXME: rename => PendingPushes?
Expand Down Expand Up @@ -256,9 +257,13 @@ class GLRReduce {
Family F{/*Start=*/0, /*Symbol=*/Rule.Target, /*Rule=*/RID};
TempSequence.resize_for_overwrite(Rule.Size);
auto DFS = [&](const GSS::Node *N, unsigned I, auto &DFS) {
if (I == Rule.Size) {
TempSequence[Rule.Size - 1 - I] = N->Payload;
if (I + 1 == Rule.Size) {
F.Start = TempSequence.front()->startTokenIndex();
LLVM_DEBUG(llvm::dbgs() << " --> base at S" << N->State << "\n");
LLVM_DEBUG({
for (const auto *B : N->parents())
llvm::dbgs() << " --> base at S" << B->State << "\n";
});

// Copy the chain to stable storage so it can be enqueued.
if (SequenceStorageCount == SequenceStorage.size())
Expand All @@ -269,7 +274,6 @@ class GLRReduce {
Sequences.emplace(F, PushSpec{N, Seq});
return;
}
TempSequence[Rule.Size - 1 - I] = N->Payload;
for (const GSS::Node *Parent : N->parents())
DFS(Parent, I + 1, DFS);
};
Expand Down Expand Up @@ -313,12 +317,11 @@ class GLRReduce {
FamilySequences.clear();
FamilyBases.clear();
do {
FamilySequences.emplace_back(Sequences.top().first.Rule,
*Sequences.top().second.Seq);
FamilyBases.emplace_back(
Params.Table.getGoToState(Sequences.top().second.Base->State,
F.Symbol),
Sequences.top().second.Base);
const PushSpec &Push = Sequences.top().second;
FamilySequences.emplace_back(Sequences.top().first.Rule, *Push.Seq);
for (const GSS::Node *Base : Push.LastPop->parents())
FamilyBases.emplace_back(
Params.Table.getGoToState(Base->State, F.Symbol), Base);

Sequences.pop();
} while (!Sequences.empty() && Sequences.top().first == F);
Expand Down

0 comments on commit 466eae6

Please sign in to comment.