Incorrect behavior and documentation for xsimd::rotate_left/xsimd::rotate_right

[DamRsn]

I had to use xsimd::rotate_left/xsimd::rotate_right recently in order to get the following behavior: {0.0f, 1.0f, 2.0f, 3.0f} -> {1.0f, 2.0f, 3.0f, 0.0f}.

To me, this appears like I need to do a left rotation by one element, or by sizeof(float) (4) bytes.

The documentation for xsimd::rotate_left states:

Slide the whole batch to the left by \c n bytes, and reintroduce the
slided out elements from the right. This is different from
\c rol that rotates each batch element to the left.

@tparam N Amount of bytes to rotated to the left.
@param x batch of integer values.
@return rotated batch.

Setup: I'm on mac (arm) and using batch<float> x(0, 1, 2, 3). batch<float>::size is 4 and sizeof(float) is 4 bytes. I'm using the latest version (13.0.0)

First I searched for rol but couldn't find it.

So I tried to do: xsimd::rotate_left<sizeof(float)>(x) but this gives back the same unchanged batch. Then I tried to
change the rotation amount to 1 and I got a right rotation of 1 (-> {3, 0, 1, 2})

Sidenote: xsimd::rotate_right<4>(x) does not compile while xsimd::rotate_left<4>(x) does.

So to get what I wanted, I had to do: xsimd::rotate_right<1>(x), which seem weird according to both the function name and the documentation.

Two things appears to be wrong in the documentation:

• the template parameter sets the rotation amount in number of elements (not in number of bytes as indicated)
• left and right rotation seems to be inverted. I'm less sure about this one as it's more of a convention, but the documentation indicates that in a left rotation, elements are reintroduced from the right, which is not the case.

Here's a reproducible example and its output (ran it on mac arm and on windows x86_64).

#include "xsimd/xsimd.hpp"
#include <iostream>

int main()
{
    const xsimd::batch<float> x(0.0f, 1.0f, 2.0f, 3.0f);
    std::cout << "x: " << x << std::endl;

    // Rotate left by 1
    const auto y_rotate_left_1 = xsimd::rotate_left<1>(x);
    std::cout << "Rotate left by 1: " << y_rotate_left_1 << std::endl;

    // Rotate left by 4
    const auto y_rotate_left_sizeoffloat = xsimd::rotate_left<sizeof(float)>(x);
    std::cout << "Rotate left by sizeof(float): " << y_rotate_left_sizeoffloat << std::endl;

    // Rotate right by 1
    const auto y_rotate_right_1 = xsimd::rotate_right<1>(x);
    std::cout << "Rotate right by 1: " << y_rotate_right_1 << std::endl;

    // Does not compile because: "error: argument value 4 is outside the valid range [0, 3]"
    // Rotate right by 4
    // const auto y_rotate_right_sizeoffloat = xsimd::rotate_right<sizeof(float)>(x);
    // std::cout << "Rotate right by sizeof(float): " << y_rotate_right_sizeoffloat << std::endl;

    return 0;
}

Output:

x: (0, 1, 2, 3)
Rotate left by 1: (3, 0, 1, 2)
Rotate left by sizeof(float): (0, 1, 2, 3)
Rotate right by 1: (1, 2, 3, 0)

What do you think is the best way to address this? I'm happy to help if I can.
IMHO, I think that the behavior of rotate_left and rotate_right should be swapped, and the documentation updated. But that be a breaking change.

Anyway, thanks for making xsimd, I love to work with this library!

[serge-sans-paille]

According to https://en.wikipedia.org/wiki/Left_rotation rotate left should result in rol({0, 1, 2, 3}) -> {1, 2, 3, 0}. So you're right. I'll fix that and submit a release because it indeed breaks the API.

[DamRsn]

Great thanks!

[serge-sans-paille]

Can you confirm #1065 works as expected?

[serge-sans-paille]

@JohanMabille we'll have to do a minor release after this one, I think.

[DamRsn]

Can you confirm #1065 works as expected?

I've tried with my simple example and it now works as expected.

The updated documentation still mentions the rol function:

This is different from rol that rotates each batch element to the left.

but there's no such function in xsimd. What does this refer to? Maybe this mention should be removed.

Anyway thanks for fixing this!

[serge-sans-paille]

doc fixed, it should mentioning rotr and rotl.