On Sun, Mar 29, 2020 at 8:53 AM Bruno Haible <br...@clisp.org> wrote:
>
> Hi Jeffrey,
>
> > It looks like test-bitrotate.c is missing test cases. It is missing
> > the 32-bit rotl and rotr of 0-bits.
> >
> > The 0-bit rotate should tickle undefined behavior.
> >
> > If you want to clear the undefined behavior, then use this code. ...
>
> The functions are specified in bitrotate.h, e.g. like this:
>
> /* Given an unsigned 64-bit argument X, return the value corresponding
> to rotating the bits N steps to the left. N must be between 1 and
> 63 inclusive. */
> BITROTATE_INLINE uint64_t
> rotl64 (uint64_t x, int n)
>
> I think it is on purpose that N = 0 and N = 64 are not allowed. Namely,
> when N = 0 or N = 64, you would have a different, more efficient code
> branch anyway.
>
> > It will be compiled down to a single instruction on platforms like IA-32.
>
> Yes, this is the intent. And we should help the compiler produce good
> code, for example by adding statements like
> assume (n > 0 && n < 64);
> Allowing N = 0 or N = 64 goes backwards, because on some platforms it
> will prevent the compiler from choosing the best possible instruction.
Forgive my ignorance... No'oping 0 leaks timing information, and
no'oping 64 is undefined behavior. (And in the current implementation
No'oping 0 is also undefined behavior).
Is that what is desired?
I also don't think developers are going to write a rotate like:
if (n != 0)
x = rotr32(x, n);
Or maybe, I have never seen a shift or rotate written like that.
Jeff
