Skip to content

Commit 466eae6

Browse files
committed
[pseudo] Store last node popped in the queue, not its parent(s). NFC
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
1 parent c078e46 commit 466eae6

File tree

1 file changed

+14
-11
lines changed
  • clang-tools-extra/pseudo/lib

1 file changed

+14
-11
lines changed

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

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,9 @@ class GLRReduce {
215215
// They specify a sequence ForestNode we may build (but we dedup first).
216216
// (The RuleID is not stored here, but rather in the Family).
217217
struct PushSpec {
218-
// A base node is the head after popping the GSS nodes we are reducing.
219-
const GSS::Node* Base = nullptr;
218+
// The last node popped before pushing. Its parent is the reduction base(s).
219+
// (Base is more fundamental, but this is cheaper to store).
220+
const GSS::Node* LastPop = nullptr;
220221
Sequence *Seq = nullptr;
221222
};
222223
KeyedQueue<Family, PushSpec> Sequences; // FIXME: rename => PendingPushes?
@@ -256,9 +257,13 @@ class GLRReduce {
256257
Family F{/*Start=*/0, /*Symbol=*/Rule.Target, /*Rule=*/RID};
257258
TempSequence.resize_for_overwrite(Rule.Size);
258259
auto DFS = [&](const GSS::Node *N, unsigned I, auto &DFS) {
259-
if (I == Rule.Size) {
260+
TempSequence[Rule.Size - 1 - I] = N->Payload;
261+
if (I + 1 == Rule.Size) {
260262
F.Start = TempSequence.front()->startTokenIndex();
261-
LLVM_DEBUG(llvm::dbgs() << " --> base at S" << N->State << "\n");
263+
LLVM_DEBUG({
264+
for (const auto *B : N->parents())
265+
llvm::dbgs() << " --> base at S" << B->State << "\n";
266+
});
262267

263268
// Copy the chain to stable storage so it can be enqueued.
264269
if (SequenceStorageCount == SequenceStorage.size())
@@ -269,7 +274,6 @@ class GLRReduce {
269274
Sequences.emplace(F, PushSpec{N, Seq});
270275
return;
271276
}
272-
TempSequence[Rule.Size - 1 - I] = N->Payload;
273277
for (const GSS::Node *Parent : N->parents())
274278
DFS(Parent, I + 1, DFS);
275279
};
@@ -313,12 +317,11 @@ class GLRReduce {
313317
FamilySequences.clear();
314318
FamilyBases.clear();
315319
do {
316-
FamilySequences.emplace_back(Sequences.top().first.Rule,
317-
*Sequences.top().second.Seq);
318-
FamilyBases.emplace_back(
319-
Params.Table.getGoToState(Sequences.top().second.Base->State,
320-
F.Symbol),
321-
Sequences.top().second.Base);
320+
const PushSpec &Push = Sequences.top().second;
321+
FamilySequences.emplace_back(Sequences.top().first.Rule, *Push.Seq);
322+
for (const GSS::Node *Base : Push.LastPop->parents())
323+
FamilyBases.emplace_back(
324+
Params.Table.getGoToState(Base->State, F.Symbol), Base);
322325

323326
Sequences.pop();
324327
} while (!Sequences.empty() && Sequences.top().first == F);

0 commit comments

Comments
 (0)