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

Add -latomic flag for -DARM #571

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

umlaeute
Copy link
Contributor

@umlaeute umlaeute commented Mar 3, 2022

This is partA of a revamped version of #504, that only includes the "-latomic" fix for arm.

this PR has the "fix typo" commits squashed into a single commit.

also note, that there are more architectures that require linking with -latomic (though most of them being rather exotic).

this is the reason we us the following in the meta-buildsystem for the giada Debian package:

noatomicarch = $(shell dpkg-architecture -qDEB_HOST_ARCH | egrep -x "(armel|powerpc|powerpcspe|m68k|mips|mipsel|sh4|riscv64)")
# link with libatomic on architectures without built-in atomic
ifeq ($(if $(noatomicarch),atomic), atomic)
        LIBS += -latomic
endif

and then inject the LIBS into the CMake invocation.

so I personally don't have any special interest in this PR (as it seems to be somewhat over-specific for my needs), and you might just want to use it as a starting point for a more generic solution (e.g. rather than if(DEFINED ARM) use an option WITH_ATOMIC or somesuch)

@gvnnz
Copy link
Contributor

gvnnz commented Mar 4, 2022

Thanks for the revamp @umlaeute. I'm talking to the original author here - actually this might not be 100% correct, because the check is performed inside the if(WITH_VST2 OR WITH_VST3) block and we are using atomics regardless of the presence of VSTs. Anyway I'll keep this one open as a memo for the -latomic flag issue for ARM.

@umlaeute
Copy link
Contributor Author

umlaeute commented Mar 5, 2022

the original author is @DatanoiseTV (just mentioning them here, so they get notified)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants