From 809ccf37ec836d0df64afd0b13023fd968d505a4 Mon Sep 17 00:00:00 2001 From: mtklein Date: Thu, 5 May 2016 10:58:39 -0700 Subject: [PATCH] Remove NEON runtime detection support. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1952953004 CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot Review-Url: https://codereview.chromium.org/1952953004 --- gyp/common_conditions.gypi | 5 -- gyp/common_variables.gypi | 1 - gyp/libjpeg-turbo.gyp | 20 ++++---- gyp/opts.gyp | 3 +- gyp/opts.gypi | 2 - platform_tools/android/bin/android_setup.sh | 6 +-- src/core/SkBitmapProcState.cpp | 4 +- src/core/SkBitmapProcState_matrixProcs.cpp | 6 +-- src/core/SkBlitter_RGB16.cpp | 8 ++-- src/core/SkOpts.cpp | 6 --- src/core/SkUtilsArm.h | 71 ++--------------------------- src/opts/SkOpts_neon.cpp | 54 ---------------------- 12 files changed, 27 insertions(+), 159 deletions(-) delete mode 100644 src/opts/SkOpts_neon.cpp diff --git a/gyp/common_conditions.gypi b/gyp/common_conditions.gypi index 0f8fa51..e784e51 100644 --- a/gyp/common_conditions.gypi +++ b/gyp/common_conditions.gypi @@ -289,11 +289,6 @@ '-mfpu=neon', ], }], - [ 'arm_neon_optional == 1', { - 'defines': [ - 'SK_ARM_HAS_OPTIONAL_NEON', - ], - }], [ 'skia_os != "linux"', { 'cflags': [ '-mfloat-abi=softfp', diff --git a/gyp/common_variables.gypi b/gyp/common_variables.gypi index bee5590..969e484 100644 --- a/gyp/common_variables.gypi +++ b/gyp/common_variables.gypi @@ -196,7 +196,6 @@ # so that siblings of the level-1 'variables' dict can see them. 'arm_version%': '<(arm_version)', 'arm_neon%': '<(arm_neon)', - 'arm_neon_optional%': 0, 'mips_arch_variant%': 'mips32', 'mips_dsp%': 0, 'skia_os%': '<(skia_os)', diff --git a/gyp/libjpeg-turbo.gyp b/gyp/libjpeg-turbo.gyp index c0853d8..7da66a1 100644 --- a/gyp/libjpeg-turbo.gyp +++ b/gyp/libjpeg-turbo.gyp @@ -129,16 +129,16 @@ [ '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', + '../third_party/externals/libjpeg-turbo/simd/jccolor-mmx.asm', '../third_party/externals/libjpeg-turbo/simd/jccolor-sse2.asm', - '../third_party/externals/libjpeg-turbo/simd/jcgray-mmx.asm', + '../third_party/externals/libjpeg-turbo/simd/jcgray-mmx.asm', '../third_party/externals/libjpeg-turbo/simd/jcgray-sse2.asm', - '../third_party/externals/libjpeg-turbo/simd/jchuff-sse2.asm', + '../third_party/externals/libjpeg-turbo/simd/jchuff-sse2.asm', '../third_party/externals/libjpeg-turbo/simd/jcsample-mmx.asm', '../third_party/externals/libjpeg-turbo/simd/jcsample-sse2.asm', - '../third_party/externals/libjpeg-turbo/simd/jdcolor-mmx.asm', + '../third_party/externals/libjpeg-turbo/simd/jdcolor-mmx.asm', '../third_party/externals/libjpeg-turbo/simd/jdcolor-sse2.asm', - '../third_party/externals/libjpeg-turbo/simd/jdmerge-mmx.asm', + '../third_party/externals/libjpeg-turbo/simd/jdmerge-mmx.asm', '../third_party/externals/libjpeg-turbo/simd/jdmerge-sse2.asm', '../third_party/externals/libjpeg-turbo/simd/jdsample-mmx.asm', '../third_party/externals/libjpeg-turbo/simd/jdsample-sse2.asm', @@ -157,11 +157,11 @@ '../third_party/externals/libjpeg-turbo/simd/jidctint-sse2.asm', '../third_party/externals/libjpeg-turbo/simd/jidctred-mmx.asm', '../third_party/externals/libjpeg-turbo/simd/jidctred-sse2.asm', - '../third_party/externals/libjpeg-turbo/simd/jquant-3dn.asm', + '../third_party/externals/libjpeg-turbo/simd/jquant-3dn.asm', '../third_party/externals/libjpeg-turbo/simd/jquantf-sse2.asm', '../third_party/externals/libjpeg-turbo/simd/jquanti-sse2.asm', - '../third_party/externals/libjpeg-turbo/simd/jquant-mmx.asm', - '../third_party/externals/libjpeg-turbo/simd/jquant-sse.asm', + '../third_party/externals/libjpeg-turbo/simd/jquant-mmx.asm', + '../third_party/externals/libjpeg-turbo/simd/jquant-sse.asm', '../third_party/externals/libjpeg-turbo/simd/jsimdcpu.asm', ], }], @@ -194,7 +194,7 @@ }], [ 'skia_arch_type == "arm"', { 'conditions': [ - [ 'arm_version >= 7 and (arm_neon == 1 or arm_neon_optional == 1)', { + [ 'arm_version >= 7 and arm_neon == 1', { 'sources': [ '../third_party/externals/libjpeg-turbo/simd/jsimd_arm.c', '../third_party/externals/libjpeg-turbo/simd/jsimd_arm_neon.S', @@ -213,7 +213,7 @@ '../third_party/externals/libjpeg-turbo/jsimd_none.c', ], }], - + # Build rules for an asm file. # On Windows, we use the precompiled yasm binary. # On Linux, we build our patched yasm and use it. diff --git a/gyp/opts.gyp b/gyp/opts.gyp index ae4b294..ad8da33 100644 --- a/gyp/opts.gyp +++ b/gyp/opts.gyp @@ -61,10 +61,9 @@ # ARM), the compiler doesn't like that. 'cflags!': [ '-fno-omit-frame-pointer', '-mapcs-frame', '-mapcs' ], 'cflags': [ '-fomit-frame-pointer' ], - 'variables': { 'arm_neon_optional%': '<(arm_neon_optional>' }, 'sources': [ '<@(armv7_sources)' ], 'conditions': [ - [ 'arm_neon == 1 or arm_neon_optional == 1', { + [ 'arm_neon == 1', { 'dependencies': [ 'opts_neon' ] }], ], diff --git a/gyp/opts.gypi b/gyp/opts.gypi index f3f61ef..fbc3d49 100644 --- a/gyp/opts.gypi +++ b/gyp/opts.gypi @@ -19,7 +19,6 @@ '<(skia_src_path)/opts/SkBitmapProcState_matrixProcs_neon.cpp', '<(skia_src_path)/opts/SkBlitMask_opts_arm_neon.cpp', '<(skia_src_path)/opts/SkBlitRow_opts_arm_neon.cpp', - '<(skia_src_path)/opts/SkOpts_neon.cpp', ], 'arm64_sources': [ '<(skia_src_path)/opts/SkBitmapProcState_arm_neon.cpp', @@ -29,7 +28,6 @@ '<(skia_src_path)/opts/SkBlitMask_opts_arm_neon.cpp', '<(skia_src_path)/opts/SkBlitRow_opts_arm.cpp', '<(skia_src_path)/opts/SkBlitRow_opts_arm_neon.cpp', - '<(skia_src_path)/opts/SkOpts_neon.cpp', ], 'mips_dsp_sources': [ diff --git a/platform_tools/android/bin/android_setup.sh b/platform_tools/android/bin/android_setup.sh index f242bfa..0a6fd5f 100755 --- a/platform_tools/android/bin/android_setup.sh +++ b/platform_tools/android/bin/android_setup.sh @@ -35,13 +35,13 @@ while (( "$#" )); do elif [[ "$1" == "--gcc" ]]; then USE_CLANG="false" elif [[ "$1" == "--clang" ]]; then - USE_CLANG="true" + USE_CLANG="true" elif [[ "$1" == "--logcat" ]]; then LOGCAT=1 elif [[ "$1" == "--verbose" ]]; then VERBOSE="true" elif [[ "$1" == "--vulkan" ]]; then - SKIA_VULKAN="true" + SKIA_VULKAN="true" else APP_ARGS=("${APP_ARGS[@]}" "${1}") fi @@ -126,7 +126,7 @@ setup_device() { ANDROID_ARCH="arm" ;; arm_v7 | xoom) - DEFINES="${DEFINES} skia_arch_type=arm arm_neon_optional=1 arm_version=7" + DEFINES="${DEFINES} skia_arch_type=arm arm_neon=0 arm_version=7" ANDROID_ARCH="arm" ;; arm_v7_neon | nexus_4 | nexus_5 | nexus_6 | nexus_7 | nexus_10) diff --git a/src/core/SkBitmapProcState.cpp b/src/core/SkBitmapProcState.cpp index fc74911..33dd8f5 100644 --- a/src/core/SkBitmapProcState.cpp +++ b/src/core/SkBitmapProcState.cpp @@ -19,7 +19,7 @@ #include "SkImageEncoder.h" #include "SkResourceCache.h" -#if !SK_ARM_NEON_IS_NONE +#if defined(SK_ARM_HAS_NEON) // These are defined in src/opts/SkBitmapProcState_arm_neon.cpp extern const SkBitmapProcState::SampleProc32 gSkBitmapProcStateSample32_neon[]; extern void S16_D16_filter_DX_neon(const SkBitmapProcState&, const uint32_t*, int, uint16_t*); @@ -296,7 +296,7 @@ bool SkBitmapProcState::chooseScanlineProcs(bool trivialMatrix, bool clampClamp) return false; } -#if !SK_ARM_NEON_IS_ALWAYS +#if !defined(SK_ARM_HAS_NEON) static const SampleProc32 gSkBitmapProcStateSample32[] = { S32_opaque_D32_nofilter_DXDY, S32_alpha_D32_nofilter_DXDY, diff --git a/src/core/SkBitmapProcState_matrixProcs.cpp b/src/core/SkBitmapProcState_matrixProcs.cpp index 16f1bc6..1b747de 100644 --- a/src/core/SkBitmapProcState_matrixProcs.cpp +++ b/src/core/SkBitmapProcState_matrixProcs.cpp @@ -47,16 +47,16 @@ void decal_filter_scale(uint32_t dst[], SkFixed fx, SkFixed dx, int count); /////////////////////////////////////////////////////////////////////////////// // Compile neon code paths if needed -#if !SK_ARM_NEON_IS_NONE +#if defined(SK_ARM_HAS_NEON) // These are defined in src/opts/SkBitmapProcState_matrixProcs_neon.cpp extern const SkBitmapProcState::MatrixProc ClampX_ClampY_Procs_neon[]; extern const SkBitmapProcState::MatrixProc RepeatX_RepeatY_Procs_neon[]; -#endif // !SK_ARM_NEON_IS_NONE +#endif // defined(SK_ARM_HAS_NEON) // Compile non-neon code path if needed -#if !SK_ARM_NEON_IS_ALWAYS +#if !defined(SK_ARM_HAS_NEON) #define MAKENAME(suffix) ClampX_ClampY ## suffix #define TILEX_PROCF(fx, max) SkClampMax((fx) >> 16, max) #define TILEY_PROCF(fy, max) SkClampMax((fy) >> 16, max) diff --git a/src/core/SkBlitter_RGB16.cpp b/src/core/SkBlitter_RGB16.cpp index 38edd60..066ec61 100644 --- a/src/core/SkBlitter_RGB16.cpp +++ b/src/core/SkBlitter_RGB16.cpp @@ -20,7 +20,7 @@ extern void blitmask_d565_opaque_mips(int width, int height, uint16_t* device, uint32_t expanded32, unsigned maskRB); #endif -#if SK_ARM_NEON_IS_ALWAYS && defined(SK_CPU_LENDIAN) +#if defined(SK_ARM_HAS_NEON) && defined(SK_CPU_LENDIAN) #include extern void SkRGB16BlitterBlitV_neon(uint16_t* device, int height, @@ -381,7 +381,7 @@ void SkRGB16_Opaque_Blitter::blitMask(const SkMask& mask, unsigned maskRB = mask.fRowBytes - width; uint32_t expanded32 = fExpandedRaw16; -#if SK_ARM_NEON_IS_ALWAYS && defined(SK_CPU_LENDIAN) +#if defined(SK_ARM_HAS_NEON) && defined(SK_CPU_LENDIAN) #define UNROLL 8 do { int w = width; @@ -475,7 +475,7 @@ void SkRGB16_Opaque_Blitter::blitV(int x, int y, int height, SkAlpha alpha) { unsigned scale5 = SkAlpha255To256(alpha) >> 3; uint32_t src32 = fExpandedRaw16 * scale5; scale5 = 32 - scale5; -#if SK_ARM_NEON_IS_ALWAYS && defined(SK_CPU_LENDIAN) +#if defined(SK_ARM_HAS_NEON) && defined(SK_CPU_LENDIAN) SkRGB16BlitterBlitV_neon(device, height, deviceRB, scale5, src32); #else do { @@ -654,7 +654,7 @@ void SkRGB16_Blitter::blitV(int x, int y, int height, SkAlpha alpha) { unsigned scale5 = SkAlpha255To256(alpha) * fScale >> (8 + 3); uint32_t src32 = fExpandedRaw16 * scale5; scale5 = 32 - scale5; -#if SK_ARM_NEON_IS_ALWAYS && defined(SK_CPU_LENDIAN) +#if defined(SK_ARM_HAS_NEON) && defined(SK_CPU_LENDIAN) SkRGB16BlitterBlitV_neon(device, height, deviceRB, scale5, src32); #else do { diff --git a/src/core/SkOpts.cpp b/src/core/SkOpts.cpp index 54463b2..c6ff43b 100644 --- a/src/core/SkOpts.cpp +++ b/src/core/SkOpts.cpp @@ -82,7 +82,6 @@ namespace SkOpts { void Init_sse42() {} void Init_avx() {} void Init_avx2() {} - void Init_neon(); static void init() { // TODO: Chrome's not linking _sse* opts on iOS simulator builds. Bug or feature? @@ -92,11 +91,6 @@ namespace SkOpts { if (SkCpu::Supports(SkCpu::SSE42)) { Init_sse42(); } if (SkCpu::Supports(SkCpu::AVX )) { Init_avx(); } if (SkCpu::Supports(SkCpu::AVX2 )) { Init_avx2(); } - - #elif defined(SK_CPU_ARM32) && \ - defined(SK_BUILD_FOR_ANDROID) && \ - !defined(SK_BUILD_FOR_ANDROID_FRAMEWORK) - if (SkCpu::Supports(SkCpu::NEON)) { Init_neon(); } #endif } diff --git a/src/core/SkUtilsArm.h b/src/core/SkUtilsArm.h index dde933b..0d35193 100644 --- a/src/core/SkUtilsArm.h +++ b/src/core/SkUtilsArm.h @@ -8,75 +8,12 @@ #ifndef SkUtilsArm_DEFINED #define SkUtilsArm_DEFINED -#include "SkCpu.h" -#include "SkUtils.h" +#include "SkTypes.h" -// Define SK_ARM_NEON_MODE to one of the following values -// corresponding respectively to: -// - No ARM Neon support at all (not targetting ARMv7-A, or don't have NEON) -// - Full ARM Neon support (i.e. assume the CPU always supports it) -// - Optional ARM Neon support (i.e. probe CPU at runtime) -// -#define SK_ARM_NEON_MODE_NONE 0 -#define SK_ARM_NEON_MODE_ALWAYS 1 -#define SK_ARM_NEON_MODE_DYNAMIC 2 - -#if defined(SK_ARM_HAS_OPTIONAL_NEON) -# define SK_ARM_NEON_MODE SK_ARM_NEON_MODE_DYNAMIC -#elif defined(SK_ARM_HAS_NEON) -# define SK_ARM_NEON_MODE SK_ARM_NEON_MODE_ALWAYS -#else -# define SK_ARM_NEON_MODE SK_ARM_NEON_MODE_NONE -#endif - -// Convenience test macros, always defined as 0 or 1 -#define SK_ARM_NEON_IS_NONE (SK_ARM_NEON_MODE == SK_ARM_NEON_MODE_NONE) -#define SK_ARM_NEON_IS_ALWAYS (SK_ARM_NEON_MODE == SK_ARM_NEON_MODE_ALWAYS) -#define SK_ARM_NEON_IS_DYNAMIC (SK_ARM_NEON_MODE == SK_ARM_NEON_MODE_DYNAMIC) - -// The sk_cpu_arm_has_neon() function returns true iff the target device -// is ARMv7-A and supports Neon instructions. In DYNAMIC mode, this actually -// probes the CPU at runtime (and caches the result). - -static inline bool sk_cpu_arm_has_neon(void) { -#if SK_ARM_NEON_IS_NONE - return false; +#if defined(SK_ARM_HAS_NEON) + #define SK_ARM_NEON_WRAP(x) (x ## _neon) #else - return SkCpu::Supports(SkCpu::NEON); -#endif -} - -// Use SK_ARM_NEON_WRAP(symbol) to map 'symbol' to a NEON-specific symbol -// when applicable. This will transform 'symbol' differently depending on -// the current NEON configuration, i.e.: -// -// NONE -> 'symbol' -// ALWAYS -> 'symbol_neon' -// DYNAMIC -> 'symbol' or 'symbol_neon' depending on runtime check. -// -// The goal is to simplify user code, for example: -// -// return SK_ARM_NEON_WRAP(do_something)(params); -// -// Replaces the equivalent: -// -// #if SK_ARM_NEON_IS_NONE -// return do_something(params); -// #elif SK_ARM_NEON_IS_ALWAYS -// return do_something_neon(params); -// #elif SK_ARM_NEON_IS_DYNAMIC -// if (sk_cpu_arm_has_neon()) -// return do_something_neon(params); -// else -// return do_something(params); -// #endif -// -#if SK_ARM_NEON_IS_NONE -# define SK_ARM_NEON_WRAP(x) (x) -#elif SK_ARM_NEON_IS_ALWAYS -# define SK_ARM_NEON_WRAP(x) (x ## _neon) -#elif SK_ARM_NEON_IS_DYNAMIC -# define SK_ARM_NEON_WRAP(x) (sk_cpu_arm_has_neon() ? x ## _neon : x) + #define SK_ARM_NEON_WRAP(x) (x) #endif #endif // SkUtilsArm_DEFINED diff --git a/src/opts/SkOpts_neon.cpp b/src/opts/SkOpts_neon.cpp deleted file mode 100644 index 751bea2..0000000 --- a/src/opts/SkOpts_neon.cpp +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright 2015 Google Inc. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#include "SkOpts.h" - -#define SK_OPTS_NS sk_neon -#include "SkBlitMask_opts.h" -#include "SkBlitRow_opts.h" -#include "SkBlurImageFilter_opts.h" -#include "SkColorCubeFilter_opts.h" -#include "SkMorphologyImageFilter_opts.h" -#include "SkSwizzler_opts.h" -#include "SkTextureCompressor_opts.h" -#include "SkXfermode_opts.h" - -namespace SkOpts { - void Init_neon() { - create_xfermode = sk_neon::create_xfermode; - - box_blur_xx = sk_neon::box_blur_xx; - box_blur_xy = sk_neon::box_blur_xy; - box_blur_yx = sk_neon::box_blur_yx; - - dilate_x = sk_neon::dilate_x; - dilate_y = sk_neon::dilate_y; - erode_x = sk_neon::erode_x; - erode_y = sk_neon::erode_y; - - texture_compressor = sk_neon::texture_compressor; - fill_block_dimensions = sk_neon::fill_block_dimensions; - - blit_mask_d32_a8 = sk_neon::blit_mask_d32_a8; - - blit_row_color32 = sk_neon::blit_row_color32; - blit_row_s32a_opaque = sk_neon::blit_row_s32a_opaque; - - color_cube_filter_span = sk_neon::color_cube_filter_span; - - RGBA_to_BGRA = sk_neon::RGBA_to_BGRA; - RGBA_to_rgbA = sk_neon::RGBA_to_rgbA; - RGBA_to_bgrA = sk_neon::RGBA_to_bgrA; - RGB_to_RGB1 = sk_neon::RGB_to_RGB1; - RGB_to_BGR1 = sk_neon::RGB_to_BGR1; - gray_to_RGB1 = sk_neon::gray_to_RGB1; - grayA_to_RGBA = sk_neon::grayA_to_RGBA; - grayA_to_rgbA = sk_neon::grayA_to_rgbA; - inverted_CMYK_to_RGB1 = sk_neon::inverted_CMYK_to_RGB1; - inverted_CMYK_to_BGR1 = sk_neon::inverted_CMYK_to_BGR1; - } -} -- 2.7.4