Messages in this thread | | | From | Måns Rullgård <> | Subject | Re: [RFC][PATCH 2/3] math128: Introduce {mult,add,cmp}_u128 | Date | Tue, 24 Apr 2012 21:12:20 +0100 |
| |
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Tue, Apr 24, 2012 at 9:10 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote: >> Grow rudimentary u128 support without relying on gcc/libgcc. >> >> +#ifndef add_u128 >> +static inline u128 add_u128(u128 a, u128 b) >> +{ >> + u128 res; >> + >> + res.hi = a.hi + b.hi; >> + res.lo = a.lo + b.lo; >> + >> + if (res.lo < a.lo || res.lo < b.lo) >> + res.hi++; > > This is wrong. Or at least stupid. > > Just do one of the comparisons, not both. If overflow occurs, the > result will be smaller than *either* of the added numbers, so > comparing both is just silly and confused. > > So just pick one. > > Also, it might be worth looking at code generation, to see if it's > better to just do > > a.hi += b.hi; > a.low += b.low; > if (a.low < b.low) > a.hi++; > return a;
I have no idea if it makes a difference, but that if statement can be written as a.hi += a.low < b.low. Just an observation.
> because that might make it clear that there are fewer actual values > live at any particular time. But gcc may not care. Try it. > > Also, for the multiply, please make sure gcc knows to do a "32x32->64" > multiplication, rather than thinking it needs to do full 64x64 > multiplies..
On ARM it does the right thing at least since 4.3, which is the oldest ARM compiler I have at hand.
-- Måns Rullgård mans@mansr.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |