1) in config.h FLAC__HAS_X86INTRIN macro is always defined and empty, even if x86intrin.h is not available. 2) sse_os is defined as 'yes' or 'no', but AM_CONDITIONAL tests it for 'true': AM_CONDITIONAL(FLaC__SSE_OS, test "x$sse_os" = xtrue) It seems that it should be changed to AM_CONDITIONAL(FLaC__SSE_OS, test "x$sse_os" = xyes) 3) configure --disable-sse prints: " SSE optimizations : ................... no" but this option actually doesn't disable SSE, so this message is a bit misleading. (Also, maybe it's better to rename --(dis|en)able-sse ?)
lvqcl wrote:> 1) in config.h FLAC__HAS_X86INTRIN macro is always defined and empty, > even if x86intrin.h is not available. > > 2) sse_os is defined as 'yes' or 'no', but AM_CONDITIONAL tests it for 'true':The patch is attached. Please check it.> 3) configure --disable-sse prints: > " SSE optimizations : ................... no" > but this option actually doesn't disable SSE, so this message > is a bit misleading.Instead of the current message echo " SSE optimizations : ................... ${sse_os}" it's technically more correct to write: echo " OS is guaranteed to support SSE : ..... ${sse_os}" but this message is much less comprehensible. -------------- next part -------------- A non-text attachment was scrubbed... Name: conf_ac.patch Type: application/octet-stream Size: 1074 bytes Desc: not available Url : http://lists.xiph.org/pipermail/flac-dev/attachments/20141013/8e7d3f18/attachment.obj
lvqcl wrote:> lvqcl wrote: > > > 1) in config.h FLAC__HAS_X86INTRIN macro is always defined and empty, > > even if x86intrin.h is not available. > > > > 2) sse_os is defined as 'yes' or 'no', but AM_CONDITIONAL tests it for 'true': > > The patch is attached. Please check it.Looks good. I need to do a little testing.> > 3) configure --disable-sse prints: > > " SSE optimizations : ................... no" > > but this option actually doesn't disable SSE, so this message > > is a bit misleading. > > Instead of the current message > > echo " SSE optimizations : ................... ${sse_os}" > > it's technically more correct to write: > > echo " OS is guaranteed to support SSE : ..... ${sse_os}" > > but this message is much less comprehensible.This is also a little misleading as its not the OS we are checking for SSE support but the compiler. I'll fix this. Cheers, Erik -- ---------------------------------------------------------------------- Erik de Castro Lopo http://www.mega-nerd.com/
> lvqcl wrote: > >> 1) in config.h FLAC__HAS_X86INTRIN macro is always defined and empty, >> even if x86intrin.h is not available. >> >> 2) sse_os is defined as 'yes' or 'no', but AM_CONDITIONAL tests it for 'true': > > The patch is attached. Please check it.Ping. Is the fix of FLAC__HAS_X86INTRIN definition necessary? Anyway I think that it should not be _always_ defined (as it does now).