Hi, Sergey

thanks for the patch, LGTM.

See a minor comment below.


Sergey

On 14.08.2024 16:55, Sergey Kaplun wrote:
> Before this patch +jit flag is given to the LuaJIT test suite
> unconditionally even when LuaJIT is built as a pure interpreter.
>
> This patch fixes the behaviour by setting the flag only when
> `LUAJIT_DISABLE_JIT` option is not set.
>
> Needed for tarantool/tarantool#9398
> ---
>   test/LuaJIT-tests/CMakeLists.txt | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/test/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt
> index 9fbe9bf5..019762e0 100644
> --- a/test/LuaJIT-tests/CMakeLists.txt
> +++ b/test/LuaJIT-tests/CMakeLists.txt
> @@ -42,11 +42,15 @@ if(LUAJIT_USE_ASAN)
>     endif()
>   endif()
>   
> +if(NOT LUAJIT_DISABLE_JIT)
> +  list(APPEND LUAJIT_TEST_TAGS_EXTRA +jit)
> +endif()
> +
>   if(LUAJIT_NO_UNWIND)
>     # Test <catch_cpp.lua> verifies the interoperability with C++
>     # ABI exceptions, so it requires external unwinding enabled.
>     # Hence, skip them otherwise.
> -  set(LUAJIT_TEST_TAGS_EXTRA +internal_unwinder)
> +  list(APPEND LUAJIT_TEST_TAGS_EXTRA +internal_unwinder)
>   endif()
>   
>   if(CMAKE_C_FLAGS MATCHES "-march=skylake-avx512")
> @@ -55,7 +59,7 @@ if(CMAKE_C_FLAGS MATCHES "-march=skylake-avx512")
>     # built with the enabled AVX512 instruction set, see
>     #https://github.com/tarantool/tarantool/issues/6787.
>     # Hence, skip this when "skylake-avx512" is passed.
> -  set(LUAJIT_TEST_TAGS_EXTRA +avx512)
> +  list(APPEND LUAJIT_TEST_TAGS_EXTRA +avx512)
>   endif()
>   
>   set(TEST_SUITE_NAME "LuaJIT-tests")
> @@ -73,7 +77,7 @@ add_test_suite_target(LuaJIT-tests
>   set(test_title "test/${TEST_SUITE_NAME}")
>   add_test(NAME "${test_title}"
>     COMMAND ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/test.lua
> -                                 +slow +ffi +bit +jit ${LUAJIT_TEST_TAGS_EXTRA}
> +                                 +slow +ffi +bit ${LUAJIT_TEST_TAGS_EXTRA}

LUAJIT_TEST_TAGS_EXTRA is a list and to convert CMake list to a 
whitespace-separated strings

a list should be put in double quotes.

However, I've run tests with options for avx512 and -DLUAJIT_NO_UNWIND=ON

and passing test flags works fine without double quotes.

>     WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
>   )
>   set_tests_properties("${test_title}" PROPERTIES