[Tarantool-patches] [PATCH luajit 1/6] Cleanup and enable external unwinding for more platforms.
Sergey Kaplun
skaplun at tarantool.org
Mon Oct 3 10:36:44 MSK 2022
Hi, Maksim!
Thanks for the patch!
LGTM, just a few nits and several small side notes below.
On 26.09.22, Maksim Kokryashkin wrote:
> (cherry picked from commit e131936133c58de4426c595db2341caf5a1665b5)
>
> This commit enables external unwiding on all platforms, that
Typo: s/unwiding/unwinding/
> create unwind tables by default. Corresponding check is added
> to the build routine.
>
> Also, GC64 mode is now enabled on MacOS by default.
Maybe we should notice, that non-GC64 mode is now forbidden for MacOs?
>
> Maxim Kokryashkin:
> * added the description for the patch
>
> Needed for tarantool/tarantool#6096
> Needed for tarantool/tarantool#7230
> ---
> CMakeLists.txt | 3 +
> cmake/SetTargetFlags.cmake | 22 ++-
> doc/extensions.html | 22 +--
> src/Makefile.original | 11 +-
> src/lj_arch.h | 27 ++--
> src/lj_err.c | 321 +++++++++++++++++++------------------
> 6 files changed, 213 insertions(+), 193 deletions(-)
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 8b49f9d7..c4f3ef62 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -176,6 +176,9 @@ endif()
>
> # Enable GC64 mode for x64.
> option(LUAJIT_ENABLE_GC64 "GC64 mode for x64" OFF)
> +if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
> + set(LUAJIT_ENABLE_GC64 ON)
> +endif()
Side note: So, we shouldn't forget to remove it additional set in the
Tarantool build system.
Also, I see no fortified enabled in the LuaJIT original commit. Is this
because this commit is later than enabling GC64 mode by default?
> if(LUAJIT_ENABLE_GC64)
> AppendFlags(TARGET_C_FLAGS -DLUAJIT_ENABLE_GC64)
> endif()
> diff --git a/cmake/SetTargetFlags.cmake b/cmake/SetTargetFlags.cmake
> index b544d2ac..36896aff 100644
> --- a/cmake/SetTargetFlags.cmake
> +++ b/cmake/SetTargetFlags.cmake
> @@ -15,6 +15,22 @@ endif()
> LuaJITTestArch(TESTARCH "${TARGET_C_FLAGS}")
> LuaJITArch(LUAJIT_ARCH "${TESTARCH}")
>
> +string(FIND ${TARGET_C_FLAGS} "LJ_NO_UNWIND 1" UNWIND_POS)
> +if(UNWIND_POS EQUAL -1)
> + execute_process(
> + COMMAND bash -c "exec 2>/dev/null; echo 'extern void b(void);int a(void){b();return 0;}' | ${CMAKE_C_COMPILER} -c -x c - -o tmpunwind.o && grep -qa -e eh_frame -e __unwind_info tmpunwind.o && echo E; rm -f tmpunwind.o"
Side note: I'm OK with the same check as Mike did.
Does `check_c_compiler_flag(-funwind-tables TESTUNWIND)` check the same
thing, but in a more readable way?
> + WORKING_DIRECTORY ${LUAJIT_SOURCE_DIR}
> + OUTPUT_VARIABLE TESTUNWIND
> + RESULT_VARIABLE TESTUNWIND_RC
> + )
> + if(TESTUNWIND_RC EQUAL 0)
> + string(FIND "${TESTUNWIND}" "E" UNW_TEST_POS)
> + if(NOT UNW_TEST_POS EQUAL -1)
> + AppendFlags(TARGET_C_FLAGS -DLUAJIT_UNWIND_EXTERNAL)
> + endif()
> + endif()
> +endif()
> +
> # Target-specific compiler options.
> #
> # x86/x64 only: For GCC 4.2 or higher and if you don't intend to
> @@ -25,12 +41,6 @@ if(LUAJIT_ARCH STREQUAL "x86")
> endif()
>
<snipped>
> diff --git a/doc/extensions.html b/doc/extensions.html
> index e0f136e2..777e9928 100644
> --- a/doc/extensions.html
> +++ b/doc/extensions.html
<snipped>
> diff --git a/src/Makefile.original b/src/Makefile.original
> index dd1c6a76..c9609700 100644
> --- a/src/Makefile.original
> +++ b/src/Makefile.original
<snipped>
> diff --git a/src/lj_arch.h b/src/lj_arch.h
> index 40129d9e..730be5bf 100644
> --- a/src/lj_arch.h
> +++ b/src/lj_arch.h
<snipped>
> diff --git a/src/lj_err.c b/src/lj_err.c
> index c310daf6..298e5434 100644
> --- a/src/lj_err.c
> +++ b/src/lj_err.c
<snipped>
>
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list