We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4634 - SDL_SimdAlloc/Free etc breaks emscripten build.
Summary: SDL_SimdAlloc/Free etc breaks emscripten build.
Status: RESOLVED INVALID
Alias: None
Product: SDL
Classification: Unclassified
Component: *don't know* (show other bugs)
Version: don't know
Hardware: x86_64 Windows 10
: P2 major
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords: target-2.0.12
Depends on:
Blocks:
 
Reported: 2019-05-19 10:09 UTC by UNDEFINED-BEHAVIOR
Modified: 2019-11-04 03:50 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description UNDEFINED-BEHAVIOR 2019-05-19 10:09:15 UTC
Changes introduced in

https://hg.libsdl.org/SDL/rev/66cd8731c3b1

Breaks emscripten build because cpuinfo folder aren't included on that platform. The source/object file cannot see cpuinfo/SDL_simd.h
Comment 1 Ryan C. Gordon 2019-05-19 17:19:38 UTC
Is this built with CMake or configure or something else? Our buildbot is building it okay...
Comment 2 UNDEFINED-BEHAVIOR 2019-05-19 18:13:36 UTC
Its build with cmake with modified cmakelist.

This is from latest master branch btw.

I was under the impression that SIMD call is hardcoded right now.
ex: SDL_surface.c
Comment 3 Ryan C. Gordon 2019-05-20 20:07:50 UTC
This works here.

I did a fresh install of Emscripten like this (on a Mac, instructions are similar on other platforms)...

rm -rf ~/.emscripten*
rm -rf emsdk
git clone https://github.com/emscripten-core/emsdk.git
cd emsdk
./emsdk install latest
./emsdk activate latest
source ./emsdk_env.sh

(go to SDL clone)
mkdir cmake-build
cd cmake-build
emcmake cmake -DCMAKE_BUILD_TYPE=Debug ..
emmake make

--ryan.
Comment 4 UNDEFINED-BEHAVIOR 2019-05-28 14:29:25 UTC
The library does compile with the method above. 
Even more surprising, function that seems to directly uses SIMD_alloc/free like SDL_CreateRGBSurfaceWithFormat in simple test program also apparently works in the browser..
(I'm using emscripten master)

I'm not sure about how to properly modify cmake for SDL now.
Our goal was to stripped out some seemingly unnecessary SDL codes on emscripten platform, cpuinfo related stuff included. So it might have been a fault with our setup.

There is one think I still didn't understand.

https://hg.libsdl.org/SDL/file/96bca5da4965/CMakeLists.txt#l254
# Emscripten/Javascript does not have assembly support, a dynamic library
# loading architecture, low-level CPU inspection or multithreading.
set(SDL_CPUINFO_ENABLED_BY_DEFAULT OFF)
and --disable-cpuinfo options passed to emconfigure

What does this option really means in SDL term?
Totally exclude the source/ header from compilation? Or actually included in the compilation but only as a stub?
Because even when it's certainly set to OFF, SDL_cpuinfo.h/.c and cpuinfo/SDL_simd.h seems to be included and compiled anyways.
Even if some subsystem is marked as disabled should all the header/sources be visible to the program?

Also, commenting this out doesn't seems to cause any unresolved symbol errors in the library build. (using the procedure above)
https://hg.libsdl.org/SDL/file/96bca5da4965/CMakeLists.txt#l360
Comment 5 Ryan C. Gordon 2019-07-30 17:49:34 UTC
(Sorry if you get several emails like this, we're marking a bunch of bugs.)

We're hoping to ship SDL 2.0.11 on a much shorter timeframe than we have historically done releases, so I'm starting to tag bugs we hope to have closed in this release cycle.

Note that this tag means we just intend to scrutinize this bug for the 2.0.11 release: we may fix it, reject it, or even push it back to a later release for now, but this helps give us both a goal and a wishlist for the next release.

If this bug has been quiet for a few months and you have new information (such as, "this is definitely still broken" or "this got fixed at some point"), please feel free to retest and/or add more notes to the bug.

--ryan.
Comment 6 Ryan C. Gordon 2019-09-20 20:47:39 UTC
We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.
Comment 7 Ryan C. Gordon 2019-09-20 20:48:38 UTC
We're changing how we do SDL release versions; now releases will be even numbers (2.0.10, 2.0.12, etc), and as soon as we tag a release, we'll move the internal version number to an odd number (2.0.12 ships, we tag the latest in revision control as 2.0.13 immediately, which will become 2.0.14 on release, etc).

As such, I'm moving the bugs tagged with target-2.0.11 to target 2.0.12. Sorry if you get a lot of email from this change!

Thanks,
--ryan.
Comment 8 Ryan C. Gordon 2019-11-04 03:50:28 UTC
(In reply to UNDEFINED-BEHAVIOR from comment #4)

> There is one think I still didn't understand.
> 
> https://hg.libsdl.org/SDL/file/96bca5da4965/CMakeLists.txt#l254
> # Emscripten/Javascript does not have assembly support, a dynamic library
> # loading architecture, low-level CPU inspection or multithreading.
> set(SDL_CPUINFO_ENABLED_BY_DEFAULT OFF)
> and --disable-cpuinfo options passed to emconfigure
> 
> What does this option really means in SDL term?

Through a convoluted series of macros, this eventually makes sure the C code compiles with SDL_CPUINFO_DISABLED defined. This doesn't remove the functions from SDL, but removes the bulk of the implementation, so things like SDL_HasSSE() will return SDL_FALSE without actually doing anything to check if the CPU actually has SSE at all (etc).

But the source file (and the entry points) are still compiled, with a lot of the actual code eliminated by the C preprocessor before the compiler can see it.

Anyhow, I'm going to resolve this bug, since the initially reported issue appears to be settled. If you have other issues, please feel free to reopen this bug or start a new one, though!

--ryan.