Proper way for declaring binary rotation preprocessor macro

Question

I would like to create macros for binary rotations. My goal was to make those macros universal for both uint32_t and uint64_t operand types.

I came to this implementation:

#define ROTL(X, N)  (((X) << (N)) | ((X) >> (8 * sizeof(X) - (N))))
#define ROTR(X, N)  (((X) >> (N)) | ((X) << (8 * sizeof(X) - (N))))

Those macros work fine but gcc compiler produces warnings during compilation:

warning: right shift count >= width of type [enabled by default]
#define ROTL(X, N)  (((X) << (N)) | ((X) >> (8 * sizeof(X) - (N))))

warning: left shift count >= width of type [enabled by default]
#define ROTL(X, N)  (((X) << (N)) | ((X) >> (8 * sizeof(X) - (N))))

I know that compiler is complaining about possible mismatch between type of X and N. But warnings are produced even when I cast both X and N:

ROTL((uint32_t)0xdeadbeef, (uint32_t)0U);

What can I do to get rid of these warning the proper way?

This is not very efficient in terms of generated assembly code. If you need speed, you might want to use compiler intrinsics (heavily platform/compiler dependent). — Jabberwocky, Oct 02 '18 at 13:17
You may have a look to this https://stackoverflow.com/questions/776508/best-practices-for-circular-shift-rotate-operations-in-c — Giovanni Cerretani, Oct 02 '18 at 13:21
Since these macros evaluate each argument twice, be careful that you don't call them with arguments that have side effects. — Ian Abbott, Oct 02 '18 at 13:52
@Jabberwocky: Actually, pretty much all compilers are nowadays smart enough to understand what's going on with the macro, and replace it with a proper intrinsic (see https://godbolt.org/z/R7WQgM). You would get this with gcc, clang, mcvc. — vgru, Oct 02 '18 at 13:55
you need to show the actual macro calls to see why the compiler is giving you that warning. — Luis Colorado, Oct 05 '18 at 17:42

score 3 · Accepted Answer · edited Oct 02 '18 at 15:12

3

You get some problems when the second argument is zero or greater then the number of bits of the first argument. You may try to use the module to get rid of this problem. This seems to work, with this simple example:

#include <stdint.h>
#include <inttypes.h>
#include <stdio.h>
#include <limits.h>

#define ROTL(X, N) \
    (((X) << ((N) % (CHAR_BIT * sizeof(X)))) | \
     ((X) >> ((CHAR_BIT * sizeof(X) - (N)) % (CHAR_BIT * sizeof(X)))))

int main() {
    for (int i = 0; i < 100; i++) {
        uint32_t a = ROTL(UINT32_C(0xdeadbeef), i);
        printf("%d\t%"PRIX32"\n", i, a);
    }
    return 0;
}

I've used CHAR_BIT from limits.h and UINT32_C and PRIX32 from inttypes.h. You may adjust by yourself ROTR to do the same.

edited Oct 02 '18 at 15:12

chux - Reinstate Monica

143,097
13
135
256

answered Oct 02 '18 at 13:46

Giovanni Cerretani

1,693
1
16
30

You might shift by negative amounts if N is negative. Fixing that well is hard. Using a bit mask such as `& 0x1F` instead of modulus might work, but the mask has to be chosen correctly per type. – Jonathan Leffler Oct 02 '18 at 13:58
The mask should work, but is technically UB for signed types and negative values. Or treat a negative rotation as a rotate the other way. The compiler should optimise it all away when the rotate factor is a constant. – Gem Taylor Oct 02 '18 at 14:36
1

@JonathanLeffler Concerning [Fixing that well is hard](https://stackoverflow.com/questions/52608994/proper-way-for-declaring-binary-rotation-preprocessor-macro/52611163#comment92154124_52609697). I would recommend to `CHAR_BIT` --> `(uintmax_t)CHAR_BIT` to insure an unsigned `%` operation. – chux - Reinstate Monica Oct 02 '18 at 17:27

score 2 · Answer 2 · edited Oct 02 '18 at 13:41

2

You get a warning only when the second argument is zero. So, just do nothing if N == 0:

#define ROTL(X, N)  ((N) == 0 ? (X) : (((X) << (N)) | ((X) >> (8 * sizeof(X) - (N)))))

edited Oct 02 '18 at 13:41

Giovanni Cerretani

1,693
1
16
30

answered Oct 02 '18 at 13:18

ReAl

1,231
1
8
19

How does this answer the question? – Jabberwocky Oct 02 '18 at 13:21
This version of macro answers to `What can I do to get rid of these warning the proper way?`, both for `uint32_t` and `uint64_t` – ReAl Oct 02 '18 at 13:23
1

He gets a warning only when the second argument is zero. So handling it in a special way is, actually, a solution to the problem. – Giovanni Cerretani Oct 02 '18 at 13:24
1

@Giovanni it doesn't handle this case: `ROTL(x, 32)` – Jabberwocky Oct 02 '18 at 13:25
@Jabberwocky The same warning will be for `(x << 32)` without _any_ macro – ReAl Oct 02 '18 at 13:26
2

Well, actually the second argument must be in [0, (sizeof(X) * 8 - 1)], otherwise the algorithm does not work at all. – Giovanni Cerretani Oct 02 '18 at 13:28

0___________ · Answer 3 · 2018-10-02T13:13:53.093

0

cast the shifted operand to the max possible width.

#define ROTL(X, N)  (((uint64_t)(X) << (N)) | ((uint64_t)(X) >> (8 * sizeof(X) - (N))))

edited Oct 02 '18 at 13:13

answered Oct 02 '18 at 13:12

0___________

60,014
4
34
74

3

This solution may works for `uint32_t` as input, but is not general, as produces the same warning when using `uint64_t` instead of `uint32_t`. – Giovanni Cerretani Oct 02 '18 at 13:19

Proper way for declaring binary rotation preprocessor macro

3 Answers3