-
Notifications
You must be signed in to change notification settings - Fork 272
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
clang compilation error with unknown argument, works with gcc 4.9 #111
Comments
https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html
Android is only arm5+ and aapcs. This option is meaningless for Android. Those flags should just be removed from the For the sake of compatibility though, @stephenhines, thoughts? All I could really find was https://lists.gnu.org/archive/html/grub-devel/2013-12/msg00094.html, which says the thumb interwork is the default for Clang. Assuming that's true, does Clang have precedent for adding no-op flags for GCC command line compatibility? |
Alternatively, we could do #92. |
That only helps the build systems that we fix though. I'm not even sure if there is a (correct) way to do this for cmake. |
Oh, that's for ndk-build? I thought that was for replacing the standalone toolchain clang with a wrapper that strips out gcc-only flags. |
Good point. Once we have the toolchain wrappers we can fix this there. I don't think we'll have those for some time though, so a clang fix might be nice to have now if it's reasonable. |
Any clang fix would probably not land until r13 at the earliest, and I would hope that we can have some simple wrappers at that point too. I don't think that the upstream community is too fond of flags that are no-ops either, although they will match gcc flags for actual features. In this case, since the feature is going to be always-on, I can't see struggling to add this for very little value. |
@DanAlbert I am not sure whether -mthumb-interwork is meaningless for android. Before submitting this issue, I did go through the gcc compiler options and the minimum android CTS requirement of ARMv5TE or better. Logically, I inferred that the flag could be done away with. However, I chanced upon this thread on the android-ndk google group - https://groups.google.com/forum/#!topic/android-ndk/AVz2FsOoMdI.
Since the reply seems to be from an engineer on the android NDK team, I presume that removing the flag may not be prudent. This is further supported empirically, although unscientifically, by the presence of the flag across many results - https://groups.google.com/forum/#!searchin/android-ndk/mthumb-interwork . May be someone could reach out to the gentleman within Google to better understand whether the flag can actually be removed while still maintaining ABI compatibility (is this his handle? - https://github.com/digit-android). In case that flag is required and is relevant for clang, then may be we need a different solution than a no-op. In case that flag is not required, its great (and may I add, logical). In either case, it would be interesting to understand the technical reason behind that thread exchange. On a related note, you referenced https://lists.gnu.org/archive/html/grub-devel/2013-12/msg00094.html (emphasis added) -
Based on my very limited read, clang does not generate code that is interworkable since Thumb-1 seems to be supported on armeabi (https://developer.android.com/ndk/guides/abis.html#sa). So I am curious to understand your deduction that thumb-interwork is default for clang. Again, I am not an expert on compilers/instruction sets/archs. |
Bumping this. Would appreciate guidance with respect to resolving this bug. I may be wrong here but from what I understand, gcc is no longer supported (as of r13) and clang is not there yet. |
@stephenhines @pirama-arumuga-nainar: Could either of you confirm Clang's behavior here? If Clang does the right thing here by default we can close, otherwise we'll need to get a fix for this. |
Could someone from within Google connect internally with David 'Digit' Turner (@digit-android) to better understand this? I request this because in that NDK google group thread (https://groups.google.com/forum/#!topic/android-ndk/AVz2FsOoMdI), he indicated the need for -mthumb-interwork for ABI compatibility. As such, I presume he would be in a better, possibly definitive, position to help us out. |
Hello, Dan kindly contacted me regarding this issue. For some reason I was not receiving any github email :-/ The short answer is that Android uses the AAPCS ARM ABI specification, which requires interworking (see section 5.6). Whether Technically, interworking means that function pointer calls should always use the ARMv7-A introduced changes to the way instructions are decoded by the CPU, e.g. So in a nutshell: If you want to run your code on ARMv5TE or ARMv6 CPUs, you must implement interworking or you won't be able to run properly on Android systems. If your code only runs on ARMv7-A or higher, you should not care about interworking at all. What happens with -mthumb-interwork / -mno-thumb-interwork is compiler-specific, e.g. recent versions of GCC will ignore it since -mabi=aapc is the default (or is it aapc-linux, I haven't followed through since a long time). |
I just performed some tests and it looks like the prebuilt GCC 4.9 that comes with AOSP ignores In other words, just omitting the flags completely from the build scripts should be enough for GCC. Don't know about CLang though. |
Dan - Thanks for connecting with David. From what I understand - if (arm_arch < 7) -mthumb-interwork is required, else not. Since we use mabi=aapcs, GCC always generates code that interworks between thumb1 and arm instructions. This is further confirmed from your tests with AOSP (==NDK?) GCC 4.9. The armv7a arch is a requirement for API level >= 19 (https://source.android.com/compatibility/4.4/android-4.4-cdd.pdf#section-3.3) but there are 18.6% users with API level < 19 as of September 2016 (https://developer.android.com/about/dashboards/index.html). There is no public dashboard of arch with API levels, so one has to support arm_arch < 7. Assuming there are at least a billion android devices, one can hardly afford to disregard 186 million of them - unless someone here knows more about this.
As for this bug, the focus here shifts to clang/llvm. Based on David's explanation, I went spelunking into toolchain/clang code with a rudimentary search for the string "interwork". I will share my understanding here (caveat - no expertise/knowledge apart from basic ability to read and search): The relevant part in clang code seems to start from https://android.googlesource.com/toolchain/clang/+/master/lib/Basic/Targets.cpp#4553 where the ARMTargetInfo class is defined and ends at https://android.googlesource.com/toolchain/clang/+/master/lib/Basic/Targets.cpp#5493. I would suggest reading it to understand what clang seems to be doing with the ARM Target. Could I request the clang experts here to suggest a way forward? |
First, there are very probably far less than 18.6% of devices running on ARMv6 or lower, we don't have the real numbers but a lot of the devices supporting API level < 19 are already running on ARMv7-A. The exceptions are really old phones, and, surprisingly the 1st generation Rasperry Pi (The Pi 2 and Pi 3 are on ARMv7-A). As an Android developer, I would probably not bother too much about ARMv6 support, just put a runtime check for ARMv7-A at the start of your application, and mention it on your Store page that this doesn't support older phones. I don't have much time to perform the checks myself, but essentially you can do that by compiling a small object file containing a single function that performs a function-pointer-call as in:
And look at the disassembly, if a "blx" instruction is used to implement the call, then interwork was enabled by the compiler, otherwise, it will be a "bl". For example on how to look at the results (assuming the source file is named "foo.c"): /path/to/compiler -O2 -march=armv4t -mno-thumb-interwork foo.c /path/to/compiler -O2 -march=armv4t -mthumb-interwork foo.c try with different versions for -march= (e.g. armv5te, armv6. armv7-a) and see the results. Regarding aapcs versus aapcs-linux, I believe the latter specifies a few things that are left unresolved in aapcs, see http://www.boost.org/doc/libs/1_51_0_beta1/libs/context/doc/pdf/arm-linux-aapcs.pdf The biggest change is that enum are 32-bit ints by default (while they have variable size depending on the enum values in aapcs). IIRC, Android uses the aapcs-linux variant, but I'm fairly rusty on this one :-/ |
Isn't it enough to avoid building @DanAlbert Should perhaps dropping |
@fornwall we're actually working on a roadmap to dropping armeabi support, perhaps even starting gently in r15 by no longer building it by default. |
We don't plan to implement a -mthumb-interwork no-op flag in Clang, so I am closing this issue out. The proper fix is to not pass this flag when compiling with Clang. |
I am not an expert at compilers, so please bear with me.
I am trying to compile https://github.com/projectNe10/Ne10/ using clang (NDK version r11c), since ndk has deprecated gcc. Using gcc 4.9 works fine. However, using clang, the compilation barfs with:
Changing the llvm-as to arm-linux-androideabi-as does not resolve the compilation error, although it does resolve the warning
The relevant files are:
https://github.com/projectNe10/Ne10/blob/master/android/android_config.cmake and https://github.com/projectNe10/Ne10/blob/master/CMakeLists.txt.
Based on my limited understanding, -mthumb-interwork is required to be passed for the code to work on android. However, a cursory glance at http://clang.llvm.org/docs/UsersManual.html#arm does not seem to reveal any useful information, nor does searching the bugzilla database at llvm.org. So, either clang does not support this compile time option or I am missing something really obvious here.
The text was updated successfully, but these errors were encountered: