commit a490a59242e33d3986db43d0e85f35c15bbd09b0 Author: Amit Khandekar Date: Thu Jul 16 13:37:44 2020 +0800 gporca: Use portable way to get frame address. GPOS_ASMFP() used x86_64 assembly instructions to get current frame address. This obviously doesn't compile on other architectures like ARM64. So instead use __builtin_frame_address(), which is available in gcc and presumably clang. Since gcc and clang are the two most common compilers, and since we don't want to support GPORCA on exotic architectures and compilers, don't bother to use any other way to get the frame address. Let configure fail if __builtin_frame_address() is not found, but don't do this check if gporca is disabled. GPORCA's CStackDescriptor::Backtrace() uses frame address. But there is also gp_backtrace() in the backend code that has similar functionality. This commit does not merge these two places. But it prepares the infrastructure to do the merge, including a new macro HAVE__BUILTIN_FRAME_ADDRESS defined in pg_config.h. Discussion: https://groups.google.com/a/greenplum.org/forum/#!topic/gpdb-dev/FgaR_4sGYrk Reviewed-by: Heikki Linnakangas diff --git a/config/c-compiler.m4 b/config/c-compiler.m4 index 710dd86f88..bb5577fb1b 100644 --- a/config/c-compiler.m4 +++ b/config/c-compiler.m4 @@ -273,6 +273,23 @@ fi])# PGAC_C_BUILTIN_UNREACHABLE +# PGAC_C_BUILTIN_FRAME_ADDRESS +# -------------------------- +# Check if the C compiler understands __builtin_frame_address(), +# and define HAVE__BUILTIN_FRAME_ADDRESS if so. +AC_DEFUN([PGAC_C_BUILTIN_FRAME_ADDRESS], +[AC_CACHE_CHECK(for __builtin_frame_address, pgac_cv__builtin_frame_address, +[AC_LINK_IFELSE([AC_LANG_PROGRAM([], +[(void) __builtin_frame_address(0);])], +[pgac_cv__builtin_frame_address=yes], +[pgac_cv__builtin_frame_address=no])]) +if test x"$pgac_cv__builtin_frame_address" = xyes ; then +AC_DEFINE(HAVE__BUILTIN_FRAME_ADDRESS, 1, + [Define to 1 if your compiler understands __builtin_frame_address.]) +fi])# PGAC_C_BUILTIN_FRAME_ADDRESS + + + # PGAC_C_VA_ARGS # -------------- # Check if the C compiler understands C99-style variadic macros, diff --git a/configure b/configure index 7ad9ae3af6..fc9f96ab30 100755 --- a/configure +++ b/configure @@ -15021,6 +15021,37 @@ if test x"$pgac_cv__builtin_unreachable" = xyes ; then $as_echo "#define HAVE__BUILTIN_UNREACHABLE 1" >>confdefs.h +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_frame_address" >&5 +$as_echo_n "checking for __builtin_frame_address... " >&6; } +if ${pgac_cv__builtin_frame_address+:} false; then : + $as_echo_n "(cached) " >&6 +else + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +int +main () +{ +(void) __builtin_frame_address(0); + ; + return 0; +} +_ACEOF +if ac_fn_c_try_link "$LINENO"; then : + pgac_cv__builtin_frame_address=yes +else + pgac_cv__builtin_frame_address=no +fi +rm -f core conftest.err conftest.$ac_objext \ + conftest$ac_exeext conftest.$ac_ext +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__builtin_frame_address" >&5 +$as_echo "$pgac_cv__builtin_frame_address" >&6; } +if test x"$pgac_cv__builtin_frame_address" = xyes ; then + +$as_echo "#define HAVE__BUILTIN_FRAME_ADDRESS 1" >>confdefs.h + fi { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __VA_ARGS__" >&5 $as_echo_n "checking for __VA_ARGS__... " >&6; } @@ -15543,6 +15574,13 @@ _ACEOF fi +if test x"$pgac_cv__builtin_frame_address" = xno && test "$enable_orca" = yes; then : + as_fn_error $? " +Built-in function __builtin_frame_address() is required for ORCA. +Use --disable-orca to disable ORCA support. +" "$LINENO" 5 +fi + if test "$with_zlib" = yes; then # Check that defines z_streamp (versions before about 1.0.4 # did not). While we could work around the lack of z_streamp, it diff --git a/configure.in b/configure.in index 51e836afcf..cc7a221e83 100644 --- a/configure.in +++ b/configure.in @@ -1843,6 +1843,7 @@ PGAC_C_STATIC_ASSERT PGAC_C_TYPES_COMPATIBLE PGAC_C_BUILTIN_CONSTANT_P PGAC_C_BUILTIN_UNREACHABLE +PGAC_C_BUILTIN_FRAME_ADDRESS PGAC_C_VA_ARGS PGAC_STRUCT_TIMEZONE PGAC_UNION_SEMUN @@ -1868,6 +1869,13 @@ AC_CHECK_TYPES([struct option], [], [], #include #endif]) +if test x"$pgac_cv__builtin_frame_address" = xno && test "$enable_orca" = yes; then : + AC_MSG_ERROR([[ +Built-in function __builtin_frame_address() is required for ORCA. +Use --disable-orca to disable ORCA support. +]]) +fi + if test "$with_zlib" = yes; then # Check that defines z_streamp (versions before about 1.0.4 # did not). While we could work around the lack of z_streamp, it diff --git a/src/backend/gporca/libgpos/include/gpos/utils.h b/src/backend/gporca/libgpos/include/gpos/utils.h index b355168d51..8b62ab8901 100644 --- a/src/backend/gporca/libgpos/include/gpos/utils.h +++ b/src/backend/gporca/libgpos/include/gpos/utils.h @@ -19,35 +19,15 @@ #include "gpos/io/COstreamBasic.h" #include "gpos/types.h" -#define GPOS_ASMFP asm volatile("movq %%rbp, %0" : "=g"(ulp)); -#define GPOS_ASMSP asm volatile("movq %%rsp, %0" : "=g"(ulp)); - -#define ALIGNED_16(x) \ - (((ULONG_PTR) x >> 1) << 1 == (ULONG_PTR) x) // checks 16-bit alignment -#define ALIGNED_32(x) \ - (((ULONG_PTR) x >> 2) << 2 == (ULONG_PTR) x) // checks 32-bit alignment -#define ALIGNED_64(x) \ - (((ULONG_PTR) x >> 3) << 3 == (ULONG_PTR) x) // checks 64-bit alignment +#define ALIGNED_16(x) (((ULONG_PTR) x >> 1) << 1 == (ULONG_PTR) x) // checks 16-bit alignment +#define ALIGNED_32(x) (((ULONG_PTR) x >> 2) << 2 == (ULONG_PTR) x) // checks 32-bit alignment +#define ALIGNED_64(x) (((ULONG_PTR) x >> 3) << 3 == (ULONG_PTR) x) // checks 64-bit alignment #define MAX_ALIGNED(x) ALIGNED_64(x) #define ALIGN_STORAGE __attribute__((aligned(8))) -#define GPOS_GET_FRAME_POINTER(x) \ - do \ - { \ - ULONG_PTR ulp; \ - GPOS_ASMFP; \ - x = ulp; \ - } while (0) -#define GPOS_GET_STACK_POINTER(x) \ - do \ - { \ - ULONG_PTR ulp; \ - GPOS_ASMSP; \ - x = ulp; \ - } while (0) - +#define GPOS_GET_FRAME_POINTER(x) ((x) = (ULONG_PTR) __builtin_frame_address(0)) #define GPOS_MSEC_IN_SEC ((ULLONG) 1000) #define GPOS_USEC_IN_MSEC ((ULLONG) 1000) #define GPOS_USEC_IN_SEC (((ULLONG) 1000) * 1000) diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index 2d49657429..a699811824 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -780,6 +780,9 @@ /* Define to 1 if your compiler understands __builtin_constant_p. */ #undef HAVE__BUILTIN_CONSTANT_P +/* Define to 1 if your compiler understands __builtin_frame_address. */ +#undef HAVE__BUILTIN_FRAME_ADDRESS + /* Define to 1 if your compiler understands __builtin_types_compatible_p. */ #undef HAVE__BUILTIN_TYPES_COMPATIBLE_P