<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 31, 2016 at 6:06 PM, Pete Cooper <span dir="ltr"><<a href="mailto:peter_cooper@apple.com" target="_blank">peter_cooper@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi David, Chandler<br>
<br>
I’ve been measuring the number of allocations in LLVM and see millions of APInt’s in the trace.<br>
<br>
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).<br>
<br>
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.<br>
<br>
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.<br>
<br>
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.:<br>
- LHS is APInt&&, RHS is APInt&<br>
- LHS is APInt&, RHS is APInt&&<br>
- LHS is APInt&&, RHS is APInt&&<br>
<br>
I’m happy to do this, but it may eventually become unwieldy.<br>
<br>
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?<br></blockquote><div> </div><div>Brief glance at the patch seems like it's in the direction I'd expect.<br><br>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.<br><br>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)<br><br>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?).<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Cheers,<br>
Pete<br>
<br>
</blockquote></div><br></div></div>