flac dev - Jun 2016 - FLAC__SSE_OS change

About the commit
<http://git.xiph.org/?p=flac.git;a=commitdiff;h=e120037f3c67b23fd9eef7ccd04d2df57fa1a9a6>

I admit I don't understand the following lines:

  #if !FLAC__SSE_OS
         /* assume user knows better than us; turn it off */
         disable_sse(info);
  #elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) ......

Probably it should be

  #if FLAC__SSE_OS
         /* assume user knows better than us; leave as detected above */
  #elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) ......

?

lvqcl wrote:
> About the commit
<http://git.xiph.org/?p=flac.git;a=commitdiff;h=e120037f3c67b23fd9eef7ccd04d2df57fa1a9a6>
> 
> I admit I don't understand the following lines:
> 
>   #if !FLAC__SSE_OS
>          /* assume user knows better than us; turn it off */
>          disable_sse(info);
>   #elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) ......
> 
> Probably it should be
> 
>   #if FLAC__SSE_OS
>          /* assume user knows better than us; leave as detected above */
>   #elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) ......
I actually think my change is correct. It should be easier to see if you
look at the github version of the change:

   
https://github.com/xiph/flac/commit/e120037f3c67b23fd9eef7ccd04d2df57fa1a9a6#diff-9f048b83ff55071de36263cf0f403b2eL209

But yes, all those #ifs and #elses are horrible for readability and
maintainability.

CHeers,
Erik
-- 
----------------------------------------------------------------------
Erik de Castro Lopo
http://www.mega-nerd.com/

Erik de Castro Lopo wrote:
> I actually think my change is correct. It should be easier to see if you
> look at the github version of the change:
>
>    
https://github.com/xiph/flac/commit/e120037f3c67b23fd9eef7ccd04d2df57fa1a9a6#diff-9f048b83ff55071de36263cf0f403b2eL209
>
FLAC__NO_SSE_OS was never defined anywhere, so I think that
the following changes should be made:

   defined FLAC__NO_SSE_OS ->  0
  !defined FLAC__NO_SSE_OS ->  1
   defined FLAC__SSE_OS    ->  FLAC__SSE_OS
  !defined FLAC__SSE_OS    ->  !FLAC__SSE_OS

So the code

  #if defined FLAC__NO_SSE_OS
     /* assume user knows better than us; turn it off */
     disable_sse(info);
  #elif defined FLAC__SSE_OS
     /* assume user knows better than us; leave as detected above */
  #elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) ...

becomes

  #if 0
     /* assume user knows better than us; turn it off */
     disable_sse(info);
  #elif FLAC__SSE_OS
     /* assume user knows better than us; leave as detected above */
  #elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) ...

which is equivalent to

  #if FLAC__SSE_OS
     /* assume user knows better than us; leave as detected above */
  #elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) ...

(and there should be no "&& !FLAC__SSE_OS" after "#elif
defined(__linux__)")
> But yes, all those #ifs and #elses are horrible for readability and
> maintainability.
OTOH dead code elimination (as in ffmpeg) makes it impossible to build
debug version...
Probably it's better to have one main simple cpu.h and several additional
files (one per architecture) but I'm not going to volunteer for the
refactoring.
> CHeers,
> Erik
P.S. I wonder what's the point to test OS SSE support in 2016? I believe
that
users of Windows 95 and Windows NT4 don't expect new software to work on
their
OSes. Don't know about users of ancient versions of Linux/BSD/etc though.