On Tue, 2018-01-16 at 14:25 +0100, Niels Möller wrote:
Nikos Mavrogiannopoulos n.mavrogiannopoulos@gmail.com writes:
On Mon, Jan 15, 2018 at 9:37 PM Niels Möller nisse@lysator.liu.se wrote:
unsigned overflow = b2 & 0x8000000000000000;b1 <<= 1;b2 <<= 1;if (overflow)b1 |= 0x01;I'd write the shift as
b1 = (b1 << 1) | (b2 >> 63); b2 <<= 1;
I have bad experience with shifts over 31. I could switch it to that, but my understanding is that they don't universally work especially in 32-bit systems offerring 64-bit quantities. Given that this code is being used unconditionally and is not performance critical, I'd stay with the safe version, unless you have strong opinion about it.
What problems have you seen? I've not heard of any C compilers which have uint64_t but doesn't support shift properly (and this is the simplest case, with unsigned values and constant shift count). The idiom I proposed is used all over the place in gmp, but there I guess we don't use 64-bit types on 32-bit platforms.
I do not seem to find any references for similar issues. Out of memory it was a report on the failure of a longer than 32-bit shift on an mips64 system. It's been long time ago and it may have been a compiler issue.
I attach a patch to switch to the 63-bit switch on top of the previous one in case you prefer that version (seems simpler).
On the other hand, I think
unsigned overflow = b2 & 0x8000000000000000; is incorrect if int is 32 bit, won't the result be always zero?
Thank you for the catch. Hopefully the x86 run on our CI would have caught it but I never run it there. I've now sent a build with the 0001 patch at: https://gitlab.com/nmav/nettle/pipelines/16256301
regards, Nikos