Неожиданная сборка x64 для __atomic_fetch_or с gcc 7.3 - PullRequest
0 голосов
/ 25 июня 2018

Я пытаюсь использовать 64-битный интеграл в качестве растрового изображения и получить / освободить владение отдельными битами атомарно.

Для этого я написал следующий код без блокировки:

#include <cstdint>
#include <atomic>

static constexpr std::uint64_t NO_INDEX = ~std::uint64_t(0);

class AtomicBitMap {
public:
    static constexpr std::uint64_t occupied() noexcept {
        return ~std::uint64_t(0);
    }

    std::uint64_t acquire() noexcept {
        while (true) {
            auto map = mData.load(std::memory_order_relaxed);
            if (map == occupied()) {
                return NO_INDEX;
            }

            std::uint64_t index = __builtin_ctzl(~map);
            auto previous =
                mData.fetch_or(bit(index), std::memory_order_relaxed);
            if ((previous & bit(index)) == 0) {
                return index;
            }
        }
    }

private:
    static constexpr std::uint64_t bit(std::uint64_t index) noexcept {
        return std::uint64_t(1) << index;
    }

    std::atomic_uint64_t mData{ 0 };
};

int main() {
    AtomicBitMap map;
    return map.acquire();
}

Который, на Годболте , дает следующую изолированную сборку:

main:
  mov QWORD PTR [rsp-8], 0
  jmp .L3
.L10:
  not rax
  rep bsf rax, rax
  mov edx, eax
  mov eax, eax
  lock bts QWORD PTR [rsp-8], rax
  jnc .L9
.L3:
  mov rax, QWORD PTR [rsp-8]
  cmp rax, -1
  jne .L10
  ret
.L9:
  movsx rax, edx
  ret

Это именно то, что я ожидал 1 .

@ Jester героически сумел уменьшить моего воспроизводителя 97 строк до гораздо более простого 44 воспроизводителя строк , который я дополнительно уменьшил до 35 строк * * 1023

using u64 = unsigned long long;

struct Bucket {
    u64 mLeaves[16] = {};
};

struct BucketMap {
    u64 acquire() noexcept {
        while (true) {
            u64 map = mData;

            u64 index = (map & 1) ? 1 : 0;
            auto mask = u64(1) << index;

            auto previous =
                __atomic_fetch_or(&mData, mask, __ATOMIC_SEQ_CST);
            if ((previous & mask) == 0) {
                return index;
            }
        }
    }

    __attribute__((noinline)) Bucket acquireBucket() noexcept {
        acquire();
        return Bucket();
    }

    volatile u64 mData = 1;
};

int main() {
    BucketMap map;
    map.acquireBucket();
    return 0;
}

, который генерирует следующую сборку:

BucketMap::acquireBucket():
  mov r8, rdi
  mov rdx, rsi

.L2:
  mov rax, QWORD PTR [rsi]
  xor eax, eax
  lock bts QWORD PTR [rdx], rax
  setc al
  jc .L2
  mov rdi, r8
  mov ecx, 16
  rep stosq
  mov rax, r8
  ret

main:
  sub rsp, 152
  lea rsi, [rsp+8]
  lea rdi, [rsp+16]
  mov QWORD PTR [rsp+8], 1
  call BucketMap::acquireBucket()
  xor eax, eax
  add rsp, 152
  ret

xor eax,eax означает, что сборка здесь всегда пытается получить индекс 0 ..., что приводит к бесконечному циклу.

Я вижу только два объяснения этой сборке:

  1. Я как-то вызвал неопределенное поведение.
  2. В gcc есть ошибка генерации кода.

И я исчерпал все свои идеи относительно того, что может вызвать UB.

Может кто-нибудь объяснить, почему gcc сгенерирует это xor eax,eax?

Примечание: предварительно сообщается для gcc как https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86314.


Используемая версия компилятора:

$ gcc --version
gcc (GCC) 7.3.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is 
NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR 
PURPOSE.

Флаги компилятора:

-Wall -Wextra -Werror -Wduplicated-cond -Wnon-virtual-dtor -Wvla 
-rdynamic -Wno-deprecated-declarations -Wno-type-limits 
-Wno-unused-parameter -Wno-unused-local-typedefs -Wno-unused-value 
-Wno-aligned-new -Wno-implicit-fallthrough -Wno-deprecated 
-Wno-noexcept-type -Wno-register -ggdb -fno-strict-aliasing 
-std=c++17 -Wl,--no-undefined -Wno-sign-compare 
-g -O3 -mpopcnt

1 На самом деле, это лучше, чем я ожидал, компилятор понимает, что fetch_or(bit(index)) и previous & bit(index) эквивалентны использованию bts и проверке флага CF чистого золота .

Ответы [ 3 ]

0 голосов
/ 25 июня 2018

xor-zero / set flags / setcc обычно является лучшим способом создания 32-разрядного целого числа 0/1.

Очевидно, что это безопасно сделать, только если у вас есть резервный регистр для xor - ноль без уничтожения каких-либо входных данных для инструкции (ов) установки флага, так что это довольно очевидная ошибка.

(В противном случае вы можете setcc dl / movzx eax,dl. Отдельные регистры предпочтительнее, поэтомуMOVZX может иметь нулевую задержку (MOV-исключения) на некоторых ЦП, но он находится на критическом пути на других ЦП, поэтому идиома xor / set-flags / setcc предпочтительнее, поскольку на критическом пути меньше инструкций.)

IDK, почему gcc вообще создает целочисленное значение (previous & mask) == 0 в регистре;это, вероятно, часть ошибки.

0 голосов
/ 28 июня 2018

Это ошибка оптимизации глазка в gcc, см. # 86413 , влияющая на версии 7.1, 7.2, 7.3 и 8.1. Исправление уже имеется и будет доставлено в версиях 7.4 и 8.2 соответственно.


Короткий ответ заключается в том, что конкретная кодовая последовательность (fetch_or + результат проверки) генерирует setcc (установить условно, то есть на основе состояния флагов), за которым следует movzbl (перемещение и расширение с нуля); в 7.x была введена оптимизация, которая преобразует setcc с последующим movzbl в xor с последующим setcc, однако в этой оптимизации отсутствовали некоторые проверки, в результате которых xor возможно сжимал регистр, который до сих пор необходимо (в данном случае eax).


Более длинный ответ заключается в том, что fetch_or может быть реализован либо как cmpxchg для полной общности, либо, если установлен только один бит, как bts (проверка и установка битов). Как еще одна оптимизация, представленная в 7.x, gcc теперь генерирует bts здесь (gcc 6.4 все еще генерирует cmpxchg). bts устанавливает флаг переноса (CF) на предыдущее значение бита.

То есть auto previous = a.fetch_or(bit); auto n = previous & bit; сгенерирует:

  • lock bts QWORD PTR [<address of a>], <bit index> для установки бита и захвата его предыдущего значения,
  • setc <n>l для установки младших 8 битов r<n>x в значение флага переноса (CF),
  • movzx e<n>x, <n>l для обнуления старших битов r<n>x.

И тогда будет применена оптимизация глазка, которая все испортит.

gcc trunk теперь генерирует правильную сборку :

BucketMap::acquireBucket():
    mov rdx, rdi
    mov rcx, rsi
.L2:
    mov rax, QWORD PTR [rsi]
    and eax, 1
    lock bts QWORD PTR [rcx], rax
    setc al
    movzx eax, al
    jc .L2
    mov rdi, rdx
    mov ecx, 16
    rep stosq
    mov rax, rdx
    ret
main:
    sub rsp, 152
    lea rsi, [rsp+8]
    lea rdi, [rsp+16]
    mov QWORD PTR [rsp+8], 1
    call BucketMap::acquireBucket()
    xor eax, eax
    add rsp, 152
    ret

Хотя, к сожалению, оптимизация больше не применяется, поэтому у нас осталось setc + mov вместо xor + setc ... но, по крайней мере, это правильно!

0 голосов
/ 25 июня 2018

В качестве примечания вы можете найти младший бит 0 с помощью прямой битовой манипуляции:

template<class T>
T find_lowest_0_bit_mask(T value) {
    T t = value + 1;
    return (t ^ value) & t;
}

Возвращает битовую маску, а не битовый индекс.

Прецеденты: T должно быть без знака, value должно содержать хотя бы 1 нулевой бит.


mData.load должно синхронизироваться с mData.fetch_or, поэтому оно должно быть

mData.load(std::memory_order_acquire)

и

mData.fetch_or(..., std::memory_order_release)

И, IMO, есть что-то в этих битовых свойствах, которые заставляют его генерировать неправильную сборку с clang, см. .LBB0_5 цикл, который явно неверен , потому чтоон продолжает пытаться установить тот же бит, а не пересчитывать другой бит для установки.Версия, которая генерирует правильную сборку :

#include <cstdint>
#include <atomic>

static constexpr int NO_INDEX = -1;

template<class T>
T find_lowest_0_bit_mask(T value) {
    T t = value + 1;
    return (t ^ value) & t;
}

class AtomicBitMap {
public:
    static constexpr std::uint64_t occupied() noexcept { return ~std::uint64_t(0); }

    int acquire() noexcept {
        auto map = mData.load(std::memory_order_acquire);
        while(map != occupied()) {
            std::uint64_t mask = find_lowest_0_bit_mask(map);
            if(mData.compare_exchange_weak(map, map | mask, std::memory_order_release))
                return __builtin_ffsl(mask) - 1;
        }
        return NO_INDEX;
    }

    void release(int i) noexcept {
        mData.fetch_and(~bit(i), std::memory_order_release);
    }

private:
    static constexpr std::uint64_t bit(int index) noexcept { 
        return std::uint64_t(1) << index; 
    }

    std::atomic_uint64_t mData{ 0 };
};
Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...