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

android build issues #475

Closed
sezero opened this issue Oct 8, 2022 · 6 comments · Fixed by #452
Closed

android build issues #475

sezero opened this issue Oct 8, 2022 · 6 comments · Fixed by #452

Comments

@sezero
Copy link
Contributor

sezero commented Oct 8, 2022

We hit an issue building for android using cmake with x86 asm enabled
in SDL_mixer. Apparently, android toolchain advertises NASM as yasm,
and apparently yasm doesn't define __NASM_MAJOR__ and errors out.
The original discussion is at libsdl-org/SDL_mixer#457 (comment)
Here is the error -- after removing -mstackrealign which was another
source of error:

[1/153] Building ASM_NASM object external/flac/src/libFLAC/ia32/CMakeFiles/FLAC-asm.dir/cpu_asm.nasm.o
FAILED: external/flac/src/libFLAC/ia32/CMakeFiles/FLAC-asm.dir/cpu_asm.nasm.o 
/opt/android/ndk/21.4.7075529/toolchains/llvm/prebuilt/linux-x86_64/bin/yasm -DFLAC__HAS_NASM -DHAVE_CONFIG_H -D_DARWIN_C_SOURCE -D_POSIX_PTHREAD_SEMANTICS -D_TANDEM_SOURCE -D__STDC_WANT_IEC_60559_BFP_EXT__ -D__STDC_WANT_IEC_60559_DFP_EXT__ -D__STDC_WANT_IEC_60559_FUNCS_EXT__ -D__STDC_WANT_IEC_60559_TYPES_EXT__ -D__STDC_WANT_LIB_EXT2__ -D__STDC_WANT_MATH_SPEC_FUNCS__ -I/sdl_mixer/external/flac/include -I/sdl_mixer/build-android-prefab/build_x86/shared_ON/external/flac -I/sdl_mixer/external/flac/src/libFLAC/ia32 -I/sdl_mixer/external/flac/src/libFLAC/ia32/ -dOBJ_FORMAT_elf -f elf -o external/flac/src/libFLAC/ia32/CMakeFiles/FLAC-asm.dir/cpu_asm.nasm.o /sdl_mixer/external/flac/src/libFLAC/ia32/cpu_asm.nasm
/sdl_mixer/external/flac/src/libFLAC/ia32/cpu_asm.nasm:38: error: (cglobal:4) undefined symbol `__NASM_MAJOR__' in preprocessor
/sdl_mixer/external/flac/src/libFLAC/ia32/cpu_asm.nasm:39: error: (cglobal:4) undefined symbol `__NASM_MAJOR__' in preprocessor

I know there is the intention of asm removal (#452), but it would be nice if
this is resolved until that time.

CC: @madebr

@ktmf01
Copy link
Collaborator

ktmf01 commented Oct 9, 2022

How would you like to see this fixed? Do you think FLAC should make certain nasm is really nasm?

@sezero
Copy link
Contributor Author

sezero commented Oct 9, 2022

Either that, or support yasm properly too: As far as I can see, yasm supports the hidden attribute (the oldest I looked was .0.5.0 from 2006), and they define __YASM_MAJOR__.

@sezero
Copy link
Contributor Author

sezero commented Oct 9, 2022

The following works for me on linux, using yasm-1.3.0 and nasm-2.12.02

diff --git a/src/libFLAC/ia32/nasm.h b/src/libFLAC/ia32/nasm.h
index cdb8bf5..b98f4b5 100644
--- a/src/libFLAC/ia32/nasm.h
+++ b/src/libFLAC/ia32/nasm.h
@@ -68,7 +68,9 @@
 	%ifdef FLAC__PUBLIC_NEEDS_UNDERSCORE
 		global _%1
 	%else
-		%if __NASM_MAJOR__ >= 2
+		%ifdef __YASM_MAJOR__
+			global %1:function hidden
+		%elif __NASM_MAJOR__ >= 2
 			global %1:function hidden
 		%else
 			global %1

@sezero
Copy link
Contributor Author

sezero commented Oct 9, 2022

It comes with some incompatibility with win32 output, though:

cpu_asm.nasm:36: warning: Unrecognized qualifier `class'
cpu_asm.nasm:36: warning: Unrecognized qualifier `use32'
cpu_asm.nasm:41: warning: Unrecognized qualifier `class'
cpu_asm.nasm:41: warning: Unrecognized qualifier `use32'
fixed_asm.nasm:36: warning: Unrecognized qualifier `class'
fixed_asm.nasm:36: warning: Unrecognized qualifier `use32'
fixed_asm.nasm:40: warning: Unrecognized qualifier `class'
fixed_asm.nasm:40: warning: Unrecognized qualifier `use32'
lpc_asm.nasm:36: warning: Unrecognized qualifier `class'
lpc_asm.nasm:36: warning: Unrecognized qualifier `use32'
lpc_asm.nasm:42: warning: Unrecognized qualifier `class'
lpc_asm.nasm:42: warning: Unrecognized qualifier `use32'

@ktmf01
Copy link
Collaborator

ktmf01 commented Oct 9, 2022

I really prefer just getting #452 in. I suppose that would fix this issue too?

@sezero
Copy link
Contributor Author

sezero commented Oct 9, 2022

Sure.

@ktmf01 ktmf01 linked a pull request Oct 10, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants