dr_mp3 should not assume SSE2 support for WIN32 i386 about dr_libs HOT 11 CLOSED

You're absolutely right, I missed that. That _M_IX86 check is in the wrong part of that line. It's currently in the x64 check, but needs to be moved to the x86+SSE2 check.

This line is actually from the minimp3 part of dr_mp3 (dr_mp3 is a wrapper around minimp3). I'll fix it in dr_mp3 in a bit. Thanks for the report!

@lieff - I think this change needs to be applied to minimp3?

from dr_libs.

In that line, the __SSE2__ is intended to be used for checking support for non-MSVC compilers. But you're right, it's not defined by MSVC. It's from the minimp3 project which I didn't write and it's a different style to the way I do it. I usually just check the version of MSVC to determine if the intrinsics are available at compile time, and then I do a runtime check to determine whether or not it's safe to execute those instructions.

I've added a check for _M_IX86_FP == 2 to the dev branch. Thanks for the heads up on that one. Hopefully that hasn't broken the fix I did in the previous commit 🤞.

from dr_libs.

The defined(__SSE2__) part of that line is supposed to guard this. It sounds to me that the __SSE2__ option is incorrectly being defined at compile time. What's the compiler? On the surface this feels like a compiler-specific issue. I just don't see why the compiler will be throwing that error, but still defining __SSE2__.

from dr_libs.

The defined(SSE2) part of that line is supposed to guard this.

It doesn't, here is the formatted conditional:

(
   (defined(_MSC_VER) && _MSC_VER >= 1400) && 
   (defined(_M_IX86) || defined(_M_X64))
) || (
   (defined(__i386__) || defined(__x86_64__)) && defined(__SSE2__)
)

So if _M_IX86 is defined, it does not check __SSE2__

from dr_libs.

Perhaps this should just be:

- #if ((defined(_MSC_VER) && _MSC_VER >= 1400) && (defined(_M_IX86) || defined(_M_X64))) || ((defined(__i386__) || defined(__x86_64__)) && defined(__SSE2__)) 
+ #if defined(_MSC_VER) && _MSC_VER >= 1400 && defined(__SSE2__)

from dr_libs.

I've pushed a fix to the dev branch. Are you able to give that a try?

Just reviewing that line it seems a quite confusing. I might refactor that later.

from dr_libs.

Verified that it now compiles!

from dr_libs.

Are you guys sure that MSVC really defines __SSE2__? As far as I
remember, it does not (but I'd like to be corrected), therefore the
_M_IX86 + __SSE2__ combination is useless there.

from dr_libs.

Should be _M_IX86_FP == 2, see https://docs.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170

from dr_libs.

Can we close this now? @glebm Did you update after the latest change?

from dr_libs.

The issue on the xbox is fixed, I haven't updated SDL_audiolib since the latest change but the change looks OK

from dr_libs.