-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Switch Android build to using clang (needs buildbot update) #8651
Conversation
Hmm, it seems not to work on my device properly... it fails to link properly. Also travis and buildbot would need updating to r11b, otherwise they may fail linking on some __thread code, although apparently TLS doesn't work in clang right now, actually. -[Unknown] |
7ca6d8b
to
3b1694c
Compare
For me Crash too me too when linked ffmpeg /opt/android-ndk/toolchains/x86_64-4.9/prebuilt/linux-x86_64/lib/gcc/x86_64-linux-android/4.9/../../../../x86_64-linux-android/bin/ld: error: jni/../../ffmpeg/android/x86_64/lib/libavcodec.a(deinterlace.o): requires dynamic R_X86_64_PC32 reloc against 'ff_pw_4' which may overflow at runtime Later i Try with -fPIC if resolved this problem. With -fPIC Fixed this warning error later we need compile ffmpeg with clang . |
Yes it's broken because we needs recompile switch ffmpeg too with clang . I'm investigate this how compile with clang ffmpeg library. |
I'm miss for arm64 they don't like asm code with clang ./libavutil/aarch64/bswap.h:31:13: error: invalid instruction mnemonic 'rev16' How fix it? This problem is related with Arm64 only others architecture compile ffmpeg ok with clang in my internal test |
d55fd46
to
dbd1d80
Compare
This works fine now (with r11c), but there's still the ffmpeg problem. Ideally, ffmpeg needs to be rebuilt with android-9 and with --disable-asm in x86. -[Unknown] |
ok, I'll take care of rebuilding ffmpeg soon. |
74a7914
to
984d21d
Compare
bb9db6f
to
6432786
Compare
6432786
to
d053407
Compare
See #8677 (comment) for notes about switching FFmpeg to clang. Looks doable. With that we'd only need the NDK updated on the buildbot (#8677 must be fixed first.) One odd thing, in my tests I'm using everything the same - but I don't get the pic errors on x86_64, even without rebuilding ffmpeg for x86_64... unlike https://travis-ci.org/hrydgard/ppsspp/jobs/131943336. But, it builds also with clang. Apparently FFmpeg is at 3.0 now and includes "asm optimizations": http://ffmpeg.org/#pr3.0 -[Unknown] |
5117edf
to
3c3bb48
Compare
I tried 3.0, and it's probably worth updating to given we'd need to recompile a bunch of things anyway, but it doesn't help the x86 issue. Anyway, on my local, all Android platforms link fine, but on Travis (even with the same binaries), they don't. I'm not sure why. I've tried cleaning my objs and also cleaning the ccache in Travis without improvement. I definitely did recompile it with Not sure what I might've done wrong. Maybe should just wait for r12... -[Unknown] |
I Have too with linked problem with Clang compiler Edit : strange I tried with -fpic or --enable-pic and gives me the same error |
Well for some part good news disable asm ffmpeg code in x86-32bits load fine under marshamllow-x86 I tested in my notebook with latest android-x86 changes Kernel 4.4.10 + mesa driver 11.2.2 + android 6.0.1 r43 version patched security may 1. FF7 crisis core test 5x resolution and HW 2x scaler full speed without problem in x86 lib. For other bad part x86_64 won't linked fine with clang for some reason this problem no exist with GCC 4.9. |
@maximu Dumb mistake, I forgot I had tried ec491ef as an alternative to It seems FFmpeg wants to be linked with -[Unknown] |
62e8e91
to
e130757
Compare
@unknownbrackets Ah I see good find with this ec491ef change compile fine with clang compiler 👍 Edit : After Tested hours later no detected any problem in android version . For Linux version not sure if my distro problem I tested under archlinux no sound here I need rebuild ffmpeg files for sound working fine. Also I detected little speed up ffmpeg performance especially noticeable in the loads that the buffer is much cleaner. |
e130757
to
6000a82
Compare
@hrydgard only things remaining for this are:
-[Unknown] |
NDK r12 released out. |
Cool, guess it's time .. I won't have time to do the work until next week unfortunately. @Orphis , feel like getting the buildbot ready for NDK 12 in the meantime? Watch out - Google has managed to swap their windows 32-bit and 64-bit downloads on their NDK download page... |
0e44722
to
aa9a26f
Compare
aa9a26f
to
b6ca610
Compare
I'll rebase this after hrydgard/ppsspp-ffmpeg#39 is merged, and pull out the Travis/Linux experiment (b6ca610.) I'll also update it to use r12b. I think we might have to keep the unused exports thing, at least for x64. I can experiment with making that x64 only. I think we still need the buildbot update... -[Unknown] |
It's now the recommended build from NDK 11+. GCC is deprecated.
This resolves a linkage error with x64 affecting Marshmallow and clang. It also makes the unexported symbols non-visible since they don't need to be - this reduces the size of the ppsspp_jni.so too. Unfortunately, it reduces the readability of stack traces.
b6ca610
to
e269a80
Compare
Thanks for updating the branch. Hopefully we will run fine on Nexus Player now :) |
@Orphis not seeing r12 or r12b on the buildbot - no env var, and trying to build with the dir fails. -[Unknown] |
Just an update: buildbot is all working now. -[Unknown] |
Someone reported worse performance in FIFA 14 (and unnamed other games) after this commit (not sure if Clang or FFmpeg): http://forums.ppsspp.org/showthread.php?tid=20315 We could try switching back from clang - although I haven't yet reproduced the performance difference. -[Unknown] |
We're not gonna switch back. GCC is deprecated for Android now so we'll just have to live with it. If we can figure out a specific part that got slower, we might be able to optimize it back up. Or there might be some compiler option to change... |
I'm least certain about the VAO func changes (aapcs-vfp), but I think it should be safe.
Did not fix all the clang warnings, just some as I was trying to get it to compile. Have not really tested on a device yet.
-[Unknown]