From: msarett Date: Tue, 21 Jul 2015 19:01:48 +0000 (-0700) Subject: Reenable yasm for Android x86 and x86-64 on Linux host X-Git-Tag: accepted/tizen/5.0/unified/20181102.025319~1645 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=cf2a6a47e4db5682798fe46f9e3ef14a27ff4b2b;p=platform%2Fupstream%2FlibSkiaSharp.git Reenable yasm for Android x86 and x86-64 on Linux host It turns out that gyp (kind of) has support for cross compiling with a different host and target. We simply need to specify CC_host and CC_target instead of CC. Making this change allows us to compile yasm on a Linux host for Android. We run into problems on Mac because the linker on a Mac host requires different command line arguments than the linker on the Android target. In looking through the code for gyp itself and speaking to Ben, it doesn't appear to me that gyp supports passing different arguments to host and target linkers. I would imagine that we would have similar problems on Windows. Below is a link to a CL that would fix this issue in gyp. It looks like it has been dropped for a long time. Thanks to Ben for this link! https://chromiumcodereview.appspot.com/10795044/ Also I'm adding a link to the build instructions for Chrome (thanks again Ben). It looks like they only support building for Android from Linux. https://code.google.com/p/chromium/wiki/AndroidBuildInstructions My next steps are: 1) Getting in touch with Torne or someone else with gyp to see if people are aware of this issue or interested in fixing it. 2) Deciding if skia should care about this issue. 3) Deciding if skia should work around this issue. It'd be really great to hear your thoughts on (2) and (3). My first thought is that we shouldn't care because, as long as we always compile the production copy of skia for Android on Linux, we will get the fast code. Is this a valid conclusion? Is there a way to write Android apps on Mac that accidentally use the slower code? If we do care, there are workarounds: For Mac, we can check in a yasm binary - it's a little smaller than the one I am deleting in this CL :-/ For Windows, we *might* be able to use the yasm.exe binary already in externals (we get this from DEPS because this is how chromium uses yasm on Windows). Are there other platforms that we care about? Let me know what you think! BUG=skia:4028 DOCS_PREVIEW= https://skia.org/?cl=1239333002 Review URL: https://codereview.chromium.org/1239333002 --- diff --git a/gyp/libjpeg-turbo.gyp b/gyp/libjpeg-turbo.gyp index 18d70ed..7ff7634 100644 --- a/gyp/libjpeg-turbo.gyp +++ b/gyp/libjpeg-turbo.gyp @@ -125,14 +125,13 @@ # Add target-specific source files. 'conditions': [ - # FIXME (msarett): Reenable yasm on Android for x86 and x86_64 - # https://code.google.com/p/skia/issues/detail?id=4028 - [ 'skia_os == "android" and "x86" in skia_arch_type', { - 'sources': [ - '../third_party/externals/libjpeg-turbo/jsimd_none.c', - ], + # TODO (msarett): Is it possible to enable cross compiling for Android on other platforms? + [ 'skia_os == "android" and host_os != "linux" and "x86" in skia_arch_type', { + 'sources': [ + '../third_party/externals/libjpeg-turbo/jsimd_none.c', + ], }], - [ 'skia_arch_type == "x86" and skia_os != "android"', { + [ 'skia_arch_type == "x86" and (skia_os != "android" or host_os == "linux")', { 'sources': [ '../third_party/externals/libjpeg-turbo/simd/jsimd_i386.c', '../third_party/externals/libjpeg-turbo/simd/jccolor-mmx.asm', @@ -170,7 +169,7 @@ '../third_party/externals/libjpeg-turbo/simd/jsimdcpu.asm', ], }], - [ 'skia_arch_type == "x86_64" and skia_os != "android"', { + [ 'skia_arch_type == "x86_64" and (skia_os != "android" or host_os == "linux")', { 'sources': [ '../third_party/externals/libjpeg-turbo/simd/jsimd_x86_64.c', '../third_party/externals/libjpeg-turbo/simd/jccolor-sse2-64.asm', @@ -257,9 +256,13 @@ ], }, }], - [ 'skia_os == "android" and (skia_arch_type == "x86" or skia_arch_type == "x86_64")', { + [ 'skia_os == "android" and host_os == "linux" and \ + (skia_arch_type == "x86" or skia_arch_type == "x86_64")', { + 'dependencies': [ + 'yasm.gyp:yasm#host', + ], 'variables': { - 'yasm_path': '../third_party/yasm/config/android/yasm', + 'yasm_path': '<(PRODUCT_DIR)/yasm', 'conditions': [ [ 'skia_arch_type == "x86"', { 'yasm_format': '-felf', diff --git a/gyp/yasm.gyp b/gyp/yasm.gyp index 44792f0..3f57581 100644 --- a/gyp/yasm.gyp +++ b/gyp/yasm.gyp @@ -65,6 +65,7 @@ 'ldflags!': [ '-stdlib=libc++', '-fsanitize=address' ], # https://crbug.com/489901 'cflags!': [ '-fsanitize=bounds', '-fsanitize=address' ], + 'libraries!': [ '-llog', ], }, 'targets': [ { diff --git a/platform_tools/android/bin/utils/setup_toolchain.sh b/platform_tools/android/bin/utils/setup_toolchain.sh index 6bb6b00..8164493 100755 --- a/platform_tools/android/bin/utils/setup_toolchain.sh +++ b/platform_tools/android/bin/utils/setup_toolchain.sh @@ -1,3 +1,8 @@ +# Copyright 2015 Google Inc. +# +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + #!/bin/bash # # setup_toolchain.sh: Sets toolchain environment variables used by other scripts. @@ -90,22 +95,60 @@ ANDROID_TOOLCHAIN_PREFIX=${GCC%%-gcc} CCACHE=${ANDROID_MAKE_CCACHE-$(which ccache || true)} -if [ -z $USE_CLANG ]; then - exportVar CC "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-gcc" - exportVar CXX "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-g++" - exportVar LINK "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-gcc" +# Cross compiling Android on Mac is not currently supported by gyp. +# It doesn't appear to be supported on Windows either. +# As of now, we will only support cross compiling on Linux. +# libjpeg-turbo assembly code for x86 and x86-64 Android devices +# must be disabled for Android on non-Linux platforms because +# of this issue. We still support compiling on Mac and other +# variants for local development, but shipping versions of Skia +# should be compiled on Linux for performance reasons. +# TODO (msarett): Collect more information about this. +if [ $(uname) == "Linux" ]; then + if [ -z $USE_CLANG ]; then + exportVar CC_target "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-gcc" + exportVar CXX_target "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-g++" + exportVar LINK_target "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-gcc" + exportVar CC_host "$CCACHE cc" + exportVar CXX_host "$CCACHE c++" + exportVar LINK_host "$CCACHE cc" + else + # temporarily disable ccache as it is generating errors + CCACHE="" + exportVar CC_target "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-clang" + exportVar CXX_target "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-clang++" + exportVar LINK_target "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-clang" + exportVar CC_host "$CCACHE clang" + exportVar CXX_host "$CCACHE clang++" + exportVar LINK_host "$CCACHE clang" + fi + + exportVar AR_target "$ANDROID_TOOLCHAIN_PREFIX-ar" + exportVar RANLIB_target "$ANDROID_TOOLCHAIN_PREFIX-ranlib" + exportVar OBJCOPY_target "$ANDROID_TOOLCHAIN_PREFIX-objcopy" + exportVar STRIP_target "$ANDROID_TOOLCHAIN_PREFIX-strip" + exportVar AR_host "ar" + exportVar RANLIB_host "ranlib" + exportVar OBJCOPY_host "objcopy" + exportVar STRIP_host "strip" else - # temporarily disable ccache as it is generating errors - CCACHE="" - exportVar CC "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-clang" - exportVar CXX "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-clang++" - exportVar LINK "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-clang" -fi + if [ -z $USE_CLANG ]; then + exportVar CC "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-gcc" + exportVar CXX "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-g++" + exportVar LINK "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-gcc" + else + # temporarily disable ccache as it is generating errors + CCACHE="" + exportVar CC "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-clang" + exportVar CXX "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-clang++" + exportVar LINK "$CCACHE $ANDROID_TOOLCHAIN_PREFIX-clang" + fi -exportVar AR "$ANDROID_TOOLCHAIN_PREFIX-ar" -exportVar RANLIB "$ANDROID_TOOLCHAIN_PREFIX-ranlib" -exportVar OBJCOPY "$ANDROID_TOOLCHAIN_PREFIX-objcopy" -exportVar STRIP "$ANDROID_TOOLCHAIN_PREFIX-strip" + exportVar AR "$ANDROID_TOOLCHAIN_PREFIX-ar" + exportVar RANLIB "$ANDROID_TOOLCHAIN_PREFIX-ranlib" + exportVar OBJCOPY "$ANDROID_TOOLCHAIN_PREFIX-objcopy" + exportVar STRIP "$ANDROID_TOOLCHAIN_PREFIX-strip" +fi # Create symlinks for nm & readelf and add them to the path so that the ninja # build uses them instead of attempting to use the one on the system. diff --git a/site/user/quick/android.md b/site/user/quick/android.md index 9df11a8..44e3161 100644 --- a/site/user/quick/android.md +++ b/site/user/quick/android.md @@ -4,7 +4,9 @@ Android Prerequisites ------------- -_Currently we only support building Skia for Android on a Linux or Mac host!_ +_Currently we only support building Skia for Android on a Linux or Mac host! In addition, + we only use the Mac build for local development. All shipping variants are compiled on + Linux for performance reasons._ The following libraries/utilities are required in addition to those needed for a standard skia checkout: diff --git a/third_party/yasm/README.skia b/third_party/yasm/README.skia index 19a259c..94bdd78 100644 --- a/third_party/yasm/README.skia +++ b/third_party/yasm/README.skia @@ -8,10 +8,6 @@ the yasm code is pulled into externals via the DEPS file. Chromium builds yasm using the below procedure. We take a few shortcuts. We mirror Chromium's yasm repositories in our DEPS file, and we copy these config files directly from Chromium. -NOTE: We were are currently unable to build yasm for Android on x86 and x86_64. Instead we are -using a precompiled binary in the config/android directory. -TODO (msarett): Fix this! - Excerpt from [chromium] //src/third_party/yasm/README.chromium: Instructions for recreating the yasm.gyp file. diff --git a/third_party/yasm/config/android/yasm b/third_party/yasm/config/android/yasm deleted file mode 100755 index 1a2ca97..0000000 Binary files a/third_party/yasm/config/android/yasm and /dev/null differ