0

Why does the following C code not work for returning -1 for negative numbers, 0 for 0s, and 1 for positive numbers?

(((x >> 31) ^ -1) + 1) | (!x ^ 1);

Specifically, when I pass in negative numbers, it returns 1. It seems like if I have a negative number, though (i.e., the the least significant bit is a 1 after the 31 bit shift), XORing it with -1 will give me -2 (i.e., all 1s and a 0 in the least significant bit location), and adding 1 would make it -1.

user1427661
  • 11,158
  • 28
  • 90
  • 132
  • Hint: Is `x` signed or unsigned? – Mysticial Apr 22 '13 at 14:13
  • This is not working because it's not portable. You shall not be making assumptions about the binary representation of values. –  Apr 22 '13 at 14:16
  • I'm sure this is answered elsewhere, but until I find a suitable duplicate: right-shifting a signed operand _may_ fill the new top bits in order to retain the sign. – Useless Apr 22 '13 at 14:16
  • 1
    Oh, and _verifying_ your assumptions about intermediate values would have saved you some time. Why not just see what `x >> 31` actually is? – Useless Apr 22 '13 at 14:18

2 Answers2

1

>> will generally do arithmetic shift on signed data, so ((-1) >> 31) == (-1), contrary to your assumption. As pointed out by others, this is not guaranteed by the standard, but it is most likely true on modern systems. In any case, be careful with this type of bit twiddling. If portability is a concern or speed is not, you should do it a different way. See Is there a standard sign function (signum, sgn) in C/C++? for some ideas.

Community
  • 1
  • 1
jerry
  • 2,581
  • 1
  • 21
  • 32
1

According to the C99 standard, the result of x >> n if x is negative is implementation defined. So the reason you are having a problem depends on your compiler and architecture.

However, it's most likely that the x is sign extended when you shift it i.e. the top bit is repeated to keep the sign the same as the operand. This is what happens with my compiler. So for any negative number, x >> 31 is -1. Also, for any non zero number !x is 0 (i.e. false). This applies assuming x is a 32 bit integer. If you make x an unsigned int, it should work, but consider the following alternative:

(x < 0) ? -1 : ((x > 0) ? 1 : 0)

which I think is a bit less cryptic.

And here is a program that you can use to see what your expression is doing

#include <stdio.h>
#define EVALUATE(x)     printf("%s = %d\n", #x, x)
int main(int argc, char** argv)
{
    unsigned        int x = 51;

    EVALUATE(x >> 31);
    EVALUATE(((x >> 31) ^ -1));
    EVALUATE(((x >> 31) ^ -1) + 1);
    EVALUATE(!x);
    EVALUATE(!x ^ 1);
    EVALUATE((((x >> 31) ^ -1) + 1) | (!x ^ 1));
    return 0;
}
JeremyP
  • 84,577
  • 15
  • 123
  • 161
  • What's the rationale behind the `int` casts? – jerry Apr 22 '13 at 18:44
  • @jerry I was thinking if x is an unsigned integer, you'd still get the correct result, but that's bollocks now I think about it. If x is unsigned, you don't want to ever get -1. In fact, the int cast would suppress a warning, so I'm editing the answer to remove it. – JeremyP Apr 23 '13 at 08:36
  • It would be even worse than that. If `x` is `unsigned` and `> INT_MAX`, casting it to a `signed` would be implementation defined behavior. You're right that *likely* what would happen is it would become a negative number and incorrectly yield `-1`, but it could also raise a signal. Without the cast, there shouldn't be a warning, the usual arithmetic conversion will be performed and you should get the correct result for any integral type. – jerry Apr 23 '13 at 12:00