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:208
+ // Underlying storage for sequences pointed to by stored SequenceRefs.
+ std::deque<Sequence> SequenceStorage;
+ // We don't actually destroy the sequences between calls, to reuse storage.
----------------
I start to have a nervous feeling that we're going to add more and more
intermediate data structure, which increases the complexity of the code, but I
think the improvement is large enough to justify.
================
Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:220
const GSS::Node* Base = nullptr;
- Sequence Seq;
+ Sequence *Seq;
};
----------------
nit: add a default value nullptr
================
Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:270
+
+ Sequences.emplace(F, PushSpec{N, Seq});
return;
----------------
Just an idea (no action required)
If we want to do further optmization, here if a sequence have multiple Bases,
we will have multiple elements in `Sequences` -- they have the form of (F,
PushSpec{N, Seq}) where only N (the base node) is different.
- if we change the PushSpec::Base to store the child node of Base, we could
save a bunch of space in Sequences. (The base can be retrieved dynamically from
the `child->parents()`).
================
Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:317
FamilySequences.emplace_back(Sequences.top().first.Rule,
- Sequences.top().second.Seq);
+ *Sequences.top().second.Seq); // XXX
FamilyBases.emplace_back(
----------------
What's XXX?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128307/new/
https://reviews.llvm.org/D128307
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
