<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On May 31, 2016, at 11:13 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><br class="Apple-interchange-newline"><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On Tue, May 31, 2016 at 6:06 PM, Pete Cooper<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:peter_cooper@apple.com" target="_blank" class="">peter_cooper@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">Hi David, Chandler<br class=""><br class="">I’ve been measuring the number of allocations in LLVM and see millions of APInt’s in the trace.<br class=""><br class="">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 class=""><br class="">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 class=""><br class="">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 class=""><br class="">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 class="">- LHS is APInt&&, RHS is APInt&<br class="">- LHS is APInt&, RHS is APInt&&<br class="">- LHS is APInt&&, RHS is APInt&&<br class=""><br class="">I’m happy to do this, but it may eventually become unwieldy.<br class=""><br class="">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 class=""></blockquote><div class=""> </div><div class="">Brief glance at the patch seems like it's in the direction I'd expect.<br class=""><br class="">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 class=""></div></div></div></blockquote>Hadn’t thought of that reasoning, but yeah, sounds like a good idea.<br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class=""><br class="">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 class=""><br class="">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 class=""></div></div></div></blockquote>Oh yeah, this is much better. Thanks for the advice. I’ll fix up the patch and get the other variants in there.</div><div><br class=""></div><div>Cheers</div><div>Pete<br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><br class="">Cheers,<br class="">Pete</blockquote></div></div></blockquote></div><br class=""></body></html>