[PATCH] Use Rvalue refs in APInt

[Pete Cooper via llvm-commits]

> On May 31, 2016, at 11:13 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Tue, May 31, 2016 at 6:06 PM, Pete Cooper <peter_cooper at apple.com <mailto: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.
Hadn’t thought of that reasoning, but yeah, sounds like a good idea.
> 
> 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?).
Oh yeah, this is much better.  Thanks for the advice.  I’ll fix up the patch and get the other variants in there.

Cheers
Pete
>  
> 
> Cheers,
> Pete

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160601/88ebb412/attachment.html>