Intel® C++ Compiler
Support and discussions for creating C++ code that runs on platforms based on Intel® processors.

Remark #2259

kfsone
New Contributor I
491 Views

I'm porting code from GCC to ICC, the following code demonstrates that icc appears to favor an implied data type when using bitwise or and shift operators rather than working in the constraints of the data being manipulated. This introduces a new potential for errors I'll explain after the code:

[cpp]typedef unsigned short int UINT16;
typedef unsigned char UINT8;

int main(int argc, char* argv[])
{
  UINT8 x = 5;
  UINT8 y = 9;
  UINT16 a;

  a = x << 8;   // Generates #2259

  a = (UINT16) x << 8 ; // Generates #2259

  a = (((UINT16)x) << 8) | (((UINT16)y) << 8); // Generates #2259
}
[/cpp]

GCC andVisual C are fine with this code on all levels of warnings. However, icc appears to switch to "int" as the data type when performing shift and bitwise or operations, it doesn't even honor the unsigned specifier.

test.cpp(10): remark #2259: non-pointer conversion from "int" to "UINT16={unsigned short}" may lose significant bits
a = x << 8; // Generates #2259
^

test.cpp(12): remark #2259: non-pointer conversion from "int" to "UINT16={unsigned short}" may lose significant bits
a = (UINT16) x << 8 ; // Generates #2259
^

test.cpp(14): remark #2259: non-pointer conversion from "int" to "UINT16={unsigned short}" may lose significant bits
a = (((UINT16)x) << 8) | (((UINT16)y) << 8); // Generates #2259
^

^

The new opportunity for error is the first warning. Let me make it a little more obvious

[cpp]int main(int argc, char* argv[])
{
  unsigned char uchar = 10;
  unsigned short ushort = uchar << 16;
}
[/cpp]

If you maintained the type of the object having shift, or, etc, applied, you could perform sanity checks such as "shift operation exceeds bit size of value being shifted". If this was part of some complex folding operation, I could cast to work around:

unsigned short ushort = ((int)uchar << 16) >> (rand() % 8);

0 Kudos
1 Solution
ILevi1
Valued Contributor I
491 Views
Quoting - tim18

I'm not sure what your complaint is. Do you have a case which shows a difference in result between gcc and icc? Do you mean that the warning should say "unsigned int" ? Your code appears to require promotion to unsigned int, or to work as if it did. Even if you change the source so that unsigned short int would be permitted explicitly, I don't think either compiler will do anything different.

Tim, I think he wants to say that the compiler is promoting unsigned char to int before shifting and then truncates it to the unsigned short.

I know that such behavior is part of a language standard, but ICC produces completely meaningless remark because original type (as opposed to original type needlessly promoted to int) fits in the LHS with the shift amount used.

View solution in original post

9 Replies
TimP
Black Belt
491 Views

I'm not sure what your complaint is. Do you have a case which shows a difference in result between gcc and icc? Do you mean that the warning should say "unsigned int" ? Your code appears to require promotion to unsigned int, or to work as if it did. Even if you change the source so that unsigned short int would be permitted explicitly, I don't think either compiler will do anything different.

kfsone
New Contributor I
491 Views
Quoting - tim18

I'm not sure what your complaint is. Do you have a case which shows a difference in result between gcc and icc? Do you mean that the warning should say "unsigned int" ? Your code appears to require promotion to unsigned int, or to work as if it did. Even if you change the source so that unsigned short int would be permitted explicitly, I don't think either compiler will do anything different.


The first example compiles with g++ -Wall -pedantic without an error or warning (VS 2008, gcc 4.1.2). But under icc it generates the warning.

ILevi1
Valued Contributor I
492 Views
Quoting - tim18

I'm not sure what your complaint is. Do you have a case which shows a difference in result between gcc and icc? Do you mean that the warning should say "unsigned int" ? Your code appears to require promotion to unsigned int, or to work as if it did. Even if you change the source so that unsigned short int would be permitted explicitly, I don't think either compiler will do anything different.

Tim, I think he wants to say that the compiler is promoting unsigned char to int before shifting and then truncates it to the unsigned short.

I know that such behavior is part of a language standard, but ICC produces completely meaningless remark because original type (as opposed to original type needlessly promoted to int) fits in the LHS with the shift amount used.

TimP
Black Belt
491 Views
Quoting - Igor Levicki

Tim, I think he wants to say that the compiler is promoting unsigned char to int before shifting and then truncates it to the unsigned short.

I know that such behavior is part of a language standard, but ICC produces completely meaningless remark because original type (as opposed to original type needlessly promoted to int) fits in the LHS with the shift amount used.

Igor,

That's agood point.Cleaning up the warningwould require that the compiler take into account where the operand came from. Maybe that would be a reason for gcc not issuing such warnings.

JenniferJ
Moderator
491 Views
I've entered a bug report regarding the first remark. When there's any update, I'll post a news here.
Thanks for the posting.
Jennifer
JenniferJ
Moderator
491 Views
Hi,
There's an update to this remark. The reason for the 1st one is because that the operation on the right is done in 32bits. Meaning that there's an implicit conversion from 8 bits to 32 bits. So when assigning a 32 bits to 16 bits, a remark is emitted.
btw the implicate conversion to 32bits is standard. In default mode you won't see this remark, only when you want to see all the possible remarks with /W4 or /Wall option, you will get it.
So we can not change the compiler to behave like others in this area.
Jennifer
kfsone
New Contributor I
491 Views
Hi,
There's an update to this remark. The reason for the 1st one is because that the operation on the right is done in 32bits. Meaning that there's an implicit conversion from 8 bits to 32 bits. So when assigning a 32 bits to 16 bits, a remark is emitted.
btw the implicate conversion to 32bits is standard. In default mode you won't see this remark, only when you want to see all the possible remarks with /W4 or /Wall option, you will get it.
So we can not change the compiler to behave like others in this area.
Jennifer

Peculiar: GCC 3, GCC 4, GCC 4.1, Borland (CBuilder) and Microsoft all seem to inherit types from the left side, as in:

[cpp]unsigned int now = 123456 ; // UNSIGNED
unsigned int offset = 10 ; // UNSIGNED
         int received = 123440 ; // **SIGNED**

bool expired = ( (received + offset) - now < 0 ) ;[/cpp]

All of the above compilers understand this correctly and produce expired = true because the signedness of "int" propogates down to the right.

ICC complains about the pointless comparison of an unsigned value to zero.

(It /seems/ to be related)

ILevi1
Valued Contributor I
491 Views
Hi,
There's an update to this remark. The reason for the 1st one is because that the operation on the right is done in 32bits. Meaning that there's an implicit conversion from 8 bits to 32 bits. So when assigning a 32 bits to 16 bits, a remark is emitted.
btw the implicate conversion to 32bits is standard. In default mode you won't see this remark, only when you want to see all the possible remarks with /W4 or /Wall option, you will get it.
So we can not change the compiler to behave like others in this area.
Jennifer

Jennifer,

Your explanation is exactly the same as mine.

However, I have to disagree (again!) with Intel's compiler engineers!

The compiler should not be that stupid -- it should see the type of the left hand side, and if the result fits into the left hand side it should not emit that stupid remark.

JenniferJ
Moderator
491 Views

This seems an old issue. But it is resolved in the 11.1 release.

In the 11.1 compiler a new option "/W5" is added for emitting all the remarks. So for this test case the 4 remarks will be emitted only under /W5. If you use default or /W4 or /Wall, you won't see those remarks.

Jennifer

Reply