pingcap / pingcap/tiflash

Eliminate instruction set definitions

Open
#8,993 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

type/enhancement
Dominant language
C++
Stars
1k
Forks
423
Avg merge
1d 15h
Merged PRs (30d)
24

Description

Enhancement

When some instruction set is available, currently we use our own definitions. The definition is checked according to whether specific flags is supported by the compiler and whether specific instruction set is enabled by cmake options. This check is not precise: they are not checked by whether specific instruction set is actually available.

Example: we have -DTIFLASH_ENABLE_AVX_SUPPORT=1:

    set (TIFLASH_COMPILER_AVX2_FLAG "-mavx2")
    set (TEST_FLAG "${TIFLASH_COMPILER_AVX2_FLAG}")
    set (CMAKE_REQUIRED_FLAGS "${TEST_FLAG} -O0")
    check_cxx_source_compiles("
        #include <immintrin.h>
        int main() {
            auto a = _mm256_insert_epi8(__m256i(), 0, 0);
            (void)a;
            auto b = _mm256_add_epi16(__m256i(), __m256i());
            (void)b;
            return 0;
        }
    " HAVE_AVX2)
    if (HAVE_AVX2 AND TIFLASH_ENABLE_AVX_SUPPORT)
        set (COMPILER_FLAGS "${COMPILER_FLAGS} ${TEST_FLAG}")
        add_definitions(-DTIFLASH_ENABLE_AVX_SUPPORT=1)
    endif ()

When targetting at native, on a Haswell CPU, AVX2 could be used in the rest of the code by the compiler, but -DTIFLASH_ENABLE_AVX_SUPPORT may be not defined when TIFLASH_ENABLE_AVX_SUPPORT is not enabled. Users must correctly configure options like TIFLASH_ENABLE_AVX_SUPPORT in order to achieve best performance.

As an improvement, we could use __AVX2__, which is a compiler built in definition, which checks exactly whether AVX2 instruction is available in current compile process, regardless of any cmake rules. This is more precise and more friendly for cross-compilation (see https://github.com/pingcap/tiflash/issues/8976), because we could simply use compiler args like this, without writing complicated cmake detect rules any more:

-target x86_64-unknown-linux-gnu -march=x86-64-v4

Samples:

❯ clang++ -target x86_64-unknown-linux-gnu -dM -E - -march=x86-64-v4 < /dev/null | egrep "SSE|AVX|POPCNT"
#define __AVX2__ 1
#define __AVX512BW__ 1
#define __AVX512CD__ 1
#define __AVX512DQ__ 1
#define __AVX512F__ 1
#define __AVX512VL__ 1
#define __AVX__ 1
#define __POPCNT__ 1
#define __SSE2_MATH__ 1
#define __SSE2__ 1
#define __SSE3__ 1
#define __SSE4_1__ 1
#define __SSE4_2__ 1
#define __SSE_MATH__ 1
#define __SSE__ 1
#define __SSSE3__ 1

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating the CMake instruction-set definitions and compiler checks described in the issue, then trace where flags such as TIFLASH_ENABLE_AVX_SUPPORT are consumed. Compare those checks with compiler built-in macros such as AVX2; the work is done when instruction availability is determined consistently for native and cross-compilation without the redundant definitions.

Written by the indexing model from the issue text.

Assessment

Tech stack
cmake, cpp
Domain
build-system, performance
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.