| Message ID | 1377159632-7446-2-git-send-email-aik@ozlabs.ru |
|---|---|
| State | New |
| Headers | show |
Il 22/08/2013 10:20, Alexey Kardashevskiy ha scritto: > This adds macro to extend signed 64bit value to signed 128bit value. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > include/qemu/int128.h | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/include/qemu/int128.h b/include/qemu/int128.h > index 9ed47aa..987a1a9 100644 > --- a/include/qemu/int128.h > +++ b/include/qemu/int128.h > @@ -38,6 +38,11 @@ static inline Int128 int128_2_64(void) > return (Int128) { 0, 1 }; > } > > +static inline Int128 int128_exts64(int64_t a) > +{ > + return (Int128) { .lo = a, .hi = (a >> 63) ? -1 : 0 }; > +} The "? -1 : 0" is not necessary, but the compiler will remove it at -O1 or more (interestingly, or -O0 it will remove the shift and leave the conditional!). > static inline Int128 int128_and(Int128 a, Int128 b) > { > return (Int128) { a.lo & b.lo, a.hi & b.hi }; > Acked-by: Paolo Bonzini <pbonzini@redhat.com> Paolo
On 22 August 2013 10:09, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 22/08/2013 10:20, Alexey Kardashevskiy ha scritto: >> +static inline Int128 int128_exts64(int64_t a) >> +{ >> + return (Int128) { .lo = a, .hi = (a >> 63) ? -1 : 0 }; >> +} > > The "? -1 : 0" is not necessary, but the compiler will remove it at -O1 > or more (interestingly, or -O0 it will remove the shift and leave the > conditional!). We can avoid relying on implementation defined behaviour here by using .hi = (a < 0) ? -1 : 0; (I know we allow ourselves to assume right-shift of signed ints is arithmetic shift, but I think it's nicer to avoid it unless it really makes the code better.) -- PMM
Il 22/08/2013 11:47, Peter Maydell ha scritto: > On 22 August 2013 10:09, Paolo Bonzini <pbonzini@redhat.com> wrote: >> Il 22/08/2013 10:20, Alexey Kardashevskiy ha scritto: >>> +static inline Int128 int128_exts64(int64_t a) >>> +{ >>> + return (Int128) { .lo = a, .hi = (a >> 63) ? -1 : 0 }; >>> +} >> >> The "? -1 : 0" is not necessary, but the compiler will remove it at -O1 >> or more (interestingly, or -O0 it will remove the shift and leave the >> conditional!). > > We can avoid relying on implementation defined > behaviour here by using > .hi = (a < 0) ? -1 : 0; > > (I know we allow ourselves to assume right-shift of signed > ints is arithmetic shift, but I think it's nicer to avoid it unless > it really makes the code better.) This is what Alexey proposed. I suggested (a >> 63) without the ?: but he misunderstood my (probably not clear enough) suggestion. Paolo
On 22 August 2013 10:48, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 22/08/2013 11:47, Peter Maydell ha scritto: >> We can avoid relying on implementation defined >> behaviour here by using >> .hi = (a < 0) ? -1 : 0; >> >> (I know we allow ourselves to assume right-shift of signed >> ints is arithmetic shift, but I think it's nicer to avoid it unless >> it really makes the code better.) > > This is what Alexey proposed. I suggested (a >> 63) without the ?: but > he misunderstood my (probably not clear enough) suggestion. Yes, I found that email thread after sending this. I think the (a < 0) variant is better than using a shift (with or without the ?: operator). -- PMM
On 08/22/2013 07:48 PM, Paolo Bonzini wrote: > Il 22/08/2013 11:47, Peter Maydell ha scritto: >> On 22 August 2013 10:09, Paolo Bonzini <pbonzini@redhat.com> wrote: >>> Il 22/08/2013 10:20, Alexey Kardashevskiy ha scritto: >>>> +static inline Int128 int128_exts64(int64_t a) >>>> +{ >>>> + return (Int128) { .lo = a, .hi = (a >> 63) ? -1 : 0 }; >>>> +} >>> >>> The "? -1 : 0" is not necessary, but the compiler will remove it at -O1 >>> or more (interestingly, or -O0 it will remove the shift and leave the >>> conditional!). >> >> We can avoid relying on implementation defined >> behaviour here by using >> .hi = (a < 0) ? -1 : 0; >> >> (I know we allow ourselves to assume right-shift of signed >> ints is arithmetic shift, but I think it's nicer to avoid it unless >> it really makes the code better.) > > This is what Alexey proposed. I suggested (a >> 63) without the ?: but > he misunderstood my (probably not clear enough) suggestion. Yes, I misunderstood. It was not obvious to me that (signed long long)-1>>63 will be still -1. I really (really) envy people who can easily read stuff like but I cannot :( 1) return (Int128) { .lo = a, .hi = (a < 0) ? -1 : 0 }; 2) return (Int128) { .lo = a, .hi = (a < 0) }; 3) return (Int128) { .lo = a, .hi = a >> 63 }; So with which one should I repost the patch?
Il 22/08/2013 12:50, Alexey Kardashevskiy ha scritto: > 1) return (Int128) { .lo = a, .hi = (a < 0) ? -1 : 0 }; > 2) return (Int128) { .lo = a, .hi = (a < 0) }; > 3) return (Int128) { .lo = a, .hi = a >> 63 }; > > So with which one should I repost the patch? The second would be "-(a < 0)" actually. Peter wants (1) I think, so go for it. Paolo
diff --git a/include/qemu/int128.h b/include/qemu/int128.h index 9ed47aa..987a1a9 100644 --- a/include/qemu/int128.h +++ b/include/qemu/int128.h @@ -38,6 +38,11 @@ static inline Int128 int128_2_64(void) return (Int128) { 0, 1 }; } +static inline Int128 int128_exts64(int64_t a) +{ + return (Int128) { .lo = a, .hi = (a >> 63) ? -1 : 0 }; +} + static inline Int128 int128_and(Int128 a, Int128 b) { return (Int128) { a.lo & b.lo, a.hi & b.hi };
This adds macro to extend signed 64bit value to signed 128bit value. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- include/qemu/int128.h | 5 +++++ 1 file changed, 5 insertions(+)