Intel compiler bug?

[mar]

I recently found something I believe is optimizer bug in icc.
I've reported the issue. The version I used is XE 14,
haven't tried XE 15 yet (maybe someone could try?).

Code that reproduces the bug follows:

Code: Select all

#include <vector>
#include <iostream>

using namespace std;

struct PackedColor
&#123;
	unsigned int packed;

	unsigned char GetR&#40;) const &#123;
		return &#40;packed >> 16&#41; & 255;
	&#125;
	unsigned char GetG&#40;) const &#123;
		return &#40;packed >> 8&#41; & 255;
	&#125;
	void SetR&#40; int nr ) &#123;
		unsigned int r = nr << 16;
		packed &= ~&#40;255u << 16&#41;;
		packed |= r;
	&#125;
	void SetG&#40; int ng ) &#123;
		unsigned int g = ng << 8;
		packed &= ~&#40;255u << 8&#41;;
		packed |= g;
	&#125;
&#125;;

int main&#40;)
&#123;
	vector< PackedColor > hmap&#40; 256 );
	for ( int i=0; i<256; i++ ) &#123;
		hmap&#91;i&#93;.packed = 0;
		hmap&#91;i&#93;.SetR&#40; 0 );
		hmap&#91;i&#93;.SetG&#40; 128 );
	&#125;
	int h = hmap&#91;0&#93;.GetR&#40;) + 256 * hmap&#91;0&#93;.GetG&#40;);
	// expect 32768 here but get 0
	cout << "h=" << h << endl;
	return 0;
&#125;

I only hope it doesn't exploit some hidden UB (making a fool of myself) problem but I doubt.
Prints 0 here (discarding GetG fetch). Scary, isn't it?
Considering I compiled the Windows version of my engine with icc...

[Rein Halbersma]

It looks legitimate code to me, but could you try and recompile this with all occurances of "signed int" and "unsigned char" replaced with "unsigned int"? The rules for integral promotions (char to int) and signed/unsigned conversions are rather tricky.

Code: Select all

#include <vector> 
#include <iostream> 

using namespace std; 

struct PackedColor 
&#123; 
   unsigned int packed; 

   unsigned int GetR&#40;) const &#123; 
      return &#40;packed >> 16&#41; & 255; 
   &#125; 
   unsigned int GetG&#40;) const &#123; 
      return &#40;packed >> 8&#41; & 255; 
   &#125; 
   void SetR&#40; unsigned int nr ) &#123; 
      unsigned int r = nr << 16; 
      packed &= ~&#40;255u << 16&#41;; 
      packed |= r; 
   &#125; 
   void SetG&#40; unsigned int ng ) &#123; 
      unsigned int g = ng << 8; 
      packed &= ~&#40;255u << 8&#41;; 
      packed |= g; 
   &#125; 
&#125;; 

int main&#40;) 
&#123; 
   vector< PackedColor > hmap&#40; 256 ); 
   for &#40;unsigned  int i=0; i<256; i++ ) &#123; 
      hmap&#91;i&#93;.packed = 0; 
      hmap&#91;i&#93;.SetR&#40; 0 ); 
      hmap&#91;i&#93;.SetG&#40; 128 ); 
   &#125; 
   unsigned int h = hmap&#91;0&#93;.GetR&#40;) + 256 * hmap&#91;0&#93;.GetG&#40;); 
   // expect 32768 here but get 0 
   cout << "h=" << h << endl; 
   return 0; 
&#125; 

[AlvaroBegue]

Is that program minimal? What happens if you don't use a vector, but have a single PackedColor object? What do you get from GetR() and GetG() independently?

[mar]

Yes this code works, but not because of signed/unsigned.
Replacing GetR/GetG to return unsigned char triggers the bug again hower.
Funny enough, multiplying with 257 "works", returning a number >32768.
Alvaro: not sure if it's minimal, but if I use a single PackedObject the bug disappears (has nothing to do with vector itself as I normally use my own containers).
Hmm, when I use this:

Code: Select all

unsigned int h = 256u * (unsigned int)hmap[0].GetG(); 

it still returns 0.

[mar]

This is probably as small as it could get:

Code: Select all

#include <vector>
#include <iostream>

using namespace std;

struct PackedColor
&#123;
	unsigned int packed;

	unsigned char GetG&#40;) const &#123;
		return &#40;packed >> 8&#41; & 255;
	&#125;
&#125;;

int main&#40;)
&#123;
	vector< PackedColor > hmap&#40; 256 );
	for ( int i=0; i<256; i++ ) &#123;
		hmap&#91;i&#93;.packed = 128*256;
	&#125;
	int h = 256 * hmap&#91;0&#93;.GetG&#40;);
	// expect 32768 here but get 0
	cout << "h=" << h << endl;
	return 0;
&#125;

[Rein Halbersma]

Yes this code works, but not because of signed/unsigned.
Replacing GetR/GetG to return unsigned char triggers the bug again hower.
Funny enough, multiplying with 257 "works", returning a number >32768.
Alvaro: not sure if it's minimal, but if I use a single PackedObject the bug disappears (has nothing to do with vector itself as I normally use my own containers).
Hmm, when I use this:

Code: Select all

unsigned int h = 256u * (unsigned int)hmap[0].GetG(); 

it still returns 0.

still sounds like a compiler bug (Clang/gcc give your expected answer). In any case, why exactly are you mixing these different integer types?

[xmas79]

Yes this code works, but not because of signed/unsigned.
Replacing GetR/GetG to return unsigned char triggers the bug again hower.
Funny enough, multiplying with 257 "works", returning a number >32768.
Alvaro: not sure if it's minimal, but if I use a single PackedObject the bug disappears (has nothing to do with vector itself as I normally use my own containers).
Hmm, when I use this:

Code: Select all

unsigned int h = 256u * (unsigned int)hmap[0].GetG(); 

it still returns 0.

Why don't you look at generated assembly? With 257 the compiler emits a multiply instruction. The 256 instead is probably optimized as left shift, and extra care could be needed (to prevent the compiler bug)?

Code: Select all

unsigned int h = 256u * ((unsigned int)hmap[0].GetG());

Maybe?

[mar]

In any case, why exactly are you mixing these different integer types?

I simplified the original code a lot, originally it grabs an int and clamps it to byte range, then packing it in:

Code: Select all

inline void SetG( Int ng ) {
	UInt g = &#40;UInt&#41;Core&#58;&#58;Clamp&#40; ng, 0, 255 ) << 8;
	packed &= ~&#40;255u << 8&#41;;
	packed |= g;
&#125;

[mar]

Why don't you look at generated assembly? With 257 the compiler emits a multiply instruction. The 256 instead is probably optimized as left shift, and extra care could be needed (to prevent the compiler bug)?

I don't want to prevent the bug, I want it gone!
Assembly:

Code: Select all

movzx esi,byte ptr [esi]
and esi,0FFFFFF00h

Anyway it fetches the first byte instead of second (x86 is little endian).

[xmas79]

Code: Select all

...
	vector< PackedColor > hmap&#40; 256 );
	for ( int i=0; i<256; i++ ) &#123;
		hmap&#91;i&#93;.packed = 128*256;
	&#125;
	int h = 256 * hmap&#91;0&#93;.GetG&#40;);
	// expect 32768 here but get 0
	cout << "h=" << h << endl;
	return 0;
&#125;

All the for loop stuff is usually folded away (in these simple cases) since you are accessing only the first element. The compiler then keeps only

Code: Select all

int h = 256 * hmap[0].GetG();

which produces 0, because GetG() returns an uchar that will be shifted left by 8 bits, leaving you with 0 as result... as expected!