Re: test-bitrotate.c missing test cases

[Jeffrey Walton]

On Sun, Mar 29, 2020 at 8:53 AM Bruno Haible <address@hidden> 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