[PATCH] D128307: [pseudo] Store reduction sequences by pointer in heaps, instead of by value.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:270
+
+        Sequences.emplace(F, PushSpec{N, Seq});
         return;
----------------
sammccall wrote:
> hokein wrote:
> > 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()`).
> This is interesting! Essentially the reason we do the DFS eagerly (and pay to store its results) is we have to establish the Family via the start token, but this means we have to go only as far as the end of the sequence, rather than one further. (One further feels "cleaner" somehow, but who cares...)
> 
> I'll give this a try (not in this patch)
This is a ~5% speedup, for almost no extra code (just some comments).
Landed as 466eae6aa3574aea6604d42666f099025718ffa4


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128307/new/

https://reviews.llvm.org/D128307