First off, this code is horrible to read and work on. The recent commits are the first of what I hope is a massive clean up of this code. lvqcl wrote:> So if I understand things correctly, the current meaning of --(en|dis)able-sse is: > > on Linux: > --enable-sse: > add -msse2 to the compiler switches > do not test SSE OS support (assume that SSE is supported) > --disable-sse: > do NOT add -msse2 > test SSE OS support > > on other OSes: > --enable-sse: > add -msse2 to the compiler switches > test SSE OS support (why?) > --disable-sse: > do NOT add -msse2 > test SSE OS support > > It's a bit contradictory: why test whether *BSD etc support SSE or not > but at the same time allow compiler to use SSE/SSE2 unconditionally?Yes, that needs to be fixed. I think the way it works on Linux makes the most sense.> Also: are there any compilers / target OSes such that libFLAC currently > has no way to test OS support of SSE?No idea. Thats for other people to test and bring to our attention. Adding support for this stuff will become easier after this code is cleaned up.> The current code will always > disable SSE for such builds: > --enable-sse: > add -msse2 to the compiler switches > disable the use of SSE code > --disable-sse: > do NOT add -msse2 > disable the use of SSE code > > previously --enable-sse made it possible to override this.Once this code is cleaned up further improvements are likely.> > Please test compiling on Windows and anything else you can get your > > hands on. > > Will do. > MSVC 2015: builds OK.Great. Thanks. Erik -- ---------------------------------------------------------------------- Erik de Castro Lopo http://www.mega-nerd.com/
On 06/25/16 10:43 PM, Erik de Castro Lopo wrote:> First off, this code is horrible to read and work on. The recent commits > are the first of what I hope is a massive clean up of this code. > > lvqcl wrote: > >> >So if I understand things correctly, the current meaning of --(en|dis)able-sse is: >> > >> >on Linux: >> > --enable-sse: >> > add -msse2 to the compiler switches >> > do not test SSE OS support (assume that SSE is supported) >> > --disable-sse: >> > do NOT add -msse2 >> > test SSE OS support >> > >> >on other OSes: >> > --enable-sse: >> > add -msse2 to the compiler switches >> > test SSE OS support (why?) >> > --disable-sse: >> > do NOT add -msse2 >> > test SSE OS support >> > >> >It's a bit contradictory: why test whether *BSD etc support SSE or not >> >but at the same time allow compiler to use SSE/SSE2 unconditionally? > Yes, that needs to be fixed. I think the way it works on Linux makes > the most sense. >Doesn't SSE support imply SSE2+ support? The problem is that the OS has to preserve the XMMS registers when doing a context switch. Once the kernel is preserving the XMMS registers, I'd assume that all SSEx instructions should work. I have a '96 install of an OS, it has been upgraded until end of life, and it handles SSE4+ instructions fine even though the instruction set was released more recently then the last kernel. Dave
Dave Yeo wrote:> Doesn't SSE support imply SSE2+ support?Not for the CPU. Just because a CPU supports SSE, does not mean it is guaranteed to support SSE2+. For OS support, I'm not sure. Didn't later version of SSE add new registers?> I have a '96 install of an OS, it has been upgraded until end of life, > and it handles SSE4+ instructions fine even though the instruction set > was released more recently then the last kernel.How many CPU cores to you have? Have you tried running more processes than you have cores at the same time? Say you have 4 cores, start up 8 terminals, each with their own clone of the FLAC sources and run 'make check' in all. If the OS is not save all SSE registers, these procesess will trample over each other's state and the FLAC tests should fail in non-reproducable ways. Erik -- ---------------------------------------------------------------------- Erik de Castro Lopo http://www.mega-nerd.com/
Erik de Castro Lopo wrote:>> MSVC 2015: builds OK. > > Great. Thanks.The current code builds OK on MSVC 2015 and 2010 but not on MSVC 2005: error C2059: syntax error : ';' cpu.c 153 error C2059: syntax error : 'type' cpu.c 153 error C2061: syntax error : identifier 'cpu_xgetbv_x86' cpu.c 153 It doesn't know about uint32_t type, so the definition of cpu_xgetbv_x86() fails. It can be fixed by adding "#include share/compat.h" to cpu.c (or by using FLAC__uint32 from FLAC/ordinals.h). When I fix this, the following problem occurs: error LNK2019: unresolved external symbol ___cpuidex referenced in function _FLAC__cpu_info_x86 libflac_static.lib fatal error LNK1120: 1 unresolved externals flac.exe The code if (FLAC__AVX_SUPPORTED) __cpuidex(cpuinfo, level, 0); /* for AVX2 detection */ else __cpuid(cpuinfo, level); /* some old compilers don't support __cpuidex */ adds reference to __cpuidex() even though MSVC2005 doesn't have it (according to MSDN, it was added to MSVC 2008 SP1).
lvqcl wrote:> It doesn't know about uint32_t type, so the definition of cpu_xgetbv_x86() fails. > It can be fixed by adding "#include share/compat.h" to cpu.c (or by using > FLAC__uint32 from FLAC/ordinals.h).Ok, added share/compat.h.> When I fix this, the following problem occurs: > > error LNK2019: unresolved external symbol ___cpuidex referenced in function _FLAC__cpu_info_x86 libflac_static.lib > fatal error LNK1120: 1 unresolved externals flac.exe > > The code > > if (FLAC__AVX_SUPPORTED) > __cpuidex(cpuinfo, level, 0); /* for AVX2 detection */ > else > __cpuid(cpuinfo, level); /* some old compilers don't support __cpuidex */ > > adds reference to __cpuidex() even though MSVC2005 doesn't have it > (according to MSDN, it was added to MSVC 2008 SP1).That suggests that FLAC__AVX_SUPPORTED is true. Can you set it to false for MSVC2005? Erik -- ---------------------------------------------------------------------- Erik de Castro Lopo http://www.mega-nerd.com/
Dave Yeo wrote:>>> >on other OSes: >>> > --enable-sse: >>> > add -msse2 to the compiler switches >>> > test SSE OS support (why?)>>> >It's a bit contradictory: why test whether *BSD etc support SSE or not >>> >but at the same time allow compiler to use SSE/SSE2 unconditionally? >> Yes, that needs to be fixed. I think the way it works on Linux makes >> the most sense. >> > > Doesn't SSE support imply SSE2+ support?There's no point to test OS SSE support if a compiler already inserted SSE/SSE2 instructions everywhere (because of -msse2 switch).> The problem is that the OS has > to preserve the XMMS registers when doing a context switch. Once the > kernel is preserving the XMMS registers, I'd assume that all SSEx > instructions should work.If the OS doesn't support SSE instructions but -msse2 option was used then libFLAC will crash: it cannot disable the use of SSE/SSE2 instructions that were generated by the compiler.