[PATCH] Use Rvalue refs in APInt

[David Blaikie via llvm-commits]

On Tue, May 31, 2016 at 6:06 PM, Pete Cooper <peter_cooper at apple.com> wrote:

> Hi David, Chandler
>
> I’ve been measuring the number of allocations in LLVM and see millions of
> APInt’s in the trace.
>
> The majority of the traces come from things like ConstantRange where its
> common to see expressions like 'this_max * Other_max + 1’ (from
> ConstantRange::multiply).
>
> This patch attempts to reduce the number of allocations with r-value refs
> on expressions such as these.  Using the bitcode for verify-uselistorder,
> this reduces the total number of allocations with ‘opt -O2
> verify-uselistorder’ from 19.5m to 19.1m.
>
> There are also compiler time wins to be had.  Before I started reducing
> the allocations, ConstantRange::multiply was 6.5% of ‘opt -O2’ time on this
> use case, and after a series of patches to reduce its allocations from 26m
> to 16m, it drops to about 2.5% of compile time.
>
> I’ve only covered the case where the LHS is an APInt&& and RHS is a
> uint64_t.  I could add in all the possible cases, i.e.:
> - LHS is APInt&&, RHS is APInt&
> - LHS is APInt&, RHS is APInt&&
> - LHS is APInt&&, RHS is APInt&&
>
> I’m happy to do this, but it may eventually become unwieldy.
>
> What do you think of this particular patch, and then what do you think
> about going forwards whether its a good idea to cover the other cases with
> r-value refs?
>

Brief glance at the patch seems like it's in the direction I'd expect.

I think it's important to make sure all operations are covered equally - or
we'll end up chasing down performance issues when someone happened to use
the operands in the "bad" way through no fault of their own.

One thing to do would be to try to reduce the number of interesting
operations to reduce the code bloat. eg: anything commutative, just
implement in terms of the other (T operator+(const T& LHS, T &&RHS) {
return std::move(RHS) + LHS; } etc... and implement binary - in terms of
unary - and binary + perhaps, etc)

Also, any member overload that can be a non-member (like binary +) should
be a non-member. Then there won't be any need to deal with the
THIS_RVALUE_REF stuff for most of it (still for things that have to be
members, like possibly the unary operators?).


>
> Cheers,
> Pete
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160531/72ea6119/attachment.html>