Some strange casting in generated cpp code

Hey,

In our code we are using MurmurHash2 as fast and simple hashing algorithm (you can check it's implementation at Landman Code). Actually there is a simple implementation that uses BitConverter.ToUInt32 (I will refer to this implementation as 'simple') and there is an implementation that inlines this method (I will refer to this implementation as 'inline').

Spoiler: Simple

Code (CSharp):

1.     public class MurmurHash2Simple : IHashAlgorithm
2.     {
3.         public UInt32 Hash(Byte[] data)
4.         {
5.             return Hash(data, 0xc58f1a7b);
6.         }
7.         const UInt32 m = 0x5bd1e995;
8.         const Int32 r = 24;
9.  
10.         public UInt32 Hash(Byte[] data, UInt32 seed)
11.         {
12.             Int32 length = data.Length;
13.             if (length == 0)
14.                 return 0;
15.             UInt32 h = seed ^ (UInt32)length;
16.             Int32 currentIndex = 0;
17.             while (length >= 4)
18.             {
19.                 UInt32 k = BitConverter.ToUInt32(data, currentIndex);
20.                 k *= m;
21.                 k ^= k >> r;
22.                 k *= m;
23.  
24.                 h *= m;
25.                 h ^= k;
26.                 currentIndex += 4;
27.                 length -= 4;
28.             }
29.             switch (length)
30.             {
31.                 case 3:
32.                     h ^= BitConverter.ToUInt16(data, currentIndex);
33.                     h ^= (UInt32)data[currentIndex + 2] << 16;
34.                     h *= m;
35.                     break;
36.                 case 2:
37.                     h ^= BitConverter.ToUInt16(data, currentIndex);
38.                     h *= m;
39.                     break;
40.                 case 1:
41.                     h ^= data[currentIndex];
42.                     h *= m;
43.                     break;
44.                 default:
45.                     break;
46.             }
47.            
48.             // Do a few final mixes of the hash to ensure the last few
49.             // bytes are well-incorporated.
50.  
51.             h ^= h >> 13;
52.             h *= m;
53.             h ^= h >> 15;
54.  
55.             return h;
56.         }
57.     }

Spoiler: Inline

Code (CSharp):

1.     public class MurmurHash2InlineBitConverter : IHashAlgorithm
2.     {
3.  
4.         public UInt32 Hash(Byte[] data)
5.         {
6.             return Hash(data, 0xc58f1a7b);
7.         }
8.         const UInt32 m = 0x5bd1e995;
9.         const Int32 r = 24;
10.  
11.         public UInt32 Hash(Byte[] data, UInt32 seed)
12.         {
13.             Int32 length = data.Length;
14.             if (length == 0)
15.                 return 0;
16.             UInt32 h = seed ^ (UInt32)length;
17.             Int32 currentIndex = 0;
18.             while (length >= 4)
19.             {
20.                 UInt32 k = (UInt32)(data[currentIndex++] | data[currentIndex++] << 8 | data[currentIndex++] << 16 | data[currentIndex++] << 24);
21.                 k *= m;
22.                 k ^= k >> r;
23.                 k *= m;
24.  
25.                 h *= m;
26.                 h ^= k;
27.                 length -= 4;
28.             }
29.             switch (length)
30.             {
31.                 case 3:
32.                     h ^= (UInt16)(data[currentIndex++] | data[currentIndex++] << 8);
33.                     h ^= (UInt32)(data[currentIndex] << 16);
34.                     h *= m;
35.                     break;
36.                 case 2:
37.                     h ^= (UInt16)(data[currentIndex++] | data[currentIndex] << 8);
38.                     h *= m;
39.                     break;
40.                 case 1:
41.                     h ^= data[currentIndex];
42.                     h *= m;
43.                     break;
44.                 default:
45.                     break;
46.             }
47.  
48.             // Do a few final mixes of the hash to ensure the last few
49.             // bytes are well-incorporated.
50.  
51.             h ^= h >> 13;
52.             h *= m;
53.             h ^= h >> 15;
54.  
55.             return h;
56.         }
57.     }

And what I found is that the inline implementation doesn't work as expected when you build and run with target set to WebGL. What I saw - the hashes generated in editor / iOS / WebPlayer / Android / mono OS X generated on WebGL.

So I started to investigate the problem deeper. After some debugging (I mean, log-debugging) I found that the problem was with shifting. For example,

Code (CSharp):

1. public class Test {
2.     public static Test() {
3.         Byte b = 0x65;
4.         Debug.Log("b         << 16 = " + (b            << 16));
5.         Debug.Log("(UInt32)b << 16 = " + ((UInt32)b    << 16));
6.         Debug.Log("0x65      << 16 = " + (0x65         << 16));
7.     }
8. }

Was generating following output

Code (CSharp):

1. Editor
2.     b            << 16 = 6619136
3.     (UInt32)b    << 16 = 6619136
4.     0x65         << 16 = 6619136
5.  
6. WebGL
7.     b            << 16 = 0
8.     (UInt32)b    << 16 = 0
9.     0x65         << 16 = 6619136

Well, this example is note the best one, because c# compiler will (I believe) compute 0x65 << 16 at compile time and replace whole expression by 0x650000. But anyway, as you can see, b << 16 == 0 on WebGL. And this actually ruins inline implementation of MurmurHash2.

Right. So let’s improve example.

Code (CSharp):

1. public class HashTest : MonoBehaviour {
2.  
3.     void Start() {
4.         Byte b = 0x65;
5.         UInt32 i = 0x65;
6.         UInt32 v1 = (UInt32)b << 16;
7.         UInt32 v2 = (UInt32)(b << 16);
8.         UInt32 v3 = i << 16;
9.         Log.Warn(v1.ToString());
10.         Log.Warn(v2.ToString());
11.         Log.Warn(v3.ToString());
12.         b = 0x65;
13.         i = 0x65;
14.     }
15.  
16. }

Now everything will be compiled without any inlining. The output of this code is following:

Code (CSharp):

1. Editor:
2.     6619136
3.     6619136
4.     6619136
5.  
6. WebGL:
7.     0
8.     0
9.     6619136

So let’s check how il2cpp converts this code (I mean, il code generated from this code):

Code (CSharp):

1. // System.Void HashTest::Start()
2. extern MethodInfo HashTest_Start_m4_MethodInfo;
3. void HashTest_Start_m4 (HashTest_t2 * __this, MethodInfo* method){
4.     uint8_t V_0 = 0x0;
5.     uint32_t V_1 = 0;
6.     uint32_t V_2 = 0;
7.     uint32_t V_3 = 0;
8.     uint32_t V_4 = 0;
9.     {
10.         V_0 = ((int32_t)101);
11.         V_1 = ((int32_t)101);
12.         V_2 = ((uint8_t)(V_0<<((int32_t)16)));
13.         V_3 = ((uint8_t)(V_0<<((int32_t)16)));
14.         V_4 = ((uint32_t)(V_1<<((int32_t)16)));
15.         String_t* L_0 = UInt32_ToString_m13((&V_2),
16.           /*hidden argument*/&UInt32_ToString_m13_MethodInfo);
17.         Log_Warn_m1(NULL /*static, unused*/, L_0,
18.           /*hidden argument*/&Log_Warn_m1_MethodInfo);
19.         String_t* L_1 = UInt32_ToString_m13((&V_3),
20.           /*hidden argument*/&UInt32_ToString_m13_MethodInfo);
21.         Log_Warn_m1(NULL /*static, unused*/, L_1,
22.           /*hidden argument*/&Log_Warn_m1_MethodInfo);
23.         String_t* L_2 = UInt32_ToString_m13((&V_4),
24.           /*hidden argument*/&UInt32_ToString_m13_MethodInfo);
25.         Log_Warn_m1(NULL /*static, unused*/, L_2,
26.           /*hidden argument*/&Log_Warn_m1_MethodInfo);
27.         V_0 = ((int32_t)101);
28.         V_1 = ((int32_t)101);
29.         return;
30.     }
31. }

As you can see, there are some casting problems. I am not very familiar with c++, but AFAIK,

Code (CSharp):

1. uint8_t  v1 = 0x65;
2. uint32_t r1 = v1 << 16;
3. uint32_t r2 = (uint8_t)r1;
4. // r2 == 0

So I really don’t understand, why there is a cast to uint8_t in V_2 = ((uint8_t)(V_0<<((int32_t)16)));. V_0 is uint8_t and V_2 is uint32_t. So I think that it should cast the result to type of left expression instead of type of V_0. Isn't it a bug?

Do you need sample project for this?

Cheers,
d12frosted

 

This was fixed in patch build 5.0.0p1, so you can upgrade if you need to.

Otherwise you can work around the bug - in the short example at the end of your post, just declave v1 as uint32_t even though it is not a large number. Or if you're stuck with a narrow input, first store it in a wider variable, then shift the wider type left. I think casting inline is not enough to fool the optimizer.

Best is just to upgrade though if you can.

 

@gfoot awesome, I'll test this on 5.0.0p1.

Right, there are few possible ways to 'avoid' this bug. I created this post just to make sure that unity knows about this bug, because I already did some workarounds in our code.

Yes, it's not enough. Because from mono's point of view explicit casting is not necessary, so it removes it (right?).

So thanks for advice, I'll try again with new version of unity.

 

Just tested with Unity 5.0.0p2 - it works. Thank you for this fix, Unity!