[PATCH] D64427: LLVM Test-Suite: Support Cross-Compilation and Cross-execution targeting arm64-linux-android

[Dan Albert via Phabricator via llvm-commits]

danalbert added inline comments.


================
Comment at: cmake/caches/target-arm64-android-template.sh:8
+# REQUIRED: path to android ndk that contains necessary header files and libraries
+ANDROID_NDK_LOCATION="/data/local/tmp/docspace/android-ndk-r20"
+# REQUIRED: path to llvm build bin
----------------
ziangwan wrote:
> danalbert wrote:
> > (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.
> "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.
Then why is it a `/data` path? That's part of the Android file system. Mirroring that on the host requires root.


================
Comment at: litsupport/modules/android.py:1
+"""Test module to execute a benchmark through adb on a connected Android
+device. This assumes all relevant directories and files are present on the remote
----------------
ziangwan wrote:
> danalbert wrote:
> > Nit: docstring convention is:
> > 
> > 
> > ```
> > """Short summary sentence.
> > 
> > Additional description if needed.
> > """
> > ```
> > 
> > (also, is LLVM Python 3 yet?)
> This file can be executed using python2.
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.


Repository:
  rT test-suite

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64427/new/

https://reviews.llvm.org/D64427