On Wed, Apr 22, 2009 at 4:28 PM, Freeland Abbott <[email protected]> wrote: > [+gwtc] > > LGTM, although I had to do some work to convince myself it was safe. > > You might consider updating the comment; what I find more quickly > understandable and verifiable is something like: > > If n<32, a[HIGH]/shiftFact is guaranteed to be an integer already. For > n>32, a[HIGH]/shiftFact will have fractional bits, but we need to discard > them as they shift away. (We will end up discarding all of a[LOW] in this > case, as it divides out to entirely fractional.)
That's a much clearer explanation. Changed. > I assume the branch to test for high-n (and work only with a[HIGH] if so) is > much more expensive than the savings by avoiding the division. I hadn't thought of that. The difference in speed shouldn't be much in comparison to the create() call, which is pretty expensive. So, it's just a matter of which version is simpler. I'm honestly not sure which is simpler. I have gone ahead and put it in, at r5270, just to close the bug. If you think the version with a branch would be clearer, then let's change it. Lex --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
