Page MenuHomePhabricator

[Test-Suite] Support Cross-Compilation and Cross-execution targeting arm64-linux-android
Needs ReviewPublic

Authored by ziangwan on Jul 9 2019, 10:31 AM.

Details

Summary

This patch allows users to cross-compile and cross-execute LLVM test-suite on an Android device for testing and benchmark purposes.

Usage:

  1. Obtain llvm-test-suite and Android NDK for your host machine. The test-suite also supports SPEC CPU2006 benchmark. (SPEC CPU2017 requires too much system memory and is not suitable for an actual android device.)
  1. Get the build template inside cmake/caches/target-arm64-android-template.sh and create your own build script build.sh. Put build.sh inside the build directory you desire.
  1. Run sh build.sh to setup the build. Then do ninja to build.
  1. Run ninja push_android to push the tests onto an Android device, whose serial number is specified inside build.sh.
  1. Run llvm-lit -j1 -o result.json ./build to cross-execute.

Notes

  1. I manually exclude several tests and benchmarks for Android build by checking against TEST_SUITE_REMOTE_CLIENT STREQUAL "adb" in corresponding CMakeLists.txt files. All of them are excluded due to build failures. A list of excluded tests and benchmarks with concise build failure messages:
    • test-suite/External/SPEC/CINT2006/CMakeLists.txt:
      • Exclude cpu2006_subdir(400.perlbench)
        • incomplete definition of type 'struct __sFILE'
        • Many other errors
      • Exclude cpu2006_subdir(464.h264ref)
        • sys/timeb.h not found <- this header file is deprecated and no longer exists inside Android NDK.
      • Exclude cpu2006_subdir(483.xalancbmk)
        • undefined reference to `xercesc_2_5::XMLPlatformUtils::openFile(unsigned short const*, xercesc_2_5::MemoryManager*)'
        • Many other errors
    • test-suite/External/SPEC/CINT2006/462.libquantum/CMakeLists.txt
      • Use diff -w instead of diff --strip-trailing-cr
        • --strip-trailing-cr is not supported in Android's diff
    • test-suite/External/SPEC/CFP2006/CMakeLists.txt
      • Exclude cpu2006_subdir(446.dealII)
        • error: no template named 'pair' in namespace 'std'
    • test-suite/Multisource/Applications/CMakeLists.txt
      • Exclude add_subdirectory(JM)
      • Exclude add_subdirectory(hexxagon)
      • Exclude out add_subdirectory(SPASS)
        • sys/timeb.h not found
      • Exclude add_subdirectory(ClamAV)
        • undefined reference to `endprotoent'
      • Exclude add_subdirectory(siod)
        • undefined reference to `getpwent'
      • Exclude add_subdirectory(Burg)
        • error: conflicting types for 'atoi'
      • Exclude add_subdirectory(spiff)
        • parse.c:85: undefined reference to `index'
    • test-suite/Multisource/Benchmarks/MiBench/CMakeLists.txt
      • Exclude add_subdirectory(network-patricia)
        • undefined reference to `bcopy'
      • Exclude add_subdirectory(office-ispell)
        • fatal error: 'sgtty.h' file not found
    • test-suite/Multisource/Benchmarks/Prolangs-C/CMakeLists.txt
      • Exclude add_subdirectory(bison)
        • conflicts.c:266: undefined reference to `bcopy'
    • test-suite/Multisource/Benchmarks/DOE-ProxyApps-C++/CMakeLists.txt
      • Exclude add_subdirectory(CLAMR)
        • fatal error: 'sys/sysctl.h' file not found
    • test-suite/Multisource/Benchmarks/CMakeLists.txt
      • Exclude add_subdirectory(7zip)
        • 7zip/CPP/7zip/UI/Console/UserInputUtils.cpp:81:25: error: use of undeclared identifier 'getpass'
        • Many other errors
    • test-suite/SingleSource/UnitTests/
      • Exclude 2005-05-11-Popcount-ffs-fls.c
        • 110: undefined reference to `ffsl'
    • test-suite/Microbenchmarks
      • Exclude the whole folder because of unsupported test-format when cross-exec
        • An issue on the llvm-lit side
    • test-suite/MultiSource/Benchmarks/Prolangs-C/CMakeLists.txt
      • Exclude add_subdirectory(unix-small)
        • can't create /tmp/rmJKvjCz.
    • test-suite/MultiSource/Applications/CMakeLists.txt
      • Exclude out add_subdirectory(obsequi)
        • Unable to find library -lsupc++
  1. Initially, I just want to use the test-suite to benchmark a compiler feature. Therefore, I only care about the benchmarks in the test-suite.

The patch works locally for me. If you have any suggestions / comments / encounter bugs when targeting Android, please let me know.

Diff Detail

Event Timeline

ziangwan created this revision.Jul 9 2019, 10:31 AM

Thank you very much Ziang for sharing this work!
I'd love to more easily benchmark on Android devices - and this seems to be a good step in the right direction.

Please find some of my immediate thoughts on the patch below.

MultiSource/Benchmarks/CMakeLists.txt
32–34

I'm wondering if (NOT "${TARGET_OS}" STREQUAL "Android") wouldn't be a better test?
Isn't the reason many of these programs fail to build that some header file isn't provided on the Android platform?
Also, that would make the test more similar to other tests already in here that remove specific benchmarks for specific architectures or OSs.

cmake/caches/target-arm64-android-template.sh
1–47 ↗(On Diff #208723)

I wonder if you're also aiming to use LNT as an extra layer on top of this test-suite?
I tend to use LNT to build/run tests/benchmarks. I wonder if support was added in LNT, would there still be a lot of value in having this custom shell script?
I would love to avoid having to go through different benchmarking flows depending on which OS I'm running benchmarks on.

31 ↗(On Diff #208723)

Shouldn't the SPEC_CPU_LINUX define get added somehow as part of the External/SPEC cmake files rather than added here on the command line?

lit.site.cfg.in
10

My understanding is that adding quotes here will make the profile_generate feature break in some circumstances.

ziangwan updated this revision to Diff 209055.Jul 10 2019, 1:39 PM

Update diff:

  1. Have a workaround inside top-level CMakeLists.txt to set TARGET_OS to Android.
  2. Test against TARGET_OS instead of TEST_SUITE_REMOTE_CLIENT in the build system.
  3. lit.site.cfg.in: remove the double quote around @TEST_SUITE_PROFILE_GENERATE@.
ziangwan marked 6 inline comments as done.Jul 10 2019, 1:44 PM
ziangwan added inline comments.
MultiSource/Benchmarks/CMakeLists.txt
32–34

Most of the build failures are due to missing header files in the Android platform. There is an issue with the data format of Microbenchmark. I didn't attempt to fix that since I didn't care much about Microbenchmarks. I simply disabled them. There are a few tests/benchmarks I disabled because of bugs that prevent compilation. I didn't try to fix them either.

cmake/caches/target-arm64-android-template.sh
1–47 ↗(On Diff #208723)

I currently don't have the plan to add Android-support into LNT. My goal is to run test-suite and SPEC 2006 on Android to obtain data.

fhahn added a subscriber: fhahn.Jul 11 2019, 3:08 AM
fhahn added inline comments.
cmake/caches/target-arm64-android-template.sh
1 ↗(On Diff #209055)

I think cmake/caches should only contain cmake cache files (a set of pre-configured settings), that can be used with cmake -C, not bash scripts.

IIUC the key thing to do here is to locate the android ndk and set the paths accordingly. Is there a standard way to locate the ndk, similar to how it is done for the Xcode SDKs ? (like https://github.com/llvm/llvm-test-suite/blob/master/cmake/caches/util/xcode_sdk.cmake)

6 ↗(On Diff #209055)

This setting and the ones below seems very system/setup specific, and related to general test-suite setup, not android related.

42 ↗(On Diff #209055)

Can this be handled by the general sync logic, that copies over the other artifacts? Also, is using a libcxx built alongside clang supported?

ziangwan marked 3 inline comments as done.Jul 11 2019, 10:30 AM
ziangwan added inline comments.
cmake/caches/target-arm64-android-template.sh
6 ↗(On Diff #209055)

Exactly, there is a requirement on host system setup for llvm-test-suite to correctly execute:

Special Requirement for build directory: Due to the current state of the build system, the tests are generated using the absolute path on the host machine. For example, if you build the test-suite at /aa/bb/cc on the host machine, all the build files should also be stored at /aa/bb/cc on the target android device for the tests to correctly execute. Therefore, we need to create a build directory, whose absolute path is also a writable location on the target Android device, on the host machine. For example, if you want the tests to run on /data/local/tmp/build on the Android device, you should build the test-suite inside /data/local/tmp/build as well. You specifically cannot use arbitrary path on the host machine since Android's SeLinux prevents creating directories at sysroot.

ziangwan marked an inline comment as done.Jul 11 2019, 10:31 AM
ziangwan marked an inline comment as not done.Jul 11 2019, 10:40 AM
danalbert added a subscriber: danalbert.
danalbert added inline comments.
CMakeLists.txt
190

Got a link to a bug? CMAKE_SYSTEM_NAME should be Android for Android.

External/SPEC/CFP2006/CMakeLists.txt
10

Everything that's excluded should have a comment with a link to a bug explaining what we need to fix, and should probably also include message(WARNING "Skipping <test name> on Android: <bug number>")

External/SPEC/CINT2006/462.libquantum/CMakeLists.txt
11–24

Have you filed a bug? That sounds trivial to add to toybox.

MultiSource/Benchmarks/CMakeLists.txt
32–34

Missing platform stuff, or missing third-party stuff? i.e. is libc missing something, or is the problem that Android doesn't include the 7zip development package? The former needs to be fixed and/or have bugs filed.

cmake/caches/target-arm64-android-template.sh
6 ↗(On Diff #209055)

Probably worth a comment referencing whatever doc you're quoting.

8 ↗(On Diff #209055)

(why is this "doc" space?)

Should probably keep everything under a common subdirectory to avoid any collisions. /data/local/tmp/llvm-test-suite/...

Is it actually necessary to push the NDK to the device? None of the toolchains are built to run on Android. I can't even think of what use the sysroot would be.

10 ↗(On Diff #209055)

Same. Why are we running the compiler on the device?

32 ↗(On Diff #209055)

This is needed because the compiler being used isn't the NDK toolchain, right? It's just the bare compiler without the sysroot and all that? If so, you *might* be able to use -gcc-toolchain $ANDROID_NDK_LOCATION/toolchains/llvm/prebuilt/$HOST_SYSTEM instead of specifying the internal location of libgcc. (If not, fixing the driver to make that work would probably be good)

42 ↗(On Diff #209055)

Also, is using a libcxx built alongside clang supported?

Not really. I haven't had the time to upstream all of our patches. It might work if this only needs to support new devices, but we have a number of things we have to do to build libc++ for Jelly Bean.

43 ↗(On Diff #209055)

nit: indent continuation lines

litsupport/modules/android.py
2

Nit: docstring convention is:

"""Short summary sentence.

Additional description if needed.
"""

(also, is LLVM Python 3 yet?)

5

nit: stdlib imports come first and are separated by a blank line

import logging
import os
import subprocess

from litsupport import testplan
23

prepend

25

It's doing more than that. Maybe make_adb_command?

utils/rsync_android.sh
1 ↗(On Diff #209055)

This isn't running rsync, so don't call it that. Just sync_android.sh or push_android.sh.

11 ↗(On Diff #209055)

+jmgao: should adb push have an --unsymlink option?

31 ↗(On Diff #209055)

I'm guessing it's slower to run a lot of pushes instead of a single push command... but idk if jmgao likes the idea of adb push --filter $PATTERN

pcc added a subscriber: pcc.Jul 11 2019, 12:37 PM

FWIW, I did something similar: https://github.com/pcc/llvm-test-suite/tree/android
but never got time to clean it up and send it for review.

Here are a few differences between your changes and mine:

  • I statically linked libc++ (avoids needing to mess around with LD_LIBRARY_PATH and pushing libc++_shared.so).
  • I fixed all of the tests that fail to compile/link on Android instead of disabling them. That should probably be easy to send out as a separate review, so I'll try to do that soon.
  • I wrote cmake cache files for Android instead of using a shell script.
cmake/caches/target-arm64-android-template.sh
6 ↗(On Diff #209055)

I addressed this in my implementation by basically search-and-replacing any commands sent to the device to change host-side paths into device-side paths (which is basically /data/local/tmp/llvm-test-suite/staging plus the source-relative path).

32 ↗(On Diff #209055)

What I usually do is: -B${ANDROID_NDK_PATH}/toolchains/llvm/prebuilt/linux-x86_64.

Would it be worth considering moving libgcc.a into the sysroot in a future release of the NDK? Then I believe only --sysroot should be required.

pcc added inline comments.Jul 11 2019, 12:38 PM
utils/rsync_android.sh
31 ↗(On Diff #209055)

In my implementation I used the regular rsync command to copy the files to a staging directory and then adb pushed the staging directory.

danalbert added inline comments.Jul 11 2019, 12:55 PM
cmake/caches/target-arm64-android-template.sh
32 ↗(On Diff #209055)

Maybe. I was somewhat trying to keep toolchain components separate from the platform APIs, but that's not really that important in this case since that's a fairly blurry line anyway. On the other hand, we're hopefully switching to libclangrt_builtins soon anyway, so maybe not worth the effort for either.

ziangwan marked 15 inline comments as done.Jul 11 2019, 1:21 PM
ziangwan added inline comments.
External/SPEC/CINT2006/462.libquantum/CMakeLists.txt
11–24

I've filed a bug.

cmake/caches/target-arm64-android-template.sh
8 ↗(On Diff #209055)

"docspace" is an arbitrary name I use for myself. You don't have to use "docspace".

Yes, you can use /data/local/tmp/llvm-test-suite/.. as your path. This shell script is just a template.

I am not pushing the whole NDK to the device. It is only used during compilation.

10 ↗(On Diff #209055)

We are not running compilers on the device. This path is specified if you have a custom build of clang. In my case, I do.

litsupport/modules/android.py
2

This file can be executed using python2.

danalbert added inline comments.Jul 11 2019, 1:41 PM
cmake/caches/target-arm64-android-template.sh
8 ↗(On Diff #209055)

Then why is it a /data path? That's part of the Android file system. Mirroring that on the host requires root.

litsupport/modules/android.py
2

I mean can it be *not* Python 2. Python 2 reaches EOL at the end of the year, so if using python 3 is an option in LLVM we should be using Python 3. Specifically, if we can assume Python 3.6+, there are some cleanups that can be made here.

ziangwan marked 11 inline comments as done.Jul 11 2019, 2:37 PM
ziangwan added inline comments.
litsupport/modules/android.py
2

Yeah, we can use f-string instead of .format. However, the majority of the time spent is execution time. Replacing .format with f-strings is not a big issue.

utils/rsync_android.sh
31 ↗(On Diff #209055)

It is not that slow to use many adb push command. I takes around 3 min to do the whole transfer.

ziangwan marked an inline comment as done and an inline comment as not done.Jul 11 2019, 2:38 PM
ziangwan marked 3 inline comments as done.Jul 11 2019, 3:47 PM
ziangwan added inline comments.
cmake/caches/target-arm64-android-template.sh
8 ↗(On Diff #209055)

I will follow Peter's advice. I will do a string replace of host-side path with device-side path.

srhines added inline comments.Jul 12 2019, 3:40 AM
cmake/caches/target-arm64-android-template.sh
32 ↗(On Diff #209055)

Unfortunately, libgcc.a is here to stay until the Android unwinder can be used as part of the builtins. I don't think that is considered a high priority though. I would prefer that libgcc.a exist somewhere better in the NFL, considering we could drop the rest of the GCC dirs if we do that.

32 ↗(On Diff #209055)

Lol autocorrect. NDK, not NFL.

ziangwan retitled this revision from LLVM Test-Suite: Support Cross-Compilation and Cross-execution targeting arm64-linux-android to [Test-Suite] Support Cross-Compilation and Cross-execution targeting arm64-linux-android.Jul 12 2019, 2:45 PM
ziangwan updated this revision to Diff 210210.Jul 16 2019, 3:55 PM
ziangwan edited the summary of this revision. (Show Details)

Update diff.

  1. Create a new templates folder inside cmake to store the targeting android template shell script (subject to change)
  2. Rename and improve the file transfer script push_android.py.
  3. Use strreplace in the android module to support arbitrary build directory on the host system. In that way, we don't need root privilege to build the test cases anymore.
ziangwan marked 2 inline comments as done.Jul 16 2019, 3:56 PM
ziangwan added inline comments.
CMakeLists.txt
190

Resolved by using NDK's android.toolchain.cmake file.