Skip to content

Commit

Permalink
MDEV-27936 hardware lock elision on ppc64{,le} failing to compile
Browse files Browse the repository at this point in the history
There is only a very small range of gcc compiler versions
that allow the built_{htm} functions to be defined without -mhtm
being specified as a global C{,XX}FLAGS.

Because the design is centered around enable HTM only in the
functional blocks that use it, this breaks on the inclusion
of the htmxlintrin.h header that includes this.

As a partial mitigation, extented to GNU/clang compilers,
transaction functions gain the attribute "hot".

In general the use of htm is around the optimistic
transaction ability of the function. The key part of using the
hot attribute is to place these functions together so that
a maximization of icache, tlb and OS paging can ensure that
these can be ready to execute by any thread/cpu with the
minimum amount of overhead.

POWER is particularly affected here because the xbegin/xend
functions are not inline.

srw_lock.cc requires the -mhtm cflag, both in the storage
engine and the unit tests.
  • Loading branch information
grooverdan committed Mar 9, 2022
1 parent c61249e commit e8fc62b
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 16 deletions.
10 changes: 10 additions & 0 deletions storage/innobase/CMakeLists.txt
Expand Up @@ -380,6 +380,16 @@ IF(CMAKE_COMPILER_IS_GNUCXX AND CMAKE_SYSTEM_PROCESSOR MATCHES "aarch64"
COMPILE_FLAGS "-O0"
)
ENDIF()

# Older gcc version insist on -mhtm flag for including the
# htmxlintrin.h header. This is also true for new gcc versions
# like 11.2.0 in Debian Sid
IF(CMAKE_SYSTEM_PROCESSOR MATCHES "ppc64|powerpc64")
ADD_COMPILE_FLAGS(
sync/srw_lock.cc
COMPILE_FLAGS "-mhtm"
)
ENDIF()
IF(MSVC)
IF(CMAKE_SIZEOF_VOID_P EQUAL 8)
ADD_COMPILE_FLAGS(
Expand Down
30 changes: 15 additions & 15 deletions storage/innobase/include/transactional_lock_guard.h
Expand Up @@ -45,8 +45,8 @@ bool transactional_lock_enabled();

# include <immintrin.h>
# if defined __GNUC__ && !defined __INTEL_COMPILER
# define TRANSACTIONAL_TARGET __attribute__((target("rtm")))
# define TRANSACTIONAL_INLINE __attribute__((target("rtm"),always_inline))
# define TRANSACTIONAL_TARGET __attribute__((target("rtm"),hot))
# define TRANSACTIONAL_INLINE __attribute__((target("rtm"),hot,always_inline))
# else
# define TRANSACTIONAL_TARGET /* nothing */
# define TRANSACTIONAL_INLINE /* nothing */
Expand All @@ -70,25 +70,25 @@ TRANSACTIONAL_INLINE static inline void xabort() { _xabort(0); }

TRANSACTIONAL_INLINE static inline void xend() { _xend(); }
# elif defined __powerpc64__
# include <htmxlintrin.h>
extern bool have_transactional_memory;
bool transactional_lock_enabled();
# define TRANSACTIONAL_TARGET __attribute__((target("htm")))
# define TRANSACTIONAL_INLINE __attribute__((target("htm"),always_inline))

TRANSACTIONAL_INLINE static inline bool xbegin()
{
return have_transactional_memory &&
__TM_simple_begin() == _HTM_TBEGIN_STARTED;
}

# define TRANSACTIONAL_TARGET __attribute__((hot))
# define TRANSACTIONAL_INLINE __attribute__((hot,always_inline))

/**
Newer gcc compilers only provide __builtin_{htm}
function when the -mhtm is actually provided. So
we've got the option of including it globally, or
pushing down to one file with that enabled and removing
the inline optimization.
*/
TRANSACTIONAL_TARGET bool xbegin();
TRANSACTIONAL_TARGET void xabort();
TRANSACTIONAL_TARGET void xend();
# ifdef UNIV_DEBUG
bool xtest();
# endif

TRANSACTIONAL_INLINE static inline void xabort() { __TM_abort(); }

TRANSACTIONAL_INLINE static inline void xend() { __TM_end(); }
# endif
#endif

Expand Down
17 changes: 16 additions & 1 deletion storage/innobase/sync/srw_lock.cc
Expand Up @@ -55,6 +55,20 @@ TRANSACTIONAL_TARGET
bool xtest() { return have_transactional_memory && _xtest(); }
# endif
#elif defined __powerpc64__
# include <htmxlintrin.h>

__attribute__((target("htm"),hot))
bool xbegin()
{
return have_transactional_memory &&
__TM_simple_begin() == _HTM_TBEGIN_STARTED;
}

__attribute__((target("htm"),hot))
void xabort() { __TM_abort(); }

__attribute__((target("htm"),hot))
void xend() { __TM_end(); }
# ifdef __linux__
# include <sys/auxv.h>

Expand All @@ -79,7 +93,8 @@ bool transactional_lock_enabled()
}

# ifdef UNIV_DEBUG
TRANSACTIONAL_TARGET bool xtest()
__attribute__((target("htm"),hot))
bool xtest()
{
return have_transactional_memory &&
_HTM_STATE (__builtin_ttest ()) == _HTM_TRANSACTIONAL;
Expand Down
6 changes: 6 additions & 0 deletions storage/innobase/unittest/CMakeLists.txt
Expand Up @@ -21,6 +21,12 @@ ADD_EXECUTABLE(innodb_fts-t innodb_fts-t.cc)
TARGET_LINK_LIBRARIES(innodb_fts-t mysys mytap)
ADD_DEPENDENCIES(innodb_fts-t GenError)
MY_ADD_TEST(innodb_fts)
IF(CMAKE_SYSTEM_PROCESSOR MATCHES "ppc64|powerpc64")
ADD_COMPILE_FLAGS(
../sync/srw_lock.cc
COMPILE_FLAGS "-mhtm"
)
ENDIF()
ADD_EXECUTABLE(innodb_sync-t innodb_sync-t.cc ../sync/srw_lock.cc)
TARGET_LINK_LIBRARIES(innodb_sync-t mysys mytap)
ADD_DEPENDENCIES(innodb_sync-t GenError)
Expand Down

0 comments on commit e8fc62b

Please sign in to comment.