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

[Ziang Wan via Phabricator via llvm-commits]

ziangwan marked 11 inline comments as done.
ziangwan added inline comments.


================
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
----------------
danalbert wrote:
> 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.
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.


================
Comment at: utils/rsync_android.sh:31
+do
+    adb -s $TARGET push $file $file
+done
----------------
pcc wrote:
> danalbert wrote:
> > 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`
> In my implementation I used the regular `rsync` command to copy the files to a staging directory and then `adb push`ed the staging directory.
It is not that slow to use many `adb push` command. I takes around 3 min to do the whole transfer.


Repository:
  rT test-suite

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

https://reviews.llvm.org/D64427