Unity-Technologies / Unity.Mathematics

The C# math library used in Unity providing vector types and math functions with a shader like syntax
Other
1.38k stars 156 forks source link

math.tzcnt gives incorrect results for long and ulong #182

Closed weichx closed 3 years ago

weichx commented 3 years ago

Unless I totally don't understand this instruction, it seems like tzcnt isn't handling longs correctly when they have more than 32 bits set. Here are some sample inputs I ran

// correct if value is in integer range
math.tzcnt((ulong)(1 << 4)) == 4 
math.tzcnt((ulong)(1 << 30)) == 30

// incorrect if value is greater than integer range
math.tzcnt((ulong)(1 << 33))  == 1
math.tzcnt((ulong)(1 << 40)) == 8

I would expect the second two examples to return 33 and 40 respectively. I should add that this running without burst, just managed c#. I have no idea if burst does the correct thing or not (Edit burst also returns the same values as managed after all)

unpacklo commented 3 years ago

To clarify, I think the issue you're having here is the (1 << 33) is being treated as a signed 32 bit integer shift and the shift count of 33 will be truncated. Specifically, on page 180 of this doc https://www.ecma-international.org/publications/files/ECMA-ST/ECMA-334.pdf

image

Try using 1ul instead for the shift!

weichx commented 3 years ago

Thanks for following up, I closed this for exactly that reason, I think everything is fine now :)