lvqcl wrote: > More or less detailed explanation of this patch.
I've applied part of that patch. > 1. The first parameter of _BitScanReverse() and _BitScanReverse64() is > a pointer to unsigned long (4-byte int). On windows, yes, unsigned long is 4 bytes for both 32 and 64 bit versions. This is not true for most Unices. > However _BitScanReverse64() is called with a pointer to FLAC__uint64 > (8-byte int). IMHO it's a bug I would not call that a bug, just a result of trying to write cross platform code and relying on functions that are not specified by any standard. > 2. FLAC__clz_uint32() returns the result of BitScanReverse() XOR 31. > FLAC__bitmath_ilog2() returns 31-FLAC__clz_uint32() == 31-(BSR^31). > > As 0<=BSR<=31 so 31-(BSR^31) == BSR. But I don't think that compilers > are so smart that can optimize this expression. So it is better to change > FLAC__bitmath_ilog2() so it simply calls BitScanReverse() when possible. I want to look at this more carefully. Its probably worth writing a unit test for it. > 3. FLAC__bitmath_ilog2_wide() cannot be compiled with MSVC: it has > 'if (v == 0)...' statement followed by a variable (either idx or > DEBRUIJN_IDX64) definition. But all calls of this function have > FLAC__ASSERT statements that ensure that its argument is greater > than 0. So this 'if()' can be removed. I'd actuall prefer to remove those ASSERTS, because they get bypassed for the production compiles anyway. > 4. Am I right that FLAC__bitmath_ilog2_wide() and FLAC__bitmath_ilog2() > return the same value if their argument is less than 2^32? Then > FLAC__bitmath_ilog2_wide() works correctly only when compiled with GCC: > it should return idx, not idx^63U. Also the result of de Bruijn sequences > is off by 1. This also needs more investigation. Your patch changes the output of this function for any given input. Erik -- ---------------------------------------------------------------------- Erik de Castro Lopo http://www.mega-nerd.com/ _______________________________________________ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
