From d0e7fc858f31c2e15c1ef5cd45c7484bb7726419 Mon Sep 17 00:00:00 2001 From: Peter Harris Date: Thu, 5 Sep 2024 14:19:43 +0000 Subject: [PATCH] Move SVE length check to dedicated veneer --- Source/astcenccli_entry.cpp | 13 ++++--- Source/astcenccli_entry2.cpp | 71 ++++++++++++++++++++++++++++++++++ Source/astcenccli_toplevel.cpp | 15 +------ Source/cmake_core.cmake | 43 +++++++++++++------- 4 files changed, 109 insertions(+), 33 deletions(-) create mode 100644 Source/astcenccli_entry2.cpp diff --git a/Source/astcenccli_entry.cpp b/Source/astcenccli_entry.cpp index 0e56c34e..c5ea1734 100644 --- a/Source/astcenccli_entry.cpp +++ b/Source/astcenccli_entry.cpp @@ -18,22 +18,23 @@ /** * @brief Application entry point. * - * This module contains the command line entry point which also performs the - * role of validating the host extended ISA support meets the needs of the - * tools. + * This module contains the first command line entry point veneer, used to + * validate that the host extended ISA availability matches the tool build. + * It is compiled without any extended ISA support so it's guaranteed to be + * executable without any invalid instruction errors. */ #include /** - * @brief The main entry point. + * @brief The main veneer entry point. * * @param argc The number of arguments. * @param argv The vector of arguments. * * @return 0 on success, non-zero otherwise. */ -int astcenc_main( +int astcenc_main_veneer( int argc, char **argv); @@ -311,5 +312,5 @@ int main( return 1; } - return astcenc_main(argc, argv); + return astcenc_main_veneer(argc, argv); } diff --git a/Source/astcenccli_entry2.cpp b/Source/astcenccli_entry2.cpp new file mode 100644 index 00000000..5bb1e296 --- /dev/null +++ b/Source/astcenccli_entry2.cpp @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: Apache-2.0 +// ---------------------------------------------------------------------------- +// Copyright 2024 Arm Limited +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy +// of the License at: +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations +// under the License. +// ---------------------------------------------------------------------------- + +/** + * @brief Application entry point second veneer. + * + * This module contains the second command line entry point veneer, used to + * validate that Arm SVE vector width matches the tool build. When used, it is + * compiled with SVE ISA support but without any vector legnth override, so it + * will see the native SVE vector length exposed to the application. + */ + +#include + +#if ASTCENC_SVE != 0 + #include +#endif + +/** + * @brief The main entry point. + * + * @param argc The number of arguments. + * @param argv The vector of arguments. + * + * @return 0 on success, non-zero otherwise. + */ +int astcenc_main( + int argc, + char **argv); + +/** + * @brief Print a formatted string to stderr. + */ +template +static inline void print_error( + const char* format, + _Args...args +) { + fprintf(stderr, format, args...); +} + +int astcenc_main_veneer( + int argc, + char **argv +) { +#if ASTCENC_SVE != 0 + // svcntw() return compile-time length if used with -msve-vector-bits + if (svcntw() != ASTCENC_SVE) + { + int bits = ASTCENC_SVE * 32; + print_error("ERROR: Host SVE support is not a %u bit implementation\n", bits); + return 1; + } +#endif + + return astcenc_main(argc, argv); +} diff --git a/Source/astcenccli_toplevel.cpp b/Source/astcenccli_toplevel.cpp index cfac9c57..e71a6680 100644 --- a/Source/astcenccli_toplevel.cpp +++ b/Source/astcenccli_toplevel.cpp @@ -1889,25 +1889,12 @@ static void print_diagnostic_images( * * @return 0 on success, non-zero otherwise. */ -int -astcenc_main( +int astcenc_main( int argc, char **argv ) { set_thread_name("astc main"); -#if ASTCENC_SVE != 0 - // Do this check here because is needs SVE instructions so cannot be in - // the veneer check which is compiled as stock Armv8. We know we have SVE - // by the time we get this far, but not the vector width. - if (svcntw() != ASTCENC_SVE) - { - uint32_t bits = ASTCENC_SVE * 32; - print_error("ERROR: Host does not implement %u bit SVE ISA extension\n", bits); - return false; - } -#endif - double start_time = get_time(); if (argc < 2) diff --git a/Source/cmake_core.cmake b/Source/cmake_core.cmake index 172b8950..abe2d074 100644 --- a/Source/cmake_core.cmake +++ b/Source/cmake_core.cmake @@ -1,6 +1,6 @@ # SPDX-License-Identifier: Apache-2.0 # ---------------------------------------------------------------------------- -# Copyright 2020-2023 Arm Limited +# Copyright 2020-2024 Arm Limited # # Licensed under the Apache License, Version 2.0 (the "License"); you may not # use this file except in compliance with the License. You may obtain a copy @@ -105,9 +105,14 @@ endif() if(${ASTCENC_CLI}) # Veneer is compiled without any extended ISA so we can safely do # ISA compatability checks without triggering a SIGILL - add_library(${ASTCENC_TARGET}-veneer + add_library(${ASTCENC_TARGET}-veneer1 astcenccli_entry.cpp) + # Veneer is compiled with extended ISA but without vector length overrides + # so we can safely do SVE vector length compatability checks + add_library(${ASTCENC_TARGET}-veneer2 + astcenccli_entry2.cpp) + add_executable(${ASTCENC_TARGET} astcenccli_error_metrics.cpp astcenccli_image.cpp @@ -119,11 +124,12 @@ if(${ASTCENC_CLI}) target_link_libraries(${ASTCENC_TARGET} PRIVATE - ${ASTCENC_TARGET}-veneer + ${ASTCENC_TARGET}-veneer1 + ${ASTCENC_TARGET}-veneer2 ${ASTCENC_TARGET}-static) endif() -macro(astcenc_set_properties ASTCENC_TARGET_NAME ASTCENC_IS_VENEER) +macro(astcenc_set_properties ASTCENC_TARGET_NAME ASTCENC_VENEER_TYPE) target_compile_features(${ASTCENC_TARGET_NAME} PRIVATE @@ -305,11 +311,17 @@ macro(astcenc_set_properties ASTCENC_TARGET_NAME ASTCENC_IS_VENEER) ASTCENC_POPCNT=0 ASTCENC_F16C=0) - # Enable SVE - if (NOT ${ASTCENC_IS_VENEER}) + # Enable SVE in the core library + if (NOT ${ASTCENC_VENEER_TYPE}) target_compile_options(${ASTCENC_TARGET_NAME} PRIVATE -march=armv8-a+sve -msve-vector-bits=256) + + # Enable SVE without fixed vector length in the veneer + elseif (${ASTCENC_VENEER_TYPE} EQUAL 2) + target_compile_options(${ASTCENC_TARGET_NAME} + PRIVATE + -march=armv8-a+sve) endif() elseif(${ASTCENC_ISA_SIMD} MATCHES "sse2") @@ -340,7 +352,7 @@ macro(astcenc_set_properties ASTCENC_TARGET_NAME ASTCENC_IS_VENEER) ASTCENC_POPCNT=1 ASTCENC_F16C=0) - if (${ASTCENC_IS_VENEER}) + if (NOT ${ASTCENC_VENEER_TYPE}) # Force SSE2 on AppleClang (normally SSE4.1 is the default) target_compile_options(${ASTCENC_TARGET_NAME} PRIVATE @@ -365,7 +377,7 @@ macro(astcenc_set_properties ASTCENC_TARGET_NAME ASTCENC_IS_VENEER) ASTCENC_POPCNT=1 ASTCENC_F16C=1) - if (${ASTCENC_IS_VENEER}) + if (NOT ${ASTCENC_VENEER_TYPE}) # Force SSE2 on AppleClang (normally SSE4.1 is the default) target_compile_options(${ASTCENC_TARGET_NAME} PRIVATE @@ -387,7 +399,7 @@ macro(astcenc_set_properties ASTCENC_TARGET_NAME ASTCENC_IS_VENEER) # which significantly improve performance. Note that this DOES reduce # image quality by up to 0.2 dB (normally much less), but buys an # average of 10-15% performance improvement ... - if((NOT ${ASTCENC_INVARIANCE}) AND (NOT ${ASTCENC_IS_VENEER})) + if((NOT ${ASTCENC_INVARIANCE}) AND (NOT ${ASTCENC_VENEER_TYPE})) target_compile_options(${ASTCENC_TARGET_NAME} PRIVATE $<${is_gnu_fe}:-mfma>) @@ -446,14 +458,19 @@ if(${ASTCENC_SHAREDLIB}) endif() if(${ASTCENC_CLI}) - astcenc_set_properties(${ASTCENC_TARGET}-veneer ON) - astcenc_set_properties(${ASTCENC_TARGET} OFF) + astcenc_set_properties(${ASTCENC_TARGET}-veneer1 1) + astcenc_set_properties(${ASTCENC_TARGET}-veneer2 2) + astcenc_set_properties(${ASTCENC_TARGET} 0) - target_compile_options(${ASTCENC_TARGET} + target_compile_options(${ASTCENC_TARGET}-veneer1 PRIVATE $<${is_msvc_fe}:/W3>) - target_compile_options(${ASTCENC_TARGET}-veneer + target_compile_options(${ASTCENC_TARGET}-veneer2 + PRIVATE + $<${is_msvc_fe}:/W3>) + + target_compile_options(${ASTCENC_TARGET} PRIVATE $<${is_msvc_fe}:/W3>)