[PATCH] D50828: Add profiling and canonicalization support to demangler nodes. No functionality change intended.

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 16 19:12:27 PDT 2018


Yea this approach is fine I guess, but I’m not really a fan of gigantic
headers, so if it’s possible to use runtime polymorphism that seems better
to me. Otoh, I’m not the maintainer of the itanium demangler, so feel free
to ignore :)
On Thu, Aug 16, 2018 at 7:09 PM Erik Pilkington via Phabricator <
reviews at reviews.llvm.org> wrote:

> erik.pilkington added a comment.
>
> In https://reviews.llvm.org/D50828#1203539, @rsmith wrote:
>
> > I'm going to try to split this patch so it adds a generic visitation
> mechanism to the demangler AST, and introduce a separate layer that
> includes Support and builds profiling off the visitation mechanism. That
> seems like the only way out that meets everyone's acceptance criteria.
> However, I don't see any way to do that without moving all of the demangler
> to a public header in include/llvm (since the whole thing will need to be
> included from both the Demangle/ demangler and the profiling demangler).
> That header would only be included (and the templates in it instantiated)
> in two places in LLVM, but still, it'd be a 5000-line header file. If
> people are strongly opposed to that approach, please shout now!
>
>
> +1 from me! I was just writing an argument to do something like this.
>
> One more thing that I want to mention: If/when we go monorepo, then we can
> share code between libcxxabi and llvm the old-fashioned way. Tangling in
> support would just make it impossible to ever de-duplicate the demanglers.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D50828
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180816/3c387a7e/attachment.html>


More information about the llvm-commits mailing list