It looks like the intrinsics path on internal BitOps.TrailingZeroCount returns a different value to the software fallback, for input value 0.
For an input of 0, Bmi1.TrailingZeroCount returns the operand size, in this case 32.
But the software fallback returns 0 instead.
It's easy to eyeball the problem; see comments in the code below, assuming 0 is passed as the matches parameter.
publicstaticintTrailingZeroCount(intmatches)// assume matches==0{if(Bmi1.IsSupported){// TZCNT contract specifies 0->32return(int)Bmi1.TrailingZeroCount((uint)matches);// returns 32 <----- NOTE}else{// Ostensible fix, though we'd prefer it to be branchless// if (matches == 0)// return 32;returnUnsafe.AddByteOffset(refMemoryMarshal.GetReference(TrailingCountMultiplyDeBruijn),// 0 & -0 * c >> 27 == 0// TrailingCountMultiplyDeBruijn[0] == 0((uint)((matches&-matches)*0x077CB531U))>>27);// returns 0 <----- NOTE}}privatestaticReadOnlySpan<byte>TrailingCountMultiplyDeBruijn=>newbyte[32]{0,1,28,// .. elided};Though looking at just one code path in the original PR, this code path (value == 0) is not hit due to a guard clause.
We'd have to check the rest of the callsites, but it looks like it may be a non-intrusive fix.
However any new code calling this function may hit the bug.
cc @benaadams
It looks like the intrinsics path on
internal BitOps.TrailingZeroCountreturns a different value to the software fallback, for input value0.For an input of
0,Bmi1.TrailingZeroCountreturns the operand size, in this case32.But the software fallback returns
0instead.It's easy to eyeball the problem; see comments in the code below, assuming
0is passed as thematchesparameter.Though looking at just one code path in the original PR, this code path (
value == 0) is not hit due to a guard clause.We'd have to check the rest of the callsites, but it looks like it may be a non-intrusive fix.
However any new code calling this function may hit the bug.
cc @benaadams