[PATCH] D33548: [DWARF] - Cleanup relocation handling.

[David Blaikie via llvm-commits]

On Fri, May 26, 2017 at 8:57 AM Rui Ueyama via Phabricator <
reviews at reviews.llvm.org> wrote:

> ruiu added inline comments.
>
>
> ================
> Comment at: include/llvm/Object/RelocVisitor.h:62
> +                                            uint64_t Value);
> +  Visitor Vis;
> +
> ----------------
> grimar wrote:
> > ruiu wrote:
> > > Why don't you use std::function?
> > I am pretty sure I was asked by somebody to use poiner in one of reviews
> earlier, do not really remember why, so
> > I supposed it is preferred in LLVM before your comment, though it looks
> std::function is used widely. So I can switch to it.
> Well, maybe a bare pointer is smaller and faster than std::function, I
> guess? I'd guess David Blaikie knows more than me about that.
>

A good std::function implementation will at least use inline storage for
function pointers - but it may have some size and runtime overhead (might
use a virtual function (or at least a conditional of some kind) to dispatch
to a handler that then calls the function pointer - indeed that overhead
may be less if you pass a lambda instead of a function pointer (so it's one
indirection instead of two)).

I'm not too fussed/happy to use raw function pointers when it's
sufficiently constrained such that it's not "API surface" where you want to
support different callbacks with different needs, etc. But also don't mind
std::function - but probably worth measuring the relative performance if
the point of this is to improve perf.

- Dave


>
>
> https://reviews.llvm.org/D33548
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170526/a6f5d6ea/attachment.html>