<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Oct 3, 2011, at 1:03 PM, Nick Lewycky wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div class="gmail_quote">On 3 October 2011 09:50, Jakob Stoklund Olesen <span dir="ltr"><<a href="mailto:stoklund@2pi.dk" target="_blank">stoklund@2pi.dk</a>></span> wrote:<br><div> </div><blockquote class="gmail_quote" style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; ">
+ const SCEV *CoeffTerm = getConstant(Ty, Coeff1*Coeff2);<br>
<br>
Can this multiplication overflow?<br></blockquote><div><br></div><div>Oh good point. I spent all this time thinking about what if Ty is an i3 and Coeff1*Coeff2 doesn't fit there, but not the case where Ty is larger than unsigned. The most straight-forward thing to do is reuse umul_ov.</div></div></blockquote><div><br></div><div>Or just promote to 64-bit. ScalarEvolution::getConstant() already takes a uint64_t.</div><br><blockquote type="cite"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; "><br>Finally, your x,y,z loop is O(N^4) in the number of SCEV operands. I don't know if this is a problem in practice.<br>
<br>
The Choose() calls in that loop are computing the same intermediate results over and over. You could get an O(N^3) computation by reusing those binomials.<br></blockquote><div><br></div><div>Both true, but not yet observed to be problems in practice. One alternative approach would be to allocate a binomial-coefficient scratch space and then use the recursive algorithm. I was really hoping to avoid putting more complex implementation for Choose into this patch, there's enough gnarly math in there as it is.</div></div></blockquote><div><br></div><div>Fair enough.</div><div><br></div><div>/jakob</div><div><br></div></div></body></html>