[PATCH] D97338: [Orc] Use extensible RTTI for the orc::ObjectLayer class hierarchy

Stefan Gränitz via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 26 00:29:36 PST 2021


sgraenitz added a comment.

In D97338#2588010 <https://reviews.llvm.org/D97338#2588010>, @dblaikie wrote:

> Could you describe more what you find off-putting about it?

First of all I think the current implementation is reasonable and pragmatic. CRTP adds artificial layers to the class hierarchy, which can be a pain for the reader and the debugger. Maybe it's personal preference, years ago I worked on a project that used CRTP heavily and it was a pain (pre C++14 inheriting ctors).

> This alternative, at least, looks like it uses an extra non-static member variable, compared to the original use of a static member. That's a fairly significant tradeoff/cost to add in terms of extra size to every derived object.

Well, I considered the idea promising. For each instance overhead is "length of inheritance chain" times "1 direct call for construction and 1 byte memory (plus alignment)" plus "one pointer in the root". Bad for small payloads and inner loops of course, but ORC ObjectLayers... Would be great, if it was possible with only the one extra member, but I didn't get rid of the extra `using` and that makes it odd. Another major concern is compile time overhead and additional archive size, as the whole template machinery gets pulled into Casting.h too. So, I guess there is not much coming out of my hack for now. Maybe on a lazy Sunday I get back to it :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97338



More information about the llvm-commits mailing list