[flac-dev] FLAC__SSE_OS change

lvqcl lvqcl.mail at gmail.com
Tue Jun 21 16:01:45 UTC 2016


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.


More information about the flac-dev mailing list