lvqcl
2013-Aug-31 19:38 UTC
[flac-dev] New routine: FLAC__lpc_compute_autocorrelation_asm_ia32_sse_lag_16
Erik de Castro Lopo <mle+la at mega-nerd.com> wrote:> Patch applied, tested, commited and pushed.Great. But, what about my other patches? I admit that many of them aren't necessary, but (imho) a couple of them are important. I can explain in detail why I think this.
Erik de Castro Lopo
2013-Sep-01 00:41 UTC
[flac-dev] New routine: FLAC__lpc_compute_autocorrelation_asm_ia32_sse_lag_16
lvqcl wrote:> But, what about my other patches?Well first of all, none of them apply :-). Reading through them again I have the following: * Remove restrict definition from include/share/compat.h. Applied. * libFLAC and FLAC__ALIGN_MALLOC_DATA : Looks sane but doesn't apply. * MSVC and M_LN2 : Looks sane but doesn't apply. * bitmath.h: 1 typo, 1 warning : I don't see why the type has to change. * usefulness of fast_float_math_hack.h : Yes, probably worth removing it. Cheers, Erik -- ---------------------------------------------------------------------- Erik de Castro Lopo http://www.mega-nerd.com/
lvqcl
2013-Sep-01 15:25 UTC
[flac-dev] New routine: FLAC__lpc_compute_autocorrelation_asm_ia32_sse_lag_16
Erik de Castro Lopo <mle+la at mega-nerd.com> wrote:> Well first of all, none of them apply :-).I'll try to redo the necessary patches with git.> * Remove restrict definition from include/share/compat.h. Applied.BTW, I tried to use 'restrict' keyword with MSVS 2010 and 2012 and in fact they don't support it. Only --restrict is supported.> * libFLAC and FLAC__ALIGN_MALLOC_DATA : Looks sane but doesn't apply. > * MSVC and M_LN2 : Looks sane but doesn't apply.It is also possible to add /D "_USE_MATH_DEFINES" as an additional option to libFLAC_static.vcproj and libFLAC_dynamic.vcproj. Don't know what is better. What do you think?> * bitmath.h: 1 typo, 1 warning : I don't see why the type has to change.Declarations from MSDN: unsigned char _BitScanReverse(unsigned long * Index, unsigned long Mask); unsigned char _BitScanReverse64(unsigned long * Index, unsigned __int64 Mask); FLAC uses them as: _BitScanReverse(FLAC__uint32* idx, FLAC__uint32 v); and _BitScanReverse64(FLAC__uint64* idx, FLAC__uint64 v); AFAIK unsigned long is not the same type as FLAC__uint32 (= unsigned int) but it doesn't really matter since both of them are 4-byte unsigned integers. However, _BitScanReverse64 expects a pointer to a 4-byte uint, but it is called with a pointer to a 8-byte uint. The garbage in the most significant 4 bytes of idx is discarded then and the result of FLAC__bitmath_ilog2_wide() is correct. Still I think it's a bug. Also I noticed that all projects with '_static' suffix produce static library files (*.lib), projects with '_dynamic' suffix produce dynamic library files (*.dll), and projects without suffixes produce executables (*.exe). The only exception is win_utf8_io. Should it be renamed to win_utf8_io_static?