Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Please allow use of custom simde location #6846

Closed
falkTX opened this issue Feb 16, 2023 · 1 comment · Fixed by #6853
Closed

Please allow use of custom simde location #6846

falkTX opened this issue Feb 16, 2023 · 1 comment · Fixed by #6853
Labels
Feature Request New feature request Infrastructure Issues related to repository, CI/CD, installers, etc.

Comments

@falkTX
Copy link
Contributor

falkTX commented Feb 16, 2023

VCV Rack decided to go with simde for SSE compat under ARM, and Cardinal is following that (while initially using sse2neon instead).
Due to differences on the version shipped with Rack vs the one shipped with surge, there can be symbol conflicts.
Doing an LTO shows this message:

../include/simd-compat/../simde/simde/x86/sse2.h:136:3: error: type 'union simde__m128i_private' violates the C++ One Definition Rule [-Werror=odr]
plugins/surgext/surge/src/../libs/simde/simde/x86/sse2.h:124:3: note: a different type is defined in another translation unit
  124 | } simde__m128i_private;
      |   ^
../include/simd-compat/../simde/simde/x86/sse2.h:95:38: note: the first difference of corresponding definitions is field 'neon_f16'
plugins/surgext/surge/src/../libs/simde/simde/x86/sse2.h:94:38: note: a field with different name is defined in another translation unit
   94 |     SIMDE_ALIGN_TO_16 float32x4_t    neon_f32;

A possible solution is to be able to use a custom simde version, alike done with juce.
That should be the approach for surge-rack too IMO, since the library is already provided by the Rack SDK modules built against VCV Rack should use it instead of their own custom version.

Alternatively the built-in simde could be contained in a custom namespace, but not sure how much work that is..

@falkTX falkTX added the Feature Request New feature request label Feb 16, 2023
@baconpaul
Copy link
Collaborator

This is pretty straight forward

add_library(simde INTERFACE)

probably becomes

set(SURGE_SIMDE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/../libs/simde" CACHE STRING "Path to simde library source tree")
add_library(simde INTERFACE)
target_include_directories(simde INTERFACE ${SURGE_SIMDE_PATH})
add_library(surge::simde ALIAS simde)

then test you can sub then run through CI and test that it still works

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request New feature request Infrastructure Issues related to repository, CI/CD, installers, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants