This is an archive of the discontinued LLVM Phabricator instance.

[pseudo] Simplify the glrReduce implementation.
ClosedPublic

Authored by hokein on Jun 8 2022, 3:11 AM.

Details

Summary

glrReduce maintains two priority queues (one for bases, and the other
for Sequence), these queues are in parallel with each other, corresponding to a
single family. They can be folded into one.

This patch removes the bases priority queue, which improves the glrParse by
10%.

ASTReader.cpp: 2.03M/s (before) vs 2.26M/s (after)

Diff Detail

Event Timeline

hokein created this revision.Jun 8 2022, 3:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 8 2022, 3:11 AM
hokein requested review of this revision.Jun 8 2022, 3:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 8 2022, 3:11 AM
sammccall accepted this revision.Jun 8 2022, 4:31 AM

Very nice! Not sure why I didn't see this before :-)

clang-tools-extra/pseudo/lib/GLR.cpp
286–291

This name is a bit vague/abstract.

Maybe RootedSequence, or PushSpec?

287

nit: A base node is the head...
(no reason to talk about them collectively in this context)

288

the second part of this comment belongs below where we decorrelate the two (under "grab the sequences and bases for each family"), not here

This revision is now accepted and ready to land.Jun 8 2022, 4:31 AM
hokein updated this revision to Diff 435463.Jun 9 2022, 2:28 AM
hokein marked 3 inline comments as done.

address comments

This revision was landed with ongoing or failed builds.Jun 9 2022, 2:29 AM
This revision was automatically updated to reflect the committed changes.